[Mesa-dev] [PATCH] glsl: Treat layout-qualifier-id's as case-insensitive in desktop GLSL.

[Kenneth Graunke]

On 10/15/2013 05:50 PM, Paul Berry wrote:
> In desktop GLSL, location qualifiers are case-insensitive.  In GLSL
> ES, they are case-sensitive.  This patch handles the difference by
> using a new function to match layout qualifiers,
> match_layout_qualifier(), which calls either strcmp() or strcasecmp()
> as appropriate.
> 
> Fixes piglit tests:
> - layout-not-case-sensitive-in.geom
> - layout-not-case-sensitive-max-vert.geom
> - layout-not-case-sensitive-out.geom
> - layout-not-case-sensitive.frag
> ---
>  src/glsl/glsl_parser.yy | 74 +++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 59 insertions(+), 15 deletions(-)
> 
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index a1d593f..ba2dc63 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -43,6 +43,32 @@ _mesa_glsl_lex(YYSTYPE *val, YYLTYPE *loc, _mesa_glsl_parse_state *state)
>  {
>     return _mesa_glsl_lexer_lex(val, loc, state->scanner);
>  }
> +
> +static bool match_layout_qualifier(const char *s1, const char *s2,
> +                                   _mesa_glsl_parse_state *state)
> +{
> +   /* From the GLSL 1.50 spec, section 4.3.8 (Layout Qualifiers):
> +    *
> +    *     "The tokens in any layout-qualifier-id-list ... are not case
> +    *     sensitive, unless explicitly noted otherwise."
> +    *
> +    * The text "unless explicitly noted otherwise" appears to be
> +    * vacuous--no desktop GLSL spec (up through GLSL 4.40) notes
> +    * otherwise.

I just wanted to say that this particular piece of spec language annoys
me a great deal.  It's like, "Hey, let's write a specification where we
say one thing, then make an incompatible change just for kicks, but add
cop out language so we can be completely inconsistent and do whatever we
want."  And seriously, ES 3 can't follow the rules GL had been using for
4 years?

GLSL 1.40 introduces layout qualifiers (for UBOs); it explicitly says
they're identifiers, but doesn't explicitly mention case sensitivity.

ARB_explicit_attrib_location says that "location" is an identifier, but
doesn't mention case sensitivity.

ARB_fragment_coord_conventions says "these identifiers are not
case-sensitive, unless explicitly noted otherwise."

AMD_conservative_depth is written against 1.50; it inherits those rules.
ARB_conservative_depth is written against 4.00; it inherits those rules.

So apparently none of the earlier specifications say that it should be
case sensitive, either.  Some don't say anything, and others agree with
1.50+.  It's just ES that's inconsistent.

In light of that, I agree with your patch.

Thanks for fixing this, Paul.
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> +    *
> +    * However, the GLSL ES 3.00 spec says, in section 4.3.8 (Layout
> +    * Qualifiers):
> +    *
> +    *     "As for other identifiers, they are case sensitive."
> +    *
> +    * So we need to do a case-sensitive or a case-insensitive match,
> +    * depending on whether we are compiling for GLSL ES.
> +    */
> +   if (state->es_shader)
> +      return strcmp(s1, s2);
> +   else
> +      return strcasecmp(s1, s2);
> +}