[Mesa-dev] [PATCH] glsl: reject explicit location on atomic counter uniforms

[Nicolai Hähnle]

On 12.02.2016 17:53, Timothy Arceri wrote:
> On Thu, 2016-02-11 at 20:10 -0500, Ilia Mirkin wrote:
>> This fixes
>>
>> dEQP-GLES31.functional.uniform_location.negative.atomic_fragment
>> dEQP-GLES31.functional.uniform_location.negative.atomic_vertex
>>
>> Both of which have lines like
>>
>> layout(location = 3, binding = 0, offset = 0) uniform atomic_uint
>> uni0;
>>
>> The ARB_explicit_uniform_location spec makes a very tangential
>> mention
>> regarding atomic counters, but location isn't something that makes
>> sense
>> with them.
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> Had no clue where to stick this check... this seemed like as good a
>> place as any.
>>
>>   src/compiler/glsl/ast_to_hir.cpp | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index dbeb5c0..9fce06b 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -4179,6 +4179,11 @@ ast_declarator_list::hir(exec_list
>> *instructions,
>>               state->atomic_counter_offsets[qual_binding] =
>> qual_offset;
>>            }
>>         }
>
> Maybe we should just make this:
>     else {
>        _mesa_glsl_error(&loc, state, "invalid atomic counter layout
> qualifier");
>     }
>
> ??

FWIW, I like Ilia's original message better because it gives the user 
more information about why exactly their layout qualifier is invalid. 
Helpful error messages are a good thing.

That said, I won't complain too loudly if making the code simpler or 
easier to follow ends up making the error messages slightly less helpful.

Cheers,
Nicolai

>
>> +
>> +      if (type->qualifier.flags.q.explicit_location) {
>> +         _mesa_glsl_error(&loc, state,
>> +                          "atomic counters cannot have an explicit
>> location");
>> +      }
>>      }
>>
>>      if (this->declarations.is_empty()) {
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>