On 14 November 2015 at 13:42, Timothy Arceri <t_arc...@yahoo.com.au> wrote:
> From: Timothy Arceri <timothy.arc...@collabora.com>
>
> This patch replaces the old interger constant qualifiers with either
> the new ast_layout_expression type if the qualifier requires merging
> or ast_expression if the qualifier can't have mulitple declarations
> or if all but the newest qualifier is simply ignored.
>
> We also update the process_qualifier_constant() helper to be
> similar to the one in the ast_layout_expression class, but in
> this case it will be used to process the ast_expression qualifiers.
>
> Global shader layout qualifier validation is moved out of the parser
> in this change as we now need to evaluate any constant expression
> before doing the validation.
> ---
> src/glsl/ast.h | 33 +++++------
> src/glsl/ast_to_hir.cpp | 126
> ++++++++++++++++++++++++++++------------
> src/glsl/ast_type.cpp | 69 ++++++++--------------
> src/glsl/glsl_parser.yy | 87 +++++++++------------------
> src/glsl/glsl_parser_extras.cpp | 44 ++++++++++++--
> 5 files changed, 195 insertions(+), 164 deletions(-)
>
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -4079,9 +4113,18 @@ ast_declarator_list::hir(exec_list *instructions,
> */
> if (decl_type && decl_type->contains_atomic()) {
> if (type->qualifier.flags.q.explicit_binding &&
> - type->qualifier.flags.q.explicit_offset)
> - state->atomic_counter_offsets[type->qualifier.binding] =
> - type->qualifier.offset;
> + type->qualifier.flags.q.explicit_offset) {
> + unsigned qual_binding;
> + unsigned qual_offset;
> + if (process_qualifier_constant(state, &loc, "binding",
> + type->qualifier.binding,
> + &qual_binding)
> + && process_qualifier_constant(state, &loc, "offset",
Nitpick: Please leave the && trailing on the previous line.
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> /* Layout qualifiers for tessellation control shaders. */
> if (match_layout_qualifier("vertices", $1, state) == 0) {
> $$.flags.q.vertices = 1;
> -
> - if ($3 <= 0) {
> - _mesa_glsl_error(& @3, state,
> - "invalid vertices (%d) specified", $3);
> - YYERROR;
> - } else if ($3 > (int)state->Const.MaxPatchVertices) {
> - _mesa_glsl_error(& @3, state,
> - "vertices (%d) exceeds "
> - "GL_MAX_PATCH_VERTICES", $3);
> - YYERROR;
> - } else {
> - $$.vertices = $3;
> - if (!state->ARB_tessellation_shader_enable &&
> - !state->is_version(400, 0)) {
> - _mesa_glsl_error(& @1, state,
> - "vertices qualifier requires GLSL 4.00 or "
> - "ARB_tessellation_shader");
> - }
> + $$.vertices = new(ctx) ast_layout_expression(@1, $3);
> + if (!state->ARB_tessellation_shader_enable &&
> + !state->is_version(400, 0)) {
> + _mesa_glsl_error(& @1, state,
> + "vertices qualifier requires GLSL 4.00 or "
> + "ARB_tessellation_shader");
> }
> }
>
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index 1678d89..2f870fc 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -1644,8 +1644,20 @@ set_shader_inout_layout(struct gl_shader *shader,
> switch (shader->Stage) {
> case MESA_SHADER_TESS_CTRL:
> shader->TessCtrl.VerticesOut = 0;
> - if (state->tcs_output_vertices_specified)
> - shader->TessCtrl.VerticesOut = state->out_qualifier->vertices;
> + if (state->tcs_output_vertices_specified) {
> + unsigned vertices;
> + if (state->out_qualifier->vertices->
> + process_qualifier_constant(state, "vertices", &vertices,
> + true)) {
The existing code considering 0 an invalid amount, which afaict that
is incorrect. Splitting that into a separate patch is an overkill,
although mentioning it in the commit message is a (almost) must have
imho.
-Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
