[Mesa-dev] [PATCH 3/9] glsl: ignore all but the rightmost layout qualifier name from the rightmost layout qualifier

[Timothy Arceri]

On Wed, 2016-10-26 at 17:40 +0300, Andres Gomez wrote:
> On Wed, 2016-10-26 at 11:47 +1100, Timothy Arceri wrote:
> 
> > 
> > Did you test early_fragment_tests specifically? This is one that I
> > checked is only in merge_in_qualifier() and not merge_qualifier()
> 
> AFAIK, early_fragment_tests is the only default input layout-
> qualifier-
> name for a fragment shader (see the table in the section 4.4 (Layout
> Qualifiers) of the GLSL 4.5 spec), so I don't know how it could
> happen
> something like:
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> layout(...) layout(early_fragment_tests) in;
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> or viceversa. The only possibility would be:
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> layout(early_fragment_tests) layout(early_fragment_tests) in;
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~

Ok, you got me that was a bad example :P

My concern is that we will run into problems using merge_qualifier()
directly to merge default qualifers rather than calling
merge_out_qualifier/merge_in_qualifier.

I'm pretty sure this is a least going to cause validation to be skipped
since it is only called on the incomming qualifier we are merging not
on the merged result. Also if we change it to validate the merged
result then the line numbers might point to the wrong layout.

   /* Generate an error when invalid input layout qualifiers are used.
*/
   if ((q.flags.i & ~valid_out_mask.flags.i) != 0) {
      _mesa_glsl_error(loc, state, "invalid output layout qualifiers
used");
      return false;
   }

> 
> Therefore, I could add management for that layout-qualifier-name
> in merge_qualifier(), but that won't make a difference.
> 
> In addition, the generated tests for early_fragment_tests are
> passing,
> the same than tests/spec/arb_shader_image_load_store/early-z.c
> 
> FTR, I did a quick modification
> of tests/spec/arb_shader_image_load_store/early-z.c to check that the
> only possible combination works. It does. I don't think a test for
> that
> is valuable, though, so I won't send a patch for piglit.
>