[Mesa-dev] [PATCH v2 1/2] mesa: Add a gl_vert_result for gl_ClipVertex.

[Brian Paul]

On 10/04/2011 03:19 PM, Paul Berry wrote:
> On 4 October 2011 13:15, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>
>     On 10/03/2011 02:17 PM, Paul Berry wrote:
>
>         Before this patch, clip planes didn't work properly in Mesa
>         when using
>         vertex shaders, because Mesa assigned both gl_ClipVertex and
>         gl_Position to the same gl_vert_result (VERT_RESULT_HPOS).  As a
>         result, backends couldn't distinguish between the two
>         variables, so
>         any shader that wrote different values to them would fail to work
>         properly.
>
>         This patch paves the way for proper support of gl_ClipVertex by
>         creating a new enumerated value in gl_vert_result for it
>         (VERT_RESULT_CLIP_VERTEX).  After this patch, a back-end may add
>
>
>     What happens in drivers that aren't expecting / don't know about
>     VERT_RESULT_CLIP_VERTEX?  In other words, does this break (more)
>     i915 and all Gallium drivers?  I understand that gl_ClipVertex
>     already doesn't work anywhere, but it would be a shame to replace
>     incorrect rendering with a crash.
>
>     I can test out (and patch up) i915 today, but I'd like some
>     feedback from people that rely on TGSI.
>
>
> Dang, I should have thought of this.
>
> BTW, an easy way to assess the effect of this patch on a driver would
> be to run Piglit with "-t clip" after applying this patch.  The patch
> should only regress "vs-clip-vertex-const-reject" (which only used to
> pass by dumb luck), and nothing should crash or abort.  Also, if
> "clip-plane-transformation fixed" passes, we can have pretty high
> confidence that old fashioned fixed function clipping still works.
>
> I just did this test on Gallium's LLVM pipe and on vanilla swrast, and
> everything looked ok--no regressions other than
> vs-clip-vertex-const-reject.  BTW, both gallium LLVM pipe and vanilla
> swrast have a crash (in "vs-clip-vertex-enables" and
> "clip-plane-transformation arb", respectively), but in both cases the
> crash seems to be unrelated to this patch.

Can you show me the command lines to run these?  I can't find 
vs-clip-vertex-enables


> Unfortunately I don't have the hardware to test anything else so I'd
> appreciate feedback from others.
>
> If worse comes to worst and we do wind up regressing a driver, I
> suppose a workaround would be to add a flag to
> gl_shader_compiler_options so that the driver can tell the GLSL
> compiler either "I understand VERT_RESULT_CLIP_VERTEX" or "I don't,
> just do the old buggy behavior".  But it seems ugly so I would rather
> avoid that sort of thing if we can.

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.

-Brian