On 4 October 2011 14:49, Brian Paul <span dir="ltr"><<a href="mailto:brianp@vmware.com">brianp@vmware.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">On 10/04/2011 03:19 PM, Paul Berry wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
On 4 October 2011 13:15, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a><br></div><div><div></div><div class="h5">
<mailto:<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>>> wrote:<br>
<br>
On 10/03/2011 02:17 PM, Paul Berry wrote:<br>
<br>
Before this patch, clip planes didn't work properly in Mesa<br>
when using<br>
vertex shaders, because Mesa assigned both gl_ClipVertex and<br>
gl_Position to the same gl_vert_result (VERT_RESULT_HPOS). As a<br>
result, backends couldn't distinguish between the two<br>
variables, so<br>
any shader that wrote different values to them would fail to work<br>
properly.<br>
<br>
This patch paves the way for proper support of gl_ClipVertex by<br>
creating a new enumerated value in gl_vert_result for it<br>
(VERT_RESULT_CLIP_VERTEX). After this patch, a back-end may add<br>
<br>
<br>
What happens in drivers that aren't expecting / don't know about<br>
VERT_RESULT_CLIP_VERTEX? In other words, does this break (more)<br>
i915 and all Gallium drivers? I understand that gl_ClipVertex<br>
already doesn't work anywhere, but it would be a shame to replace<br>
incorrect rendering with a crash.<br>
<br>
I can test out (and patch up) i915 today, but I'd like some<br>
feedback from people that rely on TGSI.<br>
<br>
<br>
Dang, I should have thought of this.<br>
<br>
BTW, an easy way to assess the effect of this patch on a driver would<br>
be to run Piglit with "-t clip" after applying this patch. The patch<br>
should only regress "vs-clip-vertex-const-reject" (which only used to<br>
pass by dumb luck), and nothing should crash or abort. Also, if<br>
"clip-plane-transformation fixed" passes, we can have pretty high<br>
confidence that old fashioned fixed function clipping still works.<br>
<br>
I just did this test on Gallium's LLVM pipe and on vanilla swrast, and<br>
everything looked ok--no regressions other than<br>
vs-clip-vertex-const-reject. BTW, both gallium LLVM pipe and vanilla<br>
swrast have a crash (in "vs-clip-vertex-enables" and<br>
"clip-plane-transformation arb", respectively), but in both cases the<br>
crash seems to be unrelated to this patch.<br>
</div></div></blockquote>
<br>
Can you show me the command lines to run these? I can't find vs-clip-vertex-enables</blockquote><div><br>Oh yeah, some of those tests haven't landed yet--they're in a 3-patch series I sent to the Piglit list yesterday afternoon. If one of you wants to give me a "Reviewed-by" on the Piglit list I'll commit them to master. In the meantime you can get them from branch "clip-plane-transformation" on git://<a href="http://github.com/stereotype441/piglit.git">github.com/stereotype441/piglit.git</a>.<br>
</div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div class="im"><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Unfortunately I don't have the hardware to test anything else so I'd<br>
appreciate feedback from others.<br>
<br>
If worse comes to worst and we do wind up regressing a driver, I<br>
suppose a workaround would be to add a flag to<br>
gl_shader_compiler_options so that the driver can tell the GLSL<br>
compiler either "I understand VERT_RESULT_CLIP_VERTEX" or "I don't,<br>
just do the old buggy behavior". But it seems ugly so I would rather<br>
avoid that sort of thing if we can.<br>
</blockquote>
<br></div>
Gallium doesn't support clip distances yet so we might have to work on that. If you've tested llvmpipe and it looks OK, that's enough for me at this point.<br><font color="#888888">
<br>
-Brian<br>
</font></blockquote></div><br>