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

[Alejandro Piñeiro]

On 26/04/15 00:08, Timothy Arceri wrote:
> On Sat, 2015-04-25 at 18:46 +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.
>> ---
>>
>> Taking into account the explanation on the header about the
>> variable binding (ast.h:553)
>>
>>    /**
>>     * Binding specified via GL_ARB_shading_language_420pack's "binding" keyword.
>>     *
>>     * \note
>>     * This field is only valid if \c explicit_binding is set.
>>     */
>>    int binding;
>>
>> The binding is correct (and should be updated) if explicit_binding is true.
>> But the current behaviour was updating it if it was false. 
>>
>> This was not detected by piglit because all the calls to
>> glBindBufferBase(GL_ATOMIC_COUNTER_BUFFER are using 0.
>>
>> I tested this patch by running all piglit on my system, and I didn't
>> detect regression. I also runned make check without issues.
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=90175
> You should probably convert your test program to a piglit test also so
> this bug can be detected if it happens again in the future.

There are several piglit tests at spec/arb_shader_atomic_counters
testing that you get what you expect while using atomic counters.
Probably it would be enough to just modify some of the already existing
tests, using a non-zero binding point (for example at semantics.c).

But I don't have too much experience tweaking/creating piglit tests.
What option would be preferred for this case? A new test or just modify
a little one of the already available?

Best regards

-- 
Alejandro Piñeiro (apinheiro at igalia.com)