[Mesa-dev] [PATCH v2 23/32] mesa/glsl: add api_enabled flag to gl_transform_feedback_info

[Timothy Arceri]

On 21/10/17 10:14, Jason Ekstrand wrote:
> On Wed, Oct 18, 2017 at 10:32 PM, Jordan Justen 
> <jordan.l.justen at intel.com <mailto:jordan.l.justen at intel.com>> wrote:
> 
>     From: Timothy Arceri <timothy.arceri at collabora.com
>     <mailto:timothy.arceri at collabora.com>>
> 
>     This will be used to disable the shader cache when xfb is enabled
>     via the api as we don't currently allow for it when generating the
>     sha for the shader.
>     ---
>       src/compiler/glsl/link_varyings.cpp | 5 ++++-
>       src/mesa/main/mtypes.h              | 3 +++
>       2 files changed, 7 insertions(+), 1 deletion(-)
> 
>     diff --git a/src/compiler/glsl/link_varyings.cpp
>     b/src/compiler/glsl/link_varyings.cpp
>     index 29842ecacd..823d6899d7 100644
>     --- a/src/compiler/glsl/link_varyings.cpp
>     +++ b/src/compiler/glsl/link_varyings.cpp
>     @@ -1089,9 +1089,12 @@ store_tfeedback_info(struct gl_context *ctx,
>     struct gl_shader_program *prog,
>           * however some drivers expect to receive the list of transform
>     feedback
>           * declarations in order so sort it now for convenience.
>           */
>     -   if (has_xfb_qualifiers)
>     +   if (has_xfb_qualifiers) {
>             qsort(tfeedback_decls, num_tfeedback_decls,
>     sizeof(*tfeedback_decls),
>                   cmp_xfb_offset);
>     +   } else {
>     +      xfb_prog->sh.LinkedTransformFeedback->api_enabled = true;
>     +   }
> 
>          xfb_prog->sh.LinkedTransformFeedback->Varyings =
>             rzalloc_array(xfb_prog, struct
>     gl_transform_feedback_varying_info,
>     diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>     index a5a1398698..d94ed22b65 100644
>     --- a/src/mesa/main/mtypes.h
>     +++ b/src/mesa/main/mtypes.h
>     @@ -1779,6 +1779,9 @@ struct gl_transform_feedback_buffer
>       /** Post-link transform feedback info. */
>       struct gl_transform_feedback_info
>       {
>     +   /* Was xfb enabled via the api or in shader layout qualifiers */
>     +   bool api_enabled;
> 
> 
> I have no idea what this patch is doing.  However, the comment above 
> seems to directly contradict the code higher above.  In particular, we 
> leave this false if there *are* XFB qualifiers.  Maybe this is an 
> artifact of history and we just need a new name/comment?

XFB can be enable two different ways.

1. Via in shaders layout attributes in which case the xfb varyings are 
used in the computation of the shaders sha and everything is fine.

2. Via the API where we currently don't add the xfb varyings to the key 
so we use this patch to disable caching.

Now that we no longer have to worry about fallback paths (where the api 
might have been updated since linking was called) it's probably trivial 
to just add the varyings as part of the key. However this patch is the 
more tested path for now if we are aiming to get this into 17.3.



> 
>     +
>          unsigned NumOutputs;
> 
>          /* Bitmask of active buffer indices. */
>     --
>     2.15.0.rc0
> 
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>     <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
> 
>