glsl: Fix error checking on "flat" keyword to match GLSL ES 3.00, GLSL 1.50.
authorPaul Berry <stereotype441@gmail.com>
Sat, 9 Feb 2013 00:46:20 +0000 (16:46 -0800)
committerPaul Berry <stereotype441@gmail.com>
Wed, 13 Feb 2013 15:58:08 +0000 (07:58 -0800)
All of the GLSL specs from GLSL 1.30 (and GLSL ES 3.00) onward contain
language requiring certain integer variables to be declared with the
"flat" keyword, but they differ in exactly *when* the rule is
enforced:

(a) GLSL 1.30 and 1.40 say that vertex shader outputs having integral
type must be declared as "flat".  There is no restriction on fragment
shader inputs.

(b) GLSL 1.50 through 4.30 say that fragment shader inputs having
integral type must be declared as "flat".  There is no restriction on
vertex shader outputs.

(c) GLSL ES 3.00 says that both vertex shader outputs and fragment
shader inputs having integral type must be declared as "flat".

Previously, Mesa's behaviour was consistent with (a).  This patch
makes it consistent with (b) when compiling desktop shaders, and (c)
when compiling ES shaders.

Rationale for desktop shaders: once we add geometry shaders, (b) really
seems like the right choice, because it requires "flat" in just the
situations where it matters.  Since we may want to extend geometry
shader support back before GLSL 1.50 (via ARB_geometry_shader4), it
seems sensible to apply this rule to all GLSL versions.  Also, this
matches the behaviour of the nVidia proprietary driver for Linux, and
the expectations of Intel's oglconform test suite.

Rationale for ES shaders: since the behaviour specified in GLSL ES
3.00 matches neither pre-GLSL-1.50 nor post-GLSL-1.50 behaviour, it
seems likely that this was a deliberate choice on the part of the GLES
folks to be more restrictive.  Also, the argument in favor of (b)
doesn't apply to GLES, since it doesn't support geometry shaders at
all.

Some discussion about this has already happened on the Mesa-dev list.
See:

http://lists.freedesktop.org/archives/mesa-dev/2013-February/034199.html

Fixes piglit tests:
- glsl-1.30/compiler/interpolation-qualifiers/nonflat-*.frag
- glsl-1.30/compiler/interpolation-qualifiers/vs-flat-int-0{2,3,4,5}.vert
- glsl-es-3.00/compiler/interpolation-qualifiers/varying-struct-nonflat-{int,uint}.frag

Fixes oglconform tests:
- glsl-q-inperpol negative.fragin.{int,uint,ivec,uvec}

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
src/glsl/ast_to_hir.cpp

index 2ff44ada777afffb96fee9bcb6c0c28336a0900f..92065f5b7300c8a3e941f93fe6df20feac929bb6 100644 (file)
@@ -2821,30 +2821,46 @@ ast_declarator_list::hir(exec_list *instructions,
         }
       }
 
-      /* Integer vertex outputs must be qualified with 'flat'.
+      /* Integer fragment inputs must be qualified with 'flat'.  In GLSL ES,
+       * so must integer vertex outputs.
        *
-       * From section 4.3.6 of the GLSL 1.30 spec:
-       *    "If a vertex output is a signed or unsigned integer or integer
-       *    vector, then it must be qualified with the interpolation qualifier
+       * From section 4.3.4 ("Inputs") of the GLSL 1.50 spec:
+       *    "Fragment shader inputs that are signed or unsigned integers or
+       *    integer vectors must be qualified with the interpolation qualifier
        *    flat."
        *
-       * From section 4.3.4 of the GLSL 3.00 ES spec:
+       * From section 4.3.4 ("Input Variables") of the GLSL 3.00 ES spec:
        *    "Fragment shader inputs that are, or contain, signed or unsigned
        *    integers or integer vectors must be qualified with the
        *    interpolation qualifier flat."
        *
-       * Since vertex outputs and fragment inputs must have matching
-       * qualifiers, these two requirements are equivalent.
+       * From section 4.3.6 ("Output Variables") of the GLSL 3.00 ES spec:
+       *    "Vertex shader outputs that are, or contain, signed or unsigned
+       *    integers or integer vectors must be qualified with the
+       *    interpolation qualifier flat."
+       *
+       * Note that prior to GLSL 1.50, this requirement applied to vertex
+       * outputs rather than fragment inputs.  That creates problems in the
+       * presence of geometry shaders, so we adopt the GLSL 1.50 rule for all
+       * desktop GL shaders.  For GLSL ES shaders, we follow the spec and
+       * apply the restriction to both vertex outputs and fragment inputs.
+       *
+       * Note also that the desktop GLSL specs are missing the text "or
+       * contain"; this is presumably an oversight, since there is no
+       * reasonable way to interpolate a fragment shader input that contains
+       * an integer.
        */
-      if (state->is_version(130, 300)
-          && state->target == vertex_shader
-          && state->current_function == NULL
-          && var->type->contains_integer()
-          && var->mode == ir_var_shader_out
-          && var->interpolation != INTERP_QUALIFIER_FLAT) {
-
-         _mesa_glsl_error(&loc, state, "If a vertex output is (or contains) "
-                          "an integer, then it must be qualified with 'flat'");
+      if (state->is_version(130, 300) &&
+          var->type->contains_integer() &&
+          var->interpolation != INTERP_QUALIFIER_FLAT &&
+          ((state->target == fragment_shader && var->mode == ir_var_shader_in)
+           || (state->target == vertex_shader && var->mode == ir_var_shader_out
+               && state->es_shader))) {
+         const char *var_type = (state->target == vertex_shader) ?
+            "vertex output" : "fragment input";
+         _mesa_glsl_error(&loc, state, "If a %s is (or contains) "
+                          "an integer, then it must be qualified with 'flat'",
+                          var_type);
       }