[Mesa-dev] [PATCH] glsl: don't allow invariant qualifiers for interface blocks

[Samuel Iglesias Gonsálvez]

On Thursday 19 February 2015 11:39:13 Ian Romanick wrote:
> On 12/09/2014 02:52 AM, Eduardo Lima Mitev wrote:
> > From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> > 
> > GLSL 1.50 and GLSL 4.40 specs, they both say the same in
> > "Interface Blocks" section:
> > 
> > "If no optional qualifier is used in a member-declaration, the
> > qualification of the member includes all in, out, patch, uniform, or
> > buffer as determined by interface-qualifier. If optional qualifiers are
> > used, they can include interpolation qualifiers, auxiliary storage
> > qualifiers, and storage qualifiers and they must declare an input,
> > output, or uniform member consistent with the interface qualifier of the
> > block"
> > 
> > From GLSL ES 3.0, chapter 4.3.7 "Interface Blocks", page 38:
> > 
> > "GLSL ES 3.0 does not support interface blocks for shader inputs or
> > outputs."
> > 
> > and from GLSL ES 3.0, chapter 4.6.1 "The invariant qualifier", page 52.
> > 
> > "Only variables output from a shader can be candidates for invariance.
> > This
> > includes user-defined output variables and the built-in output variables.
> > As only outputs can be declared as invariant, an invariant output from
> > one shader stage will still match an input of a subsequent stage without
> > the input being declared as invariant."
> > 
> > From GLSL ES 1.0, chapter 4.6.1 "The invariant qualifier", page 37.
> > 
> > "Only the following variables may be declared as invariant:
> > * Built-in special variables output from the vertex shader
> > * Varying variables output from the vertex shader
> > * Built-in special variables input to the fragment shader
> > * Varying variables input to the fragment shader
> > * Built-in special variables output from the fragment shader."
> > 
> > This patch fixes the following dEQP tests:
> > 
> > dEQP-GLES3.functional.shaders.declarations.invalid_declarations.invariant_
> > uniform_block_2_vertex
> > dEQP-GLES3.functional.shaders.declarations.invalid_declarations.invariant
> > _uniform_block_2_fragment
> > 
> > No piglit regressions.
> > 
> > Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> > 
> > v2:
> > 
> > - Enable this check for GLSL.
> > 
> > Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> > ---
> > 
> >  src/glsl/glsl_parser.yy | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> > index 7fb8c38..9f2a0a3 100644
> > --- a/src/glsl/glsl_parser.yy
> > +++ b/src/glsl/glsl_parser.yy
> > 
> > @@ -2539,6 +2539,41 @@ basic_interface_block:
> >                               "interface block member does not match "
> >                               "the interface block");
> >           
> >           }
> 
> Blank line here.
> 
> > +         /* From GLSL ES 3.0, chapter 4.3.7 "Interface Blocks", page 38:
> > +          *
> > +          * "GLSL ES 3.0 does not support interface blocks for shader
> > inputs or +          * outputs."
> > +          *
> > +          * And from GLSL ES 3.0, chapter 4.6.1 "The invariant
> > qualifier", page 52. +          *
> > +          * "Only variables output from a shader can be candidates for
> > +          * invariance. This includes user-defined output variables and
> > the +          * built-in output variables. As only outputs can be
> > declared as +          * invariant, an invariant output from one shader
> > stage will +          * still match an input of a subsequent stage
> > without the input being +          * declared as invariant."
> > +          *
> > +          * From GLSL ES 1.0, chapter 4.6.1 "The invariant qualifier",
> > page 37. +          *
> > +          * "Only the following variables may be declared as invariant:
> > +          *  * Built-in special variables output from the vertex shader
> > +          *  * Varying variables output from the vertex shader
> > +          *  * Built-in special variables input to the fragment shader
> > +          *  * Varying variables input to the fragment shader
> > +          *  * Built-in special variables output from the fragment
> > shader."
> In addition to Matt's comments about the comment...
> 
> I don't think there's any reason to mention GLSL ES 1.0.  There are no
> interface blocks in GLSL ES 1.0, and this is inside processing an
> interface block.
> 
> With this and Matt's suggestions applied, this patch is
> 
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>

OK, I will fix them.

Thanks for your review!

Sam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150220/66b832a3/attachment.sig>