Re: [Mesa-dev] [PATCH] glsl: add matrix layout information to interface block types
On 21.10.2016 13:15, Iago Toral Quiroga wrote: So far we have been checking that interface block definitions had matching matrix layouts by comparing the definitions of their fields, however, this does not cover the case where the interface blocks are defined with mismatching matrix layouts but don't define any field with a matrix type. In this case Mesa will not fail to link because none of the fields will inherit the mismatching layout qualifier. This patch fixes the problem in the same way we fixed it for packing layout information: we add the the layout information to the interface type and then we check it matches during the uniform block linking process. Fixes: dEQP-GLES31.functional.shaders.linkage.uniform.block.layout_qualifier_mismatch_3 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98245 Makes sense to me. Reviewed-by: Nicolai Hähnle--- src/compiler/glsl/ast_to_hir.cpp | 2 ++ src/compiler/glsl/builtin_variables.cpp | 1 + src/compiler/glsl/link_uniform_blocks.cpp | 5 + src/compiler/glsl/linker.cpp | 6 -- src/compiler/glsl_types.cpp | 24 +++- src/compiler/glsl_types.h | 13 - src/mesa/main/mtypes.h| 1 + 7 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp index 6e2f253..adedcbb 100644 --- a/src/compiler/glsl/ast_to_hir.cpp +++ b/src/compiler/glsl/ast_to_hir.cpp @@ -7516,6 +7516,8 @@ ast_interface_block::hir(exec_list *instructions, glsl_type::get_interface_instance(fields, num_variables, packing, +matrix_layout == + GLSL_MATRIX_LAYOUT_ROW_MAJOR, this->block_name); unsigned component_size = block_type->contains_double() ? 8 : 4; diff --git a/src/compiler/glsl/builtin_variables.cpp b/src/compiler/glsl/builtin_variables.cpp index 10a8750..ca266a4 100644 --- a/src/compiler/glsl/builtin_variables.cpp +++ b/src/compiler/glsl/builtin_variables.cpp @@ -352,6 +352,7 @@ per_vertex_accumulator::construct_interface_instance() const { return glsl_type::get_interface_instance(this->fields, this->num_fields, GLSL_INTERFACE_PACKING_STD140, +false, "gl_PerVertex"); } diff --git a/src/compiler/glsl/link_uniform_blocks.cpp b/src/compiler/glsl/link_uniform_blocks.cpp index bb423c5..c0bdfa9 100644 --- a/src/compiler/glsl/link_uniform_blocks.cpp +++ b/src/compiler/glsl/link_uniform_blocks.cpp @@ -247,6 +247,7 @@ process_block_array(struct uniform_block_array_elements *ub_array, char **name, blocks[i].UniformBufferSize = 0; blocks[i]._Packing = gl_uniform_block_packing(type->interface_packing); + blocks[i]._RowMajor = type->get_interface_row_major(); parcel->process(type, blocks[i].Name); @@ -354,6 +355,7 @@ create_buffer_blocks(void *mem_ctx, struct gl_context *ctx, blocks[i].UniformBufferSize = 0; blocks[i]._Packing = gl_uniform_block_packing(block_type->interface_packing); +blocks[i]._RowMajor = block_type->get_interface_row_major(); parcel.process(block_type, b->has_instance_name ? block_type->name : ""); @@ -486,6 +488,9 @@ link_uniform_blocks_are_compatible(const gl_uniform_block *a, if (a->_Packing != b->_Packing) return false; + if (a->_RowMajor != b->_RowMajor) + return false; + for (unsigned i = 0; i < a->NumUniforms; i++) { if (strcmp(a->Uniforms[i].Name, b->Uniforms[i].Name) != 0) return false; diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 8599590..74f5c28 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -1505,9 +1505,10 @@ private: } glsl_interface_packing packing = (glsl_interface_packing) type->interface_packing; + bool row_major = (bool) type->interface_row_major; const glsl_type *new_ifc_type = glsl_type::get_interface_instance(fields, num_fields, - packing, type->name); + packing, row_major, type->name); delete [] fields; return new_ifc_type; } @@ -1535,9 +1536,10 @@ private: } glsl_interface_packing packing = (glsl_interface_packing) ifc_type->interface_packing; + bool row_major = (bool) ifc_type->interface_row_major; const glsl_type *new_ifc_type = glsl_type::get_interface_instance(fields, num_fields, packing, - ifc_type->name); +
[Mesa-dev] [PATCH] glsl: add matrix layout information to interface block types
So far we have been checking that interface block definitions had matching matrix layouts by comparing the definitions of their fields, however, this does not cover the case where the interface blocks are defined with mismatching matrix layouts but don't define any field with a matrix type. In this case Mesa will not fail to link because none of the fields will inherit the mismatching layout qualifier. This patch fixes the problem in the same way we fixed it for packing layout information: we add the the layout information to the interface type and then we check it matches during the uniform block linking process. Fixes: dEQP-GLES31.functional.shaders.linkage.uniform.block.layout_qualifier_mismatch_3 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98245 --- src/compiler/glsl/ast_to_hir.cpp | 2 ++ src/compiler/glsl/builtin_variables.cpp | 1 + src/compiler/glsl/link_uniform_blocks.cpp | 5 + src/compiler/glsl/linker.cpp | 6 -- src/compiler/glsl_types.cpp | 24 +++- src/compiler/glsl_types.h | 13 - src/mesa/main/mtypes.h| 1 + 7 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp index 6e2f253..adedcbb 100644 --- a/src/compiler/glsl/ast_to_hir.cpp +++ b/src/compiler/glsl/ast_to_hir.cpp @@ -7516,6 +7516,8 @@ ast_interface_block::hir(exec_list *instructions, glsl_type::get_interface_instance(fields, num_variables, packing, +matrix_layout == + GLSL_MATRIX_LAYOUT_ROW_MAJOR, this->block_name); unsigned component_size = block_type->contains_double() ? 8 : 4; diff --git a/src/compiler/glsl/builtin_variables.cpp b/src/compiler/glsl/builtin_variables.cpp index 10a8750..ca266a4 100644 --- a/src/compiler/glsl/builtin_variables.cpp +++ b/src/compiler/glsl/builtin_variables.cpp @@ -352,6 +352,7 @@ per_vertex_accumulator::construct_interface_instance() const { return glsl_type::get_interface_instance(this->fields, this->num_fields, GLSL_INTERFACE_PACKING_STD140, +false, "gl_PerVertex"); } diff --git a/src/compiler/glsl/link_uniform_blocks.cpp b/src/compiler/glsl/link_uniform_blocks.cpp index bb423c5..c0bdfa9 100644 --- a/src/compiler/glsl/link_uniform_blocks.cpp +++ b/src/compiler/glsl/link_uniform_blocks.cpp @@ -247,6 +247,7 @@ process_block_array(struct uniform_block_array_elements *ub_array, char **name, blocks[i].UniformBufferSize = 0; blocks[i]._Packing = gl_uniform_block_packing(type->interface_packing); + blocks[i]._RowMajor = type->get_interface_row_major(); parcel->process(type, blocks[i].Name); @@ -354,6 +355,7 @@ create_buffer_blocks(void *mem_ctx, struct gl_context *ctx, blocks[i].UniformBufferSize = 0; blocks[i]._Packing = gl_uniform_block_packing(block_type->interface_packing); +blocks[i]._RowMajor = block_type->get_interface_row_major(); parcel.process(block_type, b->has_instance_name ? block_type->name : ""); @@ -486,6 +488,9 @@ link_uniform_blocks_are_compatible(const gl_uniform_block *a, if (a->_Packing != b->_Packing) return false; + if (a->_RowMajor != b->_RowMajor) + return false; + for (unsigned i = 0; i < a->NumUniforms; i++) { if (strcmp(a->Uniforms[i].Name, b->Uniforms[i].Name) != 0) return false; diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 8599590..74f5c28 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -1505,9 +1505,10 @@ private: } glsl_interface_packing packing = (glsl_interface_packing) type->interface_packing; + bool row_major = (bool) type->interface_row_major; const glsl_type *new_ifc_type = glsl_type::get_interface_instance(fields, num_fields, - packing, type->name); + packing, row_major, type->name); delete [] fields; return new_ifc_type; } @@ -1535,9 +1536,10 @@ private: } glsl_interface_packing packing = (glsl_interface_packing) ifc_type->interface_packing; + bool row_major = (bool) ifc_type->interface_row_major; const glsl_type *new_ifc_type = glsl_type::get_interface_instance(fields, num_fields, packing, - ifc_type->name); + row_major, ifc_type->name); delete [] fields; for (unsigned i = 0; i <