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 <[email protected]>
Reviewed-by: Anuj Phogat <[email protected]>
Reviewed-by: Timothy Arceri <[email protected]>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108250

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.