[Mesa-dev] [PATCH] glsl: Fix input/output structure matching across shader stages
Vadym Shovkoplias
vadim.shovkoplias at gmail.com
Fri Oct 5 15:09:17 UTC 2018
Section 7.4.1 (Shader Interface Matching) of the OpenGL 4.30 spec says:
"Variables or block members declared as structures are considered
to match in type if and only if structure members match in name,
type, qualification, and declaration order."
Fixes:
* layout-location-struct.shader_test
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108250
Signed-off-by: Vadym Shovkoplias <vadym.shovkoplias at globallogic.com>
---
src/compiler/glsl/ast_to_hir.cpp | 2 +-
src/compiler/glsl/link_varyings.cpp | 55 +++++++++++++++++++----------
src/compiler/glsl_types.cpp | 17 ++++++---
src/compiler/glsl_types.h | 8 +++--
4 files changed, 56 insertions(+), 26 deletions(-)
diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 93e7c8ec33..67ad300236 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -7583,7 +7583,7 @@ ast_struct_specifier::hir(exec_list *instructions,
if (!type->is_anonymous() && !state->symbols->add_type(name, type)) {
const glsl_type *match = state->symbols->get_type(name);
/* allow struct matching for desktop GL - older UE4 does this */
- if (match != NULL && state->is_version(130, 0) && match->record_compare(type, false))
+ if (match != NULL && state->is_version(130, 0) && match->record_compare(type, true, false))
_mesa_glsl_warning(& loc, state, "struct `%s' previously defined", name);
else
_mesa_glsl_error(& loc, state, "struct `%s' previously defined", name);
diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index 52e493cb59..249f475a69 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -214,25 +214,42 @@ cross_validate_types_and_qualifiers(struct gl_context *ctx,
}
if (type_to_match != output->type) {
- /* There is a bit of a special case for gl_TexCoord. This
- * built-in is unsized by default. Applications that variable
- * access it must redeclare it with a size. There is some
- * language in the GLSL spec that implies the fragment shader
- * and vertex shader do not have to agree on this size. Other
- * driver behave this way, and one or two applications seem to
- * rely on it.
- *
- * Neither declaration needs to be modified here because the array
- * sizes are fixed later when update_array_sizes is called.
- *
- * From page 48 (page 54 of the PDF) of the GLSL 1.10 spec:
- *
- * "Unlike user-defined varying variables, the built-in
- * varying variables don't have a strict one-to-one
- * correspondence between the vertex language and the
- * fragment language."
- */
- if (!output->type->is_array() || !is_gl_identifier(output->name)) {
+ if (output->type->is_record()) {
+ /* Structures across shader stages can have different name
+ * and considered to match in type if and only if structure
+ * members match in name, type, qualification, and declaration
+ * order.
+ */
+ if(!output->type->record_compare(type_to_match, false, true)) {
+ linker_error(prog,
+ "%s shader output %s' declared as struct `%s', "
+ "doesn't match in type with %s shader input "
+ "declared as struct `%s'\n",
+ _mesa_shader_stage_to_string(producer_stage),
+ output->name,
+ output->type->name,
+ _mesa_shader_stage_to_string(consumer_stage),
+ input->type->name);
+ }
+ } else if (!output->type->is_array() || !is_gl_identifier(output->name)) {
+ /* There is a bit of a special case for gl_TexCoord. This
+ * built-in is unsized by default. Applications that variable
+ * access it must redeclare it with a size. There is some
+ * language in the GLSL spec that implies the fragment shader
+ * and vertex shader do not have to agree on this size. Other
+ * driver behave this way, and one or two applications seem to
+ * rely on it.
+ *
+ * Neither declaration needs to be modified here because the array
+ * sizes are fixed later when update_array_sizes is called.
+ *
+ * From page 48 (page 54 of the PDF) of the GLSL 1.10 spec:
+ *
+ * "Unlike user-defined varying variables, the built-in
+ * varying variables don't have a strict one-to-one
+ * correspondence between the vertex language and the
+ * fragment language."
+ */
linker_error(prog,
"%s shader output `%s' declared as type `%s', "
"but %s shader input declared as type `%s'\n",
diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index ca5368aa53..c0d72f8fa2 100644
--- a/src/compiler/glsl_types.cpp
+++ b/src/compiler/glsl_types.cpp
@@ -883,7 +883,8 @@ glsl_type::get_array_instance(const glsl_type *base, unsigned array_size)
bool
-glsl_type::record_compare(const glsl_type *b, bool match_locations) const
+glsl_type::record_compare(const glsl_type *b, bool match_name,
+ bool match_locations) const
{
if (this->length != b->length)
return false;
@@ -900,9 +901,16 @@ glsl_type::record_compare(const glsl_type *b, bool match_locations) const
* type definitions, and field names to be considered the same type."
*
* GLSL ES behaves the same (Ver 1.00 Sec 4.2.4, Ver 3.00 Sec 4.2.5).
+ *
+ * Section 7.4.1 (Shader Interface Matching) of the OpenGL 4.30 spec says:
+ *
+ * "Variables or block members declared as structures are considered
+ * to match in type if and only if structure members match in name,
+ * type, qualification, and declaration order."
*/
- if (strcmp(this->name, b->name) != 0)
- return false;
+ if (match_name)
+ if (strcmp(this->name, b->name) != 0)
+ return false;
for (unsigned i = 0; i < this->length; i++) {
if (this->fields.structure[i].type != b->fields.structure[i].type)
@@ -973,7 +981,8 @@ glsl_type::record_key_compare(const void *a, const void *b)
const glsl_type *const key1 = (glsl_type *) a;
const glsl_type *const key2 = (glsl_type *) b;
- return strcmp(key1->name, key2->name) == 0 && key1->record_compare(key2);
+ return strcmp(key1->name, key2->name) == 0 &&
+ key1->record_compare(key2, true);
}
diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h
index d32b580acc..05de92e36f 100644
--- a/src/compiler/glsl_types.h
+++ b/src/compiler/glsl_types.h
@@ -822,11 +822,15 @@ public:
/**
* Compare a record type against another record type.
*
- * This is useful for matching record types declared across shader stages.
+ * This is useful for matching record types declared on the same shader
+ * stage as well as across different shader stages.
+ * The option to not match name is needed for matching record types
+ * declared across different shader stages.
* The option to not match locations is to deal with places where the
* same struct is defined in a block which has a location set on it.
*/
- bool record_compare(const glsl_type *b, bool match_locations = true) const;
+ bool record_compare(const glsl_type *b, bool match_name,
+ bool match_locations = true) const;
/**
* Get the type interface packing.
--
2.18.0
More information about the mesa-dev
mailing list