[Mesa-dev] [PATCH] glsl: add matrix layout information to interface block types
Nicolai Hähnle
nhaehnle at gmail.com
Mon Oct 24 12:43:43 UTC 2016
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 <nicolai.haehnle at amd.com>
> ---
> 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 < num_fields; i++) {
> if (interface_vars[i] != NULL)
> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
> index 73e3abd..55e5285 100644
> --- a/src/compiler/glsl_types.cpp
> +++ b/src/compiler/glsl_types.cpp
> @@ -51,7 +51,7 @@ glsl_type::glsl_type(GLenum gl_type,
> gl_type(gl_type),
> base_type(base_type),
> sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
> - sampled_type(0), interface_packing(0),
> + sampled_type(0), interface_packing(0), interface_row_major(0),
> vector_elements(vector_elements), matrix_columns(matrix_columns),
> length(0)
> {
> @@ -83,7 +83,7 @@ glsl_type::glsl_type(GLenum gl_type, glsl_base_type base_type,
> base_type(base_type),
> sampler_dimensionality(dim), sampler_shadow(shadow),
> sampler_array(array), sampled_type(type), interface_packing(0),
> - length(0)
> + interface_row_major(0), length(0)
> {
> mtx_lock(&glsl_type::mutex);
>
> @@ -108,7 +108,7 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
> gl_type(0),
> base_type(GLSL_TYPE_STRUCT),
> sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
> - sampled_type(0), interface_packing(0),
> + sampled_type(0), interface_packing(0), interface_row_major(0),
> vector_elements(0), matrix_columns(0),
> length(num_fields)
> {
> @@ -149,11 +149,13 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
> }
>
> glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
> - enum glsl_interface_packing packing, const char *name) :
> + enum glsl_interface_packing packing,
> + bool row_major, const char *name) :
> gl_type(0),
> base_type(GLSL_TYPE_INTERFACE),
> sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
> sampled_type(0), interface_packing((unsigned) packing),
> + interface_row_major((unsigned) row_major),
> vector_elements(0), matrix_columns(0),
> length(num_fields)
> {
> @@ -197,7 +199,7 @@ glsl_type::glsl_type(const glsl_type *return_type,
> gl_type(0),
> base_type(GLSL_TYPE_FUNCTION),
> sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
> - sampled_type(0), interface_packing(0),
> + sampled_type(0), interface_packing(0), interface_row_major(0),
> vector_elements(0), matrix_columns(0),
> length(num_params)
> {
> @@ -229,7 +231,7 @@ glsl_type::glsl_type(const char *subroutine_name) :
> gl_type(0),
> base_type(GLSL_TYPE_SUBROUTINE),
> sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
> - sampled_type(0), interface_packing(0),
> + sampled_type(0), interface_packing(0), interface_row_major(0),
> vector_elements(1), matrix_columns(1),
> length(0)
> {
> @@ -445,7 +447,7 @@ _mesa_glsl_release_types(void)
> glsl_type::glsl_type(const glsl_type *array, unsigned length) :
> base_type(GLSL_TYPE_ARRAY),
> sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
> - sampled_type(0), interface_packing(0),
> + sampled_type(0), interface_packing(0), interface_row_major(0),
> vector_elements(0), matrix_columns(0),
> length(length), name(NULL)
> {
> @@ -882,6 +884,9 @@ glsl_type::record_compare(const glsl_type *b, bool match_locations) const
> if (this->interface_packing != b->interface_packing)
> return false;
>
> + if (this->interface_row_major != b->interface_row_major)
> + return false;
> +
> /* From the GLSL 4.20 specification (Sec 4.2):
> *
> * "Structures must have the same name, sequence of type names, and
> @@ -1028,9 +1033,10 @@ const glsl_type *
> glsl_type::get_interface_instance(const glsl_struct_field *fields,
> unsigned num_fields,
> enum glsl_interface_packing packing,
> + bool row_major,
> const char *block_name)
> {
> - const glsl_type key(fields, num_fields, packing, block_name);
> + const glsl_type key(fields, num_fields, packing, row_major, block_name);
>
> mtx_lock(&glsl_type::mutex);
>
> @@ -1044,7 +1050,7 @@ glsl_type::get_interface_instance(const glsl_struct_field *fields,
> if (entry == NULL) {
> mtx_unlock(&glsl_type::mutex);
> const glsl_type *t = new glsl_type(fields, num_fields,
> - packing, block_name);
> + packing, row_major, block_name);
> mtx_lock(&glsl_type::mutex);
>
> entry = _mesa_hash_table_insert(interface_types, t, (void *) t);
> diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h
> index b1e2f7a..e5b9f3b 100644
> --- a/src/compiler/glsl_types.h
> +++ b/src/compiler/glsl_types.h
> @@ -137,6 +137,7 @@ struct glsl_type {
> * and \c GLSL_TYPE_UINT are valid.
> */
> unsigned interface_packing:2;
> + unsigned interface_row_major:1;
>
> /* Callers of this ralloc-based new need not call delete. It's
> * easier to just ralloc_free 'mem_ctx' (or any of its ancestors). */
> @@ -282,6 +283,7 @@ struct glsl_type {
> static const glsl_type *get_interface_instance(const glsl_struct_field *fields,
> unsigned num_fields,
> enum glsl_interface_packing packing,
> + bool row_major,
> const char *block_name);
>
> /**
> @@ -770,6 +772,14 @@ struct glsl_type {
> return (enum glsl_interface_packing)interface_packing;
> }
>
> + /**
> + * Check if the type interface is row major
> + */
> + bool get_interface_row_major() const
> + {
> + return (bool) interface_row_major;
> + }
> +
> private:
>
> static mtx_t mutex;
> @@ -799,7 +809,8 @@ private:
>
> /** Constructor for interface types */
> glsl_type(const glsl_struct_field *fields, unsigned num_fields,
> - enum glsl_interface_packing packing, const char *name);
> + enum glsl_interface_packing packing,
> + bool row_major, const char *name);
>
> /** Constructor for interface types */
> glsl_type(const glsl_type *return_type,
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 88397b9..79dd1c5 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2577,6 +2577,7 @@ struct gl_uniform_block
> * cross-validating uniform blocks.
> */
> enum gl_uniform_block_packing _Packing;
> + GLboolean _RowMajor;
> };
>
> /**
>
More information about the mesa-dev
mailing list