On 12/05/15 23:17, Timothy Arceri wrote: > On Mon, 2015-05-11 at 12:37 +0200, 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> >> 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; >> >> storage->atomic_buffer_index = i; >> storage->offset = var->data.atomic.offset; > Reviewed-by: Timothy Arceri <t_arc...@yahoo.com.au>
Thanks for the review. > Although you may want to cc Ian in on this in case he hasn't seen it, as > he was the one who made the change and may have a reason for doing this > that I don't see. CCed. Ian do you have any comments? BR -- Alejandro Piñeiro (apinhe...@igalia.com) _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
