On Mon, 2016-04-18 at 19:44 +0300, Andres Gomez wrote: > Hi, > > I would really appreciate if you could find some time to review this > patch.
Is there a patch somewhere that makes use of this change? > > Thanks! > > On Mon, 2016-04-04 at 19:50 +0300, Andres Gomez wrote: > > > > This generalizes the validation also to be done for variables > > inside > > interface blocks, which, for some cases, was missing. > > > > For a discussion about the additional validation cases included see > > https://lists.freedesktop.org/archives/mesa-dev/2016-March/109117.h > > tm > > l > > and Khronos bug #15671. > > > > Signed-off-by: Andres Gomez <[email protected]> > > --- > > src/compiler/glsl/ast_to_hir.cpp | 316 +++++++++++++++++++++------ > > ------------ > > 1 file changed, 171 insertions(+), 145 deletions(-) > > > > diff --git a/src/compiler/glsl/ast_to_hir.cpp > > b/src/compiler/glsl/ast_to_hir.cpp > > index 7c9be81..e4ebc6b 100644 > > --- a/src/compiler/glsl/ast_to_hir.cpp > > +++ b/src/compiler/glsl/ast_to_hir.cpp > > @@ -2792,8 +2792,164 @@ apply_explicit_binding(struct > > _mesa_glsl_parse_state *state, > > } > > > > > > +static void > > +validate_interpolation_qualifier(struct _mesa_glsl_parse_state > > *state, > > + YYLTYPE *loc, > > + const glsl_interp_qualifier > > interpolation, > > + const struct ast_type_qualifier > > *qual, > > + const struct glsl_type *var_type, > > + ir_variable_mode mode) > > +{ > > + /* Interpolation qualifiers can only apply to shader inputs or > > outputs, but > > + * not to vertex shader inputs nor fragment shader outputs. > > + * > > + * From section 4.3 ("Storage Qualifiers") of the GLSL 1.30 > > spec: > > + * "Outputs from a vertex shader (out) and inputs to a > > fragment > > + * shader (in) can be further qualified with one or more of > > these > > + * interpolation qualifiers" > > + * ... > > + * "These interpolation qualifiers may only precede the > > qualifiers in, > > + * centroid in, out, or centroid out in a declaration. They > > do > > not apply > > + * to the deprecated storage qualifiers varying or centroid > > + * varying. They also do not apply to inputs into a vertex > > shader or > > + * outputs from a fragment shader." > > + * > > + * From section 4.3 ("Storage Qualifiers") of the GLSL ES 3.00 > > spec: > > + * "Outputs from a shader (out) and inputs to a shader (in) > > can be > > + * further qualified with one of these interpolation > > qualifiers." > > + * ... > > + * "These interpolation qualifiers may only precede the > > qualifiers > > + * in, centroid in, out, or centroid out in a declaration. > > They do > > + * not apply to inputs into a vertex shader or outputs from > > a > > + * fragment shader." > > + */ > > + if (state->is_version(130, 300) > > + && interpolation != INTERP_QUALIFIER_NONE) { > > + const char *i = interpolation_string(interpolation); > > + if (mode != ir_var_shader_in && mode != ir_var_shader_out) > > + _mesa_glsl_error(loc, state, > > + "interpolation qualifier `%s' can only > > be > > applied to " > > + "shader inputs or outputs.", i); > > + > > + switch (state->stage) { > > + case MESA_SHADER_VERTEX: > > + if (mode == ir_var_shader_in) { > > + _mesa_glsl_error(loc, state, > > + "interpolation qualifier '%s' cannot > > be > > applied to " > > + "vertex shader inputs", i); > > + } > > + break; > > + case MESA_SHADER_FRAGMENT: > > + if (mode == ir_var_shader_out) { > > + _mesa_glsl_error(loc, state, > > + "interpolation qualifier '%s' cannot > > be > > applied to " > > + "fragment shader outputs", i); > > + } > > + break; > > + default: > > + break; > > + } > > + } > > + > > + /* Interpolation qualifiers cannot be applied to 'centroid' and > > + * 'centroid varying'. > > + * > > + * From section 4.3 ("Storage Qualifiers") of the GLSL 1.30 > > spec: > > + * "interpolation qualifiers may only precede the qualifiers > > in, > > + * centroid in, out, or centroid out in a declaration. They > > do > > not apply > > + * to the deprecated storage qualifiers varying or centroid > > varying." > > + * > > + * These deprecated storage qualifiers do not exist in GLSL ES > > 3.00. > > + */ > > + if (state->is_version(130, 0) > > + && interpolation != INTERP_QUALIFIER_NONE > > + && qual->flags.q.varying) { > > + > > + const char *i = interpolation_string(interpolation); > > + const char *s; > > + if (qual->flags.q.centroid) > > + s = "centroid varying"; > > + else > > + s = "varying"; > > + > > + _mesa_glsl_error(loc, state, > > + "qualifier '%s' cannot be applied to the " > > + "deprecated storage qualifier '%s'", i, s); > > + } > > + > > + /* Integer fragment inputs must be qualified with 'flat'. In > > GLSL ES, > > + * so must integer vertex outputs. > > + * > > + * 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 ("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." > > + * > > + * 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. See Khronos bug #15671. > > + */ > > + if (state->is_version(130, 300) > > + && var_type->contains_integer() > > + && interpolation != INTERP_QUALIFIER_FLAT > > + && ((state->stage == MESA_SHADER_FRAGMENT && mode == > > ir_var_shader_in) > > + || (state->stage == MESA_SHADER_VERTEX && mode == > > ir_var_shader_out > > + && state->es_shader))) { > > + const char *shader_var_type = (state->stage == > > MESA_SHADER_VERTEX) ? > > + "vertex output" : "fragment input"; > > + _mesa_glsl_error(loc, state, "if a %s is (or contains) " > > + "an integer, then it must be qualified with > > 'flat'", > > + shader_var_type); > > + } > > + > > + /* Double fragment inputs must be qualified with 'flat'. > > + * > > + * From the "Overview" of the ARB_gpu_shader_fp64 extension > > spec: > > + * "This extension does not support interpolation of double- > > precision > > + * values; doubles used as fragment shader inputs must be > > qualified as > > + * "flat"." > > + * > > + * From section 4.3.4 ("Inputs") of the GLSL 4.00 spec: > > + * "Fragment shader inputs that are signed or unsigned > > integers, integer > > + * vectors, or any double-precision floating-point type must > > be > > + * qualified with the interpolation qualifier flat." > > + * > > + * Note that the GLSL specs are missing the text "or contain"; > > this is > > + * presumably an oversight. See Khronos bug #15671. > > + * > > + * The 'double' type does not exist in GLSL ES so far. > > + */ > > + if ((state->ARB_gpu_shader_fp64_enable > > + || state->is_version(400, 0)) > > + && var_type->contains_double() > > + && interpolation != INTERP_QUALIFIER_FLAT > > + && state->stage == MESA_SHADER_FRAGMENT > > + && mode == ir_var_shader_in) { > > + _mesa_glsl_error(loc, state, "if a fragment input is (or > > contains) " > > + "a double, then it must be qualified with > > 'flat'"); > > + } > > +} > > + > > static glsl_interp_qualifier > > interpret_interpolation_qualifier(const struct ast_type_qualifier > > *qual, > > + const struct glsl_type > > *var_type, > > ir_variable_mode mode, > > struct _mesa_glsl_parse_state > > *state, > > YYLTYPE *loc) > > @@ -2805,37 +2961,23 @@ interpret_interpolation_qualifier(const > > struct ast_type_qualifier *qual, > > interpolation = INTERP_QUALIFIER_NOPERSPECTIVE; > > else if (qual->flags.q.smooth) > > interpolation = INTERP_QUALIFIER_SMOOTH; > > - else > > - interpolation = INTERP_QUALIFIER_NONE; > > - > > - if (interpolation != INTERP_QUALIFIER_NONE) { > > - if (mode != ir_var_shader_in && mode != ir_var_shader_out) { > > - _mesa_glsl_error(loc, state, > > - "interpolation qualifier `%s' can only > > be > > applied to " > > - "shader inputs or outputs.", > > - interpolation_string(interpolation)); > > - > > - } > > - > > - if ((state->stage == MESA_SHADER_VERTEX && mode == > > ir_var_shader_in) || > > - (state->stage == MESA_SHADER_FRAGMENT && mode == > > ir_var_shader_out)) { > > - _mesa_glsl_error(loc, state, > > - "interpolation qualifier `%s' cannot be > > applied to " > > - "vertex shader inputs or fragment shader > > outputs", > > - interpolation_string(interpolation)); > > - } > > - } else if (state->es_shader && > > - ((mode == ir_var_shader_in && > > - state->stage != MESA_SHADER_VERTEX) || > > - (mode == ir_var_shader_out && > > - state->stage != MESA_SHADER_FRAGMENT))) { > > + else if (state->es_shader && > > + ((mode == ir_var_shader_in && > > + state->stage != MESA_SHADER_VERTEX) || > > + (mode == ir_var_shader_out && > > + state->stage != MESA_SHADER_FRAGMENT))) > > /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec > > says: > > * > > * "When no interpolation qualifier is present, smooth > > interpolation > > * is used." > > */ > > interpolation = INTERP_QUALIFIER_SMOOTH; > > - } > > + else > > + interpolation = INTERP_QUALIFIER_NONE; > > + > > + validate_interpolation_qualifier(state, loc, > > + interpolation, > > + qual, var_type, mode); > > > > return interpolation; > > } > > @@ -3575,7 +3717,8 @@ apply_type_qualifier_to_variable(const struct > > ast_type_qualifier *qual, > > } > > > > var->data.interpolation = > > - interpret_interpolation_qualifier(qual, (ir_variable_mode) > > var->data.mode, > > + interpret_interpolation_qualifier(qual, var->type, > > + (ir_variable_mode) var- > > > > > > data.mode, > > state, loc); > > > > /* Does the declaration use the deprecated 'attribute' or > > 'varying' > > @@ -4745,124 +4888,6 @@ ast_declarator_list::hir(exec_list > > *instructions, > > var->data.how_declared = ir_var_hidden; > > } > > > > - /* Integer fragment inputs must be qualified with > > 'flat'. In > > GLSL ES, > > - * so must integer vertex outputs. > > - * > > - * 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 ("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." > > - * > > - * 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) && > > - var->type->contains_integer() && > > - var->data.interpolation != INTERP_QUALIFIER_FLAT && > > - ((state->stage == MESA_SHADER_FRAGMENT && var->data.mode > > == ir_var_shader_in) > > - || (state->stage == MESA_SHADER_VERTEX && var- > > >data.mode > > == ir_var_shader_out > > - && state->es_shader))) { > > - const char *var_type = (state->stage == > > MESA_SHADER_VERTEX) > > ? > > - "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); > > - } > > - > > - /* Double fragment inputs must be qualified with 'flat'. */ > > - if (var->type->contains_double() && > > - var->data.interpolation != INTERP_QUALIFIER_FLAT && > > - state->stage == MESA_SHADER_FRAGMENT && > > - var->data.mode == ir_var_shader_in) { > > - _mesa_glsl_error(&loc, state, "if a fragment input is (or > > contains) " > > - "a double, then it must be qualified > > with > > 'flat'", > > - var_type); > > - } > > - > > - /* Interpolation qualifiers cannot be applied to 'centroid' > > and > > - * 'centroid varying'. > > - * > > - * From page 29 (page 35 of the PDF) of the GLSL 1.30 spec: > > - * "interpolation qualifiers may only precede the > > qualifiers in, > > - * centroid in, out, or centroid out in a declaration. > > They > > do not apply > > - * to the deprecated storage qualifiers varying or > > centroid > > varying." > > - * > > - * These deprecated storage qualifiers do not exist in GLSL > > ES > > 3.00. > > - */ > > - if (state->is_version(130, 0) > > - && this->type->qualifier.has_interpolation() > > - && this->type->qualifier.flags.q.varying) { > > - > > - const char *i = interpolation_string(var- > > > > > > data.interpolation); > > - const char *s; > > - if (this->type->qualifier.flags.q.centroid) > > - s = "centroid varying"; > > - else > > - s = "varying"; > > - > > - _mesa_glsl_error(&loc, state, > > - "qualifier '%s' cannot be applied to the > > " > > - "deprecated storage qualifier '%s'", i, > > s); > > - } > > - > > - > > - /* Interpolation qualifiers can only apply to vertex shader > > outputs and > > - * fragment shader inputs. > > - * > > - * From page 29 (page 35 of the PDF) of the GLSL 1.30 spec: > > - * "Outputs from a vertex shader (out) and inputs to a > > fragment > > - * shader (in) can be further qualified with one or more > > of > > these > > - * interpolation qualifiers" > > - * > > - * From page 31 (page 37 of the PDF) of the GLSL ES 3.00 > > spec: > > - * "These interpolation qualifiers may only precede the > > qualifiers > > - * in, centroid in, out, or centroid out in a > > declaration. > > They do > > - * not apply to inputs into a vertex shader or outputs > > from > > a > > - * fragment shader." > > - */ > > - if (state->is_version(130, 300) > > - && this->type->qualifier.has_interpolation()) { > > - > > - const char *i = interpolation_string(var- > > > > > > data.interpolation); > > - switch (state->stage) { > > - case MESA_SHADER_VERTEX: > > - if (this->type->qualifier.flags.q.in) { > > - _mesa_glsl_error(&loc, state, > > - "qualifier '%s' cannot be applied > > to > > vertex " > > - "shader inputs", i); > > - } > > - break; > > - case MESA_SHADER_FRAGMENT: > > - if (this->type->qualifier.flags.q.out) { > > - _mesa_glsl_error(&loc, state, > > - "qualifier '%s' cannot be applied > > to > > fragment " > > - "shader outputs", i); > > - } > > - break; > > - default: > > - break; > > - } > > - } > > - > > - > > /* From section 4.3.4 of the GLSL 4.00 spec: > > * "Input variables may not be declared using the patch > > in > > qualifier > > * in tessellation control or geometry shaders." > > @@ -6597,7 +6622,8 @@ > > ast_process_struct_or_iface_block_members(exec_list *instructions, > > fields[i].type = field_type; > > fields[i].name = decl->identifier; > > fields[i].interpolation = > > - interpret_interpolation_qualifier(qual, var_mode, > > state, > > &loc); > > + interpret_interpolation_qualifier(qual, field_type, > > + var_mode, state, > > &loc); > > fields[i].centroid = qual->flags.q.centroid ? 1 : 0; > > fields[i].sample = qual->flags.q.sample ? 1 : 0; > > fields[i].patch = qual->flags.q.patch ? 1 : 0; > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
