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

[Samuel Iglesias Gonsálvez]

On Wednesday 18 February 2015 11:51:45 Matt Turner wrote:
> On Tue, Dec 9, 2014 at 2:52 AM, Eduardo Lima Mitev <elima at igalia.com> 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");
> >           
> >           }
> > 
> > +         /* 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.
> 
> I'd snip the rest of the quote after this. I don't think it makes the
> point clearer, and actually just makes it harder to see the important
> bit.
> 
> > 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." +          *
> > +          * From GLSL 4.40 and GLSL 1.50, section "Interface Blocks":
> > +          *
> > +          * "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.
> 
> I'd snip the quote before this.
> 
> > 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"
> > +          */
> > +         if (qualifier.flags.q.invariant)
> > +            _mesa_glsl_error(&@1, state, "invariant qualifiers cannot be
> > used with interface blocks members");
> Try to line wrap this.
> 
> With those comments fixed:
> 
> Reviewed-by: Matt Turner <mattst88 at gmail.com>

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/874a6ea2/attachment-0001.sig>