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

[Timothy Arceri]

On Tue, 2016-10-25 at 18:08 +0300, Andres Gomez wrote:
> On Mon, 2016-10-24 at 14:09 +1100, Timothy Arceri wrote:
> > 
> > On Sat, 2016-10-22 at 23:09 +0300, Andres Gomez wrote:
> > > 
> > >  
> > 
> > 
> > <snip>
> > 
> > > 
> > > +   | layout_qualifier BUFFER ';'
> > >     ;
> > >  
> > >  layout_in_defaults:
> > >     layout_qualifier layout_in_defaults
> > >     {
> > > -      $$ = NULL;
> > > +      $$ = $1;
> > >        if (!state->has_420pack_or_es31()) {
> > >           _mesa_glsl_error(&@1, state, "duplicate layout(...)
> > > qualifiers");
> > >           YYERROR;
> > > -      } else {
> > > -         if (!state->in_qualifier->
> > > -                merge_in_qualifier(& @1, state, $1, $$, false))
> > > {
> > > -            YYERROR;
> > > -         }
> > > -         $$ = $2;
> > >        }
> > > -   }
> > > -   | layout_qualifier IN_TOK ';'
> > > -   {
> > > -      $$ = NULL;
> > > -      if (!state->in_qualifier->
> > > -             merge_in_qualifier(& @1, state, $1, $$, true)) {
> > > +      if (!$$.merge_qualifier(& @1, state, $2, false, true)) {
> > >           YYERROR;
> > >        }
> > >     }
> > 
> > 
> > After a closer look I take back my r-b. I'm pretty sure this will
> > break
> > things for non layout-qualifier-name/value pairs.
> > 
> > For example there are some qualifiers that only apply to in/out
> > globals.
> > 
> > I belive this change will break things like:
> > 
> > layout(...) layout(early_fragment_tests) in;
> > 
> > or maybe it breaks:
> > 
> > layout(early_fragment_tests) layout(...) in;
> > 
> > I forget which way we are going :P but you get the point.
> 
> I've created new extra piglit tests that will come into the v2 series
> I'm preparing and I'm not seeing any regression.

Did you test early_fragment_tests specifically? This is one that I
checked is only in merge_in_qualifier() and not merge_qualifier()

> 
> The tests are like this:
> 
> ~~~~~~~~~~~~~~~~
> 
> // [config]
> // expect_result: pass
> // glsl_version: 1.50
> // require_extensions: GL_ARB_shading_language_420pack
> GL_ARB_gpu_shader5
> // [end config]
> 
> #version 150
> #extension GL_ARB_shading_language_420pack: enable
> #extension GL_ARB_gpu_shader5 : enable
> 
> layout(lines) layout(invocations=4) in;
> layout(triangle_strip, max_vertices=3) out;
> 
> in vec4 Color1[];
> 
> uniform int foo[Color1.length() == 2 ? 1 : -1];
> 
> void main()
> {
> }
> 
> ~~~~~~~~~~~~~~~~
> 
> Also, with an inverted order for the in layout. Both pass.
> 
> I've also added similar tests for out, uniform and buffer.
> 
> FTR, before sending this series to review I run a full piglit and CTS
> run with the Intel driver for a Broadwell card.
>