[Mesa-dev] [PATCH v2] glsl: properly setting var->data.binding if explicit_binding is true

[Timothy Arceri]

On Tue, 2015-04-28 at 10:07 +0200, Alejandro Piñeiro wrote:
> There was a typo on commit c0cd5b, doing it when explicit_binding
> was false. This prevented to use any binding point different to 0.
> 
> Cc: 10.4, 10.5 <mesa-stable at lists.freedesktop.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90175
> ---
> 
> Piglit test to catch this error proposed here:
> http://lists.freedesktop.org/archives/piglit/2015-April/015877.html
> 
> 
>  src/glsl/link_atomics.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/glsl/link_atomics.cpp b/src/glsl/link_atomics.cpp
> index 603873a..9528e42 100644
> --- a/src/glsl/link_atomics.cpp
> +++ b/src/glsl/link_atomics.cpp
> @@ -201,7 +201,7 @@ link_assign_atomic_counter_resources(struct gl_context *ctx,
>           gl_uniform_storage *const storage = &prog->UniformStorage[id];
>  
>           mab.Uniforms[j] = id;
> -         if (!var->data.explicit_binding)
> +         if (var->data.explicit_binding)
>              var->data.binding = i;
>  
>           storage->atomic_buffer_index = i;

I'm no expert on atomic counters (so someone correct me if I'm wrong)
but I've taken a look into this and I think the correct solution may be
to remove the if statement completely. This would match the original
implementation.

The reason for this is that currently Mesa requires all atomic counters
to have an explicit binding point (so currently this is always true),
although the spec doesn't say that this is a requirement. Bindings can
be implied, it's likely that once Mesa does support these implied
bindings the new if statement is probably not going to make sense for
that case either.