On 30/04/15 03:15, Timothy Arceri wrote: > 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-sta...@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.
This is a good point. For example, even with the current code, you need to call glBindBufferBase with binding 0 in order to work, even when the initial value of data.binding is zero. I made that change, and run again a full run of piglit, without finding any regression. In any case, I will not provide a third version of the patch until someone confirms definitely that the if statement should be removed. Thanks for the suggestion Best regards -- Alejandro Piñeiro (apinhe...@igalia.com) _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
