Consider this example: " #version 150 core #extension GL_ARB_shading_language_420pack: require #extension GL_ARB_explicit_attrib_location: require
layout(location=0) out vec4 o; layout(binding=2) layout(binding=3, std140) uniform U { vec4 a; } u[2];" As there is 2 layout-qualifiers for the uniform U and the binding layout-qualifier-id is duplicated, the rules set by the ARB_shading_language_420pack spec state that the rightmost should prevail. Our ast_type_qualifier merges with others in a way that if the value for a layout-qualifier-id is set in both, the object being merged overwrites the value of the object invoking the merge. Hence, the merge has to happen from the left layout towards the right one and this was not happening for interface blocks because we were merging into the default layout qualifier. Now, the merge is done from left to right and, as a last step, we merge into the default layout qualifier if needed, so the values of the explicit layouts prevail over it. Signed-off-by: Andres Gomez <ago...@igalia.com> --- src/compiler/glsl/ast.h | 9 ++++++--- src/compiler/glsl/ast_type.cpp | 9 +++++++++ src/compiler/glsl/glsl_parser.yy | 16 +++++++++++----- src/compiler/glsl/glsl_parser_extras.cpp | 2 -- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h index 55f009a..54119b7 100644 --- a/src/compiler/glsl/ast.h +++ b/src/compiler/glsl/ast.h @@ -725,9 +725,6 @@ struct ast_type_qualifier { */ glsl_base_type image_base_type; - /** Flag to know if this represents a default value for a qualifier */ - bool is_default_qualifier; - /** * Return true if and only if an interpolation qualifier is present. */ @@ -748,6 +745,11 @@ struct ast_type_qualifier { */ bool has_auxiliary_storage() const; + /** + * Return true if and only if a memory qualifier is present. + */ + bool has_memory() const; + bool merge_qualifier(YYLTYPE *loc, _mesa_glsl_parse_state *state, const ast_type_qualifier &q, @@ -1140,6 +1142,7 @@ public: struct _mesa_glsl_parse_state *state); ast_type_qualifier layout; + ast_type_qualifier layout_helper; const char *block_name; /** diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp index 35acce3..f5bd936 100644 --- a/src/compiler/glsl/ast_type.cpp +++ b/src/compiler/glsl/ast_type.cpp @@ -107,6 +107,15 @@ ast_type_qualifier::has_auxiliary_storage() const || this->flags.q.patch; } +bool ast_type_qualifier::has_memory() const +{ + return this->flags.q.coherent + || this->flags.q._volatile + || this->flags.q.restrict_flag + || this->flags.q.read_only + || this->flags.q.write_only; +} + /** * This function merges both duplicate identifies within a single layout and * multiple layout qualifiers on a single variable declaration. The diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy index 38cbd3f..b2e9e6a 100644 --- a/src/compiler/glsl/glsl_parser.yy +++ b/src/compiler/glsl/glsl_parser.yy @@ -838,6 +838,12 @@ declaration: } | interface_block { + ast_interface_block *block = (ast_interface_block *) $1; + if (block->layout_helper.has_layout() || block->layout_helper.has_memory()) { + if (!block->layout.merge_qualifier(& @1, state, block->layout_helper, false)) { + YYERROR; + } + } $$ = $1; } ; @@ -2701,17 +2707,16 @@ interface_block: { ast_interface_block *block = (ast_interface_block *) $2; - if (!state->has_420pack_or_es31() && block->layout.has_layout() && - !block->layout.is_default_qualifier) { + if (!state->has_420pack_or_es31() && block->layout_helper.has_layout()) { _mesa_glsl_error(&@1, state, "duplicate layout(...) qualifiers"); YYERROR; } - if (!block->layout.merge_qualifier(& @1, state, $1, false)) { + if (!$1.merge_qualifier(& @1, state, block->layout_helper, false)) { YYERROR; } - block->layout.is_default_qualifier = false; + block->layout_helper = $1; $$ = block; } @@ -2724,9 +2729,10 @@ interface_block: "memory qualifiers can only be used in the " "declaration of shader storage blocks"); } - if (!block->layout.merge_qualifier(& @1, state, $1, false)) { + if (!$1.merge_qualifier(& @1, state, block->layout_helper, false)) { YYERROR; } + block->layout_helper = $1; $$ = block; } ; diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp index b351180..3b487b3 100644 --- a/src/compiler/glsl/glsl_parser_extras.cpp +++ b/src/compiler/glsl/glsl_parser_extras.cpp @@ -273,12 +273,10 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx, this->default_uniform_qualifier = new(this) ast_type_qualifier(); this->default_uniform_qualifier->flags.q.shared = 1; this->default_uniform_qualifier->flags.q.column_major = 1; - this->default_uniform_qualifier->is_default_qualifier = true; this->default_shader_storage_qualifier = new(this) ast_type_qualifier(); this->default_shader_storage_qualifier->flags.q.shared = 1; this->default_shader_storage_qualifier->flags.q.column_major = 1; - this->default_shader_storage_qualifier->is_default_qualifier = true; this->fs_uses_gl_fragcoord = false; this->fs_redeclares_gl_fragcoord = false; -- 2.9.3 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev