<div dir="ltr">On 27 October 2013 14:59, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> This will simplify the addition of layout(location) qualifiers for separate shader objects. This was validated with new piglit tests arb_explicit_attrib_location/1.30/compiler/not-enabled-01.vert and arb_explicit_attrib_location/1.30/compiler/not-enabled-02.vert. Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> --- src/glsl/ast_to_hir.cpp | 23 +++++++++++++++++++++++ src/glsl/glsl_parser.yy | 34 +++++++++++++++------------------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 8441360..4343176 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -2059,6 +2059,9 @@ validate_explicit_location(const struct ast_type_qualifier *qual, switch (state->target) { case vertex_shader: if (var->mode == ir_var_shader_in) { + if (!state->has_explicit_attrib_location()) + goto needed_explicit_attrib_location; + break; } @@ -2073,6 +2076,9 @@ validate_explicit_location(const struct ast_type_qualifier *qual, case fragment_shader: if (var->mode == ir_var_shader_out) { + if (!state->has_explicit_attrib_location()) + goto needed_explicit_attrib_location; + break; } @@ -2122,6 +2128,23 @@ validate_explicit_location(const struct ast_type_qualifier *qual, } } } + + return; + +needed_explicit_attrib_location: + if (state->es_shader) { + _mesa_glsl_error(loc, state, + "%s explicit location requires %s", + mode_string(var), + "GLSL ES 300"); + } else { + _mesa_glsl_error(loc, state, + "%s explicit location requires %s", + mode_string(var), + "GL_ARB_explicit_attrib_location extension or GLSL 330"); + } + + return; </blockquote><div> </div><div>I'm not really a fan of the goto statements here. We already have a convention of adding "check_" functions which return true if something is ok, and report an error and return false if it's not (see check_version, check_precision_qualifiers_allowed, and check_bitwise_operations_allowed). How about if we do a similar thing and create a "check_explicit_attrib_location_allowed()" function. Then the hunks above can just say: </div><div>if (!state->check_explicit_attrib_location_allowed()) </div><div> return; </div><div>With that change, this patch is: </div><div>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><div>I sent out comments on patches 1, 2, and 4. Patches 3 and 5 are: Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> } static void diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy index 0a0708e..4aacc67 100644 --- a/src/glsl/glsl_parser.yy +++ b/src/glsl/glsl_parser.yy @@ -1308,29 +1308,25 @@ layout_qualifier_id: { memset(& $$, 0, sizeof($$)); - if (state->has_explicit_attrib_location()) { - if (match_layout_qualifier("location", $1, state) == 0) { - $$.flags.q.explicit_location = 1; + if (match_layout_qualifier("location", $1, state) == 0) { + $$.flags.q.explicit_location = 1; - if ($3 >= 0) { - $$.location = $3; - } else { - _mesa_glsl_error(& @3, state, - "invalid location %d specified", $3); - YYERROR; - } + if ($3 >= 0) { + $$.location = $3; + } else { + _mesa_glsl_error(& @3, state, "invalid location %d specified", $3); + YYERROR; } + } - if (match_layout_qualifier("index", $1, state) == 0) { - $$.flags.q.explicit_index = 1; + if (match_layout_qualifier("index", $1, state) == 0) { + $$.flags.q.explicit_index = 1; - if ($3 >= 0) { - $$.index = $3; - } else { - _mesa_glsl_error(& @3, state, - "invalid index %d specified", $3); - YYERROR; - } + if ($3 >= 0) { + $$.index = $3; + } else { + _mesa_glsl_error(& @3, state, "invalid index %d specified", $3); + YYERROR; } } -- 1.8.1.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>