[Mesa-dev] RFC [PATCH 0/7] Fix transform feedback of builtin "varyings".

[Ian Romanick]

On 12/29/2011 09:16 AM, Paul Berry wrote:
> This patch series allows transform feedback to work properly on the
> built-in vertex shader output variables gl_PointSize, gl_ClipVertex,
> and gl_ClipDistance.  gl_PointSize and gl_ClipVertex were broken due
> to bugs in the i965 driver, and were trivial to fix--those are fixed
> in patches 1 and 2.
>
> gl_ClipDistance was harder to fix, since on i965 its 8 floats are
> packed into 2 vec4s, so the linker has to tell the back-end to select
> a single component of one of the vec4's for streaming out.  This
> required changing both core mesa and the back-ends, and adding a new
> field to gl_transform_feedback_info.  However, the work seems worth it
> because it lays some of the groundwork we will need when we get around
> to packing varyings.  Patch 3 adds the new field, patches 4-5 cause
> the back-ends to use it, and patches 6-7 update the linker to populate
> it correctly for gl_ClipDistance.
>
> I'm putting this patch series out as an RFC partly because I want to
> find out if the new field in gl_transform_feedback_info makes sense
> for other driver back-ends, and partly because it is not 100% clear
> from the spec whether transform feedback is intended to work on all
> vertex shader outputs, or just the "varyings" (the ones that are
> interpolated across the surface of a primitive).  Here are the
> arguments I can see for and against going through with this patch
> series:
>
> Arguments against: (1) The GL 3.0 spec says that "The varying
> variables specified in<varyings>  can be either built-in varying
> variables (beginning with 'gl_') or user-defined ones".  But it also
> explicitly states that gl_Position is not a varying variable.  And
> GLSL 1.20 lists gl_Position, gl_PointSize, and gl_ClipVertex in
> section 7.1 ("Vertex Shader Special Variables") rather than section
> 7.6 ("Varying Variables").  It seems clear that there was an intention
> to distinguish between "varyings" and other vertex shader outputs, and
> transform feedback is defined to work on varyings.  (2) In all
> likelihood, most code that uses transform feedback uses it on
> user-defined varyings anyhow, so fixing these built-in variables is
> unlikely to make much difference.  (3) Making transform feedback work
> on gl_ClipDistance adds a lot of complication for the benefit of a
> tiny corner case.
>
> Arguments in favor: (1) Because of transform feedback's intended use
> and its position in the pipeline, the distinction between varyings and
> other vertex shader outputs is irrelevant; in all likelihood the spec
> writers intended for it to work on all vertex shader outputs.  (2) The
> very use of the term "varying" (and hence, this distinction) has
> largely disappeared from the standard as of GLSL 1.30.  (3) nVidia's
> proprietary Linux driver supports transform feedback of all vertex
> shader outputs (except gl_ClipVertex, which has many other bugs), so
> it's conceivable that some code in the wild relies on this feature.
> (4) Fixing transform feedback of gl_ClipVertex provides a nice
> opportunity to prepare for the changes we will have to make to
> transform feedback in order to support varying packing.

Here are the results on an AMD system:

[idr at oscar piglit]$ for i in gl_Color gl_SecondaryColor gl_TexCoord 
gl_FogFragCoord gl_Position gl_PointSize gl_ClipVertex gl_ClipDistance ; 
do echo $i; bin/ext_transform_feedback-builtin-varyings -auto $i; echo; done
gl_Color
PIGLIT: {'result': 'pass' }

gl_SecondaryColor
PIGLIT: {'result': 'pass' }

gl_TexCoord
PIGLIT: {'result': 'pass' }

gl_FogFragCoord
PIGLIT: {'result': 'pass' }

gl_Position
PIGLIT: {'result': 'pass' }

gl_PointSize
Failed to link: A variable name specified with the 
TransformFeedbackVaryings is not declared as an output in the geometry 
or vertex shader.

PIGLIT: {'result': 'fail' }

gl_ClipVertex
PIGLIT: {'result': 'pass' }

gl_ClipDistance
Failed to link: Vertex shader(s) failed to link.
gl_ClipDistance[2] was not declared as an output in current vertex shader!

PIGLIT: {'result': 'fail' }

It would be interesting to see how this test fares on OS X.

> Personally, I'm swayed by the arguments in favor but I would like to
> hear what others think.

I searched a bit through old meeting notes and mailing discussions from 
the OpenGL 3.0 time frame.  It seems to me that your suspicion that "the 
spec writers intended for it to work on all vertex shader outputs" is 
almost certainly correct.

I would consider the failing cases to just be bugs in those other 
implementations.

> [PATCH 1/7] i965: Fix transform feedback of gl_PointSize.
> [PATCH 2/7] i965: Fix transform feedback of gl_ClipVertex.
> [PATCH 3/7] mesa: Add gl_transform_feedback_info::ComponentOffset.
> [PATCH 4/7] i965: Make use of gl_transform_feedback_info::ComponentOffset.
> [PATCH 5/7] gallium: Make use of gl_transform_feedback_info::ComponentOffset.
> [PATCH 6/7] mesa: Make tfeedback_decl::var_name a const char *.
> [PATCH 7/7] mesa: Fix transform feedback of gl_ClipDistance.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev