From dfb57e7d1b005e208684316a6939939f4fb73352 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Fri, 8 Feb 2013 16:46:20 -0800 Subject: [PATCH] glsl: Fix error checking on "flat" keyword to match GLSL ES 3.00, GLSL 1.50. 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 Reviewed-by: Anuj Phogat --- src/glsl/ast_to_hir.cpp | 48 +++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 2ff44ada777..92065f5b730 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -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); } -- 2.30.2