On 05/11/2015 03:37 AM, Alejandro Piñeiro wrote:
> Since commit c0cd5b var->data.binding was set only when explicit_binding
> was false, thas was wrong, should be a test to true. This prevented
> to use any binding point different to 0.
>
> In any case, that if statement is not needed. Right now mesa requires
> all atomic counters to have an explicit binding point. This would match
> the original implementation.
>
> Cc: 10.4, 10.5 <mesa-sta...@lists.freedesktop.org>
^^^^^^^^^^ You should put quotes around this, or it treats it as
two separate e-mail addresses. One is 10.4 (invalid) and the other use
a user named "10.5" at the address mesa-sta...@lists.freedesktop.org
(valid).
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90175
> ---
>
> New version based on Timothy Arceri suggestion at the list. Also
> gentle ping for a formal review of the patch.
>
> src/glsl/link_atomics.cpp | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/glsl/link_atomics.cpp b/src/glsl/link_atomics.cpp
> index 603873a..2cede91 100644
> --- a/src/glsl/link_atomics.cpp
> +++ b/src/glsl/link_atomics.cpp
> @@ -201,8 +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)
> - var->data.binding = i;
> + var->data.binding = i;
I don't think this is correct. If the user specified an explicit
binding, which you correctly assert is required, this will overwrite
that value with some other value. See src/glsl/ast_to_hir.cpp around
line 2663. I suspect the value of ir_variable::data.binding is just
getting lost somewhere (probably not being copied), and your change just
fixes it by luck.
What happens if you change your test to use binding 3 instead of 1?
> storage->atomic_buffer_index = i;
> storage->offset = var->data.atomic.offset;
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
