Mesa (master): glsl: Fix input/output structure matching across shader stages
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Fri Apr 5 00:08:54 UTC 2019
Module: Mesa
Branch: master
Commit: a7d40a13ec39df5c1e18c2afaf70988958c11933
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=a7d40a13ec39df5c1e18c2afaf70988958c11933
Author: Sergii Romantsov <sergii.romantsov at globallogic.com>
Date: Thu Jan 24 14:33:55 2019 +0200
glsl: Fix input/output structure matching across shader stages
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
v2: rebased against master and small fixes
Signed-off-by: Vadym Shovkoplias <vadym.shovkoplias at globallogic.com>
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>
Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108250
---
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 2a71a13d545..7d927dc4b80 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -7594,7 +7594,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 352cd7e522b..b8534d83076 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_struct()) {
+ /* 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 90517a5b52f..66241b34281 100644
--- a/src/compiler/glsl_types.cpp
+++ b/src/compiler/glsl_types.cpp
@@ -1008,7 +1008,8 @@ glsl_type::get_array_instance(const glsl_type *base,
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;
@@ -1025,9 +1026,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)
@@ -1098,7 +1106,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 1a774adb64e..dd9ae657019 100644
--- a/src/compiler/glsl_types.h
+++ b/src/compiler/glsl_types.h
@@ -934,11 +934,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.
More information about the mesa-commit
mailing list