[Mesa-dev] [PATCH v2] glsl: reset BufferStride with explicit xfb_stride

[Timothy Arceri]

On 22/06/17 02:41, Juan A. Suarez Romero wrote:
> link_xfb_stride_layout_qualifiers() can be called multiple times, and
> each time we call prog->TransformFeedback.BufferStride is reset to 0.
> 
> But we should only reset it if the shaders provide an explicit stride.
> 
> Fixes:
> KHR-GL45.enhanced_layouts.xfb_stride_of_empty_list
> KHR-GL45.enhanced_layouts.xfb_stride_of_empty_list_and_api
> 
> v2: do reset only if shaders provide an explicit stride
> 
> Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> ---
>   src/compiler/glsl/linker.cpp | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index adfa3b7b1d..5a261a64f5 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -1623,8 +1623,14 @@ link_xfb_stride_layout_qualifiers(struct gl_context *ctx,
>                                     struct gl_shader **shader_list,
>                                     unsigned num_shaders)
>   {
> -   for (unsigned i = 0; i < MAX_FEEDBACK_BUFFERS; i++) {
> -      prog->TransformFeedback.BufferStride[i] = 0;
> +   for (unsigned s = 0; s < num_shaders; s++) {
> +      struct gl_shader *shader = shader_list[s];
> +
> +      for (unsigned i = 0; i < MAX_FEEDBACK_BUFFERS; i++) {
> +         if (shader->TransformFeedbackBufferStride[i]) { > +            prog->TransformFeedback.BufferStride[i] = 0;
> +         }
> +      }
>      }
>   
>      for (unsigned i = 0; i < num_shaders; i++) {
> 

There should be no reason to a this extra loop/check. You will end up 
looping over the buffer in every stage for every shader source attached 
to the program in that stage.

The only stage xfb applies to is the last stage in the pipeline before 
the fragment shader. So you should be able to just do:

    if (linked->Stage != MESA_SHADER_FRAGMENT) {
       link_xfb_stride_layout_qualifiers(ctx, prog, linked, shader_list,
                                         num_shaders);
    }

The stride will get reset for earlier stages but that shouldn't matter 
because the last stage before the frag shader will be the last to set it 
and that should be all we care about.