On Fri, Feb 12, 2016 at 5:53 PM, Timothy Arceri <[email protected]> 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 <[email protected]> >> --- >> >> 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"); > }
Nope, that doesn't work. In this case layout(location = 3, binding = 0, offset = 0) it goes into the if () case above, as these are all merged by the time it goes into hir. Also, it's legal to just have binding, in which case it'd go into the else, which we don't want either. This has to be a standalone condition if I do it here. An alternative, as mentioned on IRC, is to stick it into apply_explicit_location in the if (uniform) check. Or maybe somewhere else entirely. -ilia _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
