[Mesa-dev] [PATCH V2 3/3] glsl: reject invalid input layout qualifiers

[Timothy Arceri]

On Mon, 2016-02-22 at 14:25 +0100, Samuel Iglesias Gonsálvez wrote:
> On Sun, 2016-02-14 at 20:28 +1100, Timothy Arceri wrote:
> > Global in validation is already handled, this will do the
> > validation
> > for variables, blocks and block members.
> > 
> > This fixes some CTS tests for the new enhanced layouts transform
> > feedback qualifiers.
> > 
> > V2: add some more valid input flags
> > ---
> >  src/compiler/glsl/ast_type.cpp | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/src/compiler/glsl/ast_type.cpp
> > b/src/compiler/glsl/ast_type.cpp
> > index f7c89af..ed43e7c 100644
> > --- a/src/compiler/glsl/ast_type.cpp
> > +++ b/src/compiler/glsl/ast_type.cpp
> > @@ -147,6 +147,29 @@ ast_type_qualifier::merge_qualifier(YYLTYPE
> > *loc,
> >     stream_layout_mask.flags.i = 0;
> >     stream_layout_mask.flags.q.stream = 1;
> >  
> > +   /* FIXME: We should probably do interface and function param
> > validation
> > +    * separately.
> > +    */
> 
> I prefer to have them separate but we can do it later.
> 
> > +   ast_type_qualifier input_layout_mask;
> > +   input_layout_mask.flags.i = 0;
> > +   input_layout_mask.flags.q.centroid = 1;
> > +   /* Function params can have constant */
> > +   input_layout_mask.flags.q.constant = 1;
> > +   input_layout_mask.flags.q.explicit_component = 1;
> 
> explicit_component is not defined in current master. I suppose this
> patch applies on top of your enhanced layout changes.
> 
> If you plan to push it to master now, remember to remove it.

Good point thanks, and thanks for the review.

> 
> Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> 
> Sam
> 
> > +   input_layout_mask.flags.q.explicit_location = 1;
> > +   input_layout_mask.flags.q.flat = 1;
> > +   input_layout_mask.flags.q.in = 1;
> > +   input_layout_mask.flags.q.invariant = 1;
> > +   input_layout_mask.flags.q.noperspective = 1;
> > +   input_layout_mask.flags.q.origin_upper_left = 1;
> > +   /* Function params 'inout' will set this */
> > +   input_layout_mask.flags.q.out = 1;
> > +   input_layout_mask.flags.q.patch = 1;
> > +   input_layout_mask.flags.q.pixel_center_integer = 1;
> > +   input_layout_mask.flags.q.precise = 1;
> > +   input_layout_mask.flags.q.sample = 1;
> > +   input_layout_mask.flags.q.smooth = 1;
> > +
> >     /* Uniform block layout qualifiers get to overwrite each
> >      * other (rightmost having priority), while all other
> >      * qualifiers currently don't allow duplicates.
> > @@ -271,6 +294,13 @@ ast_type_qualifier::merge_qualifier(YYLTYPE
> > *loc,
> >  
> >     this->flags.i |= q.flags.i;
> >  
> > +   if (this->flags.q.in &&
> > +       (this->flags.i & ~input_layout_mask.flags.i) != 0) {
> > +      _mesa_glsl_error(loc, state,
> > +		       "invalid input layout qualifier used");
> > +      return false;
> > +   }
> > +
> >     if (q.flags.q.explicit_location)
> >        this->location = q.location;
> >