[Mesa-dev] [PATCH 1/2] mesa: Ensure that transform feedback refers to the correct program.

[Kenneth Graunke]

On 01/22/2014 06:07 AM, Paul Berry wrote:
> Previous to this patch, the _mesa_{Begin,Resume}TransformFeedback
> functions were using ctx->Shader.CurrentProgram[MESA_SHADER_VERTEX] to
> find the program that would be the source of transform feedback data.
> This isn't correct--if there's a geometry shader present it should be
> ctx->Shader.CurrentProgram[MESA_SHADER_GEOMETRY].  (These might be
> different if separate shader objects are in use).
> 
> This patch creates a function get_xfb_source(), which figures out the
> correct program to use based on GL state, and updates
> _mesa_{Begin,Resume}TransformFeedback to call it.  get_xfb_source() is
> written in terms of the gl_shader_stage enum, so it should not need
> modification when we add tessellation shaders in the future.  It also
> creates a new driver flag, NewTransformFeedbackProg, which is flagged
> whenever this program changes.
> 
> To reduce future confusion, this patch also rewords some comments and
> error message text to avoid referring to vertex shaders.
> ---
>  src/mesa/main/mtypes.h            |  8 ++++--
>  src/mesa/main/transformfeedback.c | 52 +++++++++++++++++++++++++++++----------
>  2 files changed, 45 insertions(+), 15 deletions(-)
> 
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 3dd9678..7fd3298 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1815,8 +1815,9 @@ struct gl_transform_feedback_object
>  
>     /**
>      * The shader program active when BeginTransformFeedback() was called.
> -    * When active and unpaused, this equals
> -    * ctx->Shader.CurrentProgram[MESA_SHADER_VERTEX].
> +    * When active and unpaused, this equals ctx->Shader.CurrentProgram[stage],
> +    * where stage is the pipeline stage that is the source of data for
> +    * transform feedback.
>      */
>     struct gl_shader_program *shader_program;
>  
> @@ -3779,6 +3780,9 @@ struct gl_driver_flags
>     /** gl_context::TransformFeedback::CurrentObject */
>     GLbitfield NewTransformFeedback;
>  
> +   /** gl_context::TransformFeedback::CurrentObject::shader_program */
> +   GLbitfield NewTransformFeedbackProg;
> +
>     /** gl_context::RasterDiscard */
>     GLbitfield NewRasterizerDiscard;
>  
> diff --git a/src/mesa/main/transformfeedback.c b/src/mesa/main/transformfeedback.c
> index 74897ba..9376a9e 100644
> --- a/src/mesa/main/transformfeedback.c
> +++ b/src/mesa/main/transformfeedback.c
> @@ -24,7 +24,7 @@
>  
>  
>  /*
> - * Vertex transform feedback support.
> + * Transform feedback support.
>   *
>   * Authors:
>   *   Brian Paul
> @@ -376,25 +376,48 @@ _mesa_compute_max_transform_feedback_vertices(
>   **/
>  
>  
> +/**
> + * Figure out which stage of the pipeline is the source of transform feedback
> + * data given the current context state, and return its gl_shader_program.
> + *
> + * If no active program can generate transform feedback data (i.e. no vertex
> + * shader is active), returns NULL.
> + */
> +static struct gl_shader_program *
> +get_xfb_source(struct gl_context *ctx)
> +{
> +   int i;
> +   for (i = MESA_SHADER_FRAGMENT - 1; i >= MESA_SHADER_VERTEX; i--) {

I think this would be clearer as:

for (i = MESA_SHADER_GEOMETRY; i >= MESA_SHADER_VERTEX; i--) {
   ...
}

Note that the pipeline ordering is:
Vertex -> Tess. Control -> Tess. Eval -> Geometry -> Transform Feedback
(http://www.opengl.org/wiki/Rendering_Pipeline_Overview)

So either implementation would work even with tessellation shaders.

Either way, this series is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
Cc: 10.0 <mesa-stable at lists.freedesktop.org>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140122/2f30b96c/attachment-0001.pgp>