glsl: Fix program interface queries relating to interface blocks.

This fixes 555 dEQP tests (using the nougat-cts-dev branch), Piglit's
arb_program_interface_query/arb_program_interface_query-resource-query,
and GL45-CTS.program_interface_query.separate-programs-{tess-control,
tess-eval,geometry}.  Only one dEQP program interface failure remains.

I would have liked to split this up into several distinct changes, but
I wasn't sure how to do that given thet tangled nature of these issues.

So, the issues:

   * We need to treat interface blocks declared as an array of instances
     as a single block - removing the outer array.  The resource list
     entry's name should not include the array length.  Properties such
     as GL_ARRAY_SIZE should refer to the variable inside the block, not
     the interface block's array properties.

   * We need to do this prefixing even for structure variables.

   * We need to do this for built-ins (such as gl_PerVertex.gl_Position).

   * After interface array unwrapping, any variable which is an array
     should have [0] appended.  It doesn't matter if it's a TCS/TES/GS
     input or TCS output - that looked like an attempt to unwrap for
     per-vertex variables, but that didn't consider per-patch variables,
     and as far as I can tell there's nothing to justify this.

Several Mesa developers have suggested that Issue 16 contradicts the
main specification, but I believe that it doesn't - the main spec just
isn't terribly clear.  The main ARB_program_interface query spec says:

  "* For an active interface block not declared as an array of block
     instances, a single entry will be generated, using the block name from
     the shader source.

   * For an active interface block declared as an array of instances,
     separate entries will be generated for each active instance.  The name
     of the instance is formed by concatenating the block name, the "["
     character, an integer identifying the instance number, and the "]"
     character."

Issue 16 says that built-ins should be named "gl_PerVertex.gl_Position",
but several people suggested the second bullet above means that it
should be named "gl_PerVertex[array length].gl_Position".

There are two important things to note.  Those bullet points say
"an active interface block", while the others say "variable" or "active
shader storage block member".  They also don't mention applying the
rules recursively (unlike the other bullets).  Both suggest that
these rules apply to blocks themselves, not members of blocks.

In fact, for GL_UNIFORM_BLOCK queries, we do have "block[0]",
"block[1]", ... resource list entries - so those rules are real,
and actually used.  So if they don't apply to block members, then how
should members be named?  Unfortunately, I don't see any rules outside
of issue 16 - where the rationale is very unclear.  I hope to clarify
the spec in the future.

Signed-off-by: Kenneth Graunke <[email protected]>
Reviewed-by: Alejandro Piñeiro <[email protected]>
Reviewed-by: Ian Romanick <[email protected]>

1 files changed, 19 insertions, 73 deletions

diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index e4a8773e09c..1d1616b7eaa 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -686,31 +686,14 @@ _mesa_program_resource_find_index(struct gl_shader_program *shProg,
  * ambiguous in this regard.  However, either name can later be passed
  * to glGetUniformLocation (and related APIs), so there shouldn't be any
  * harm in always appending "[0]" to uniform array names.
- *
- * Geometry shader stage has different naming convention where the 'normal'
- * condition is an array, therefore for variables referenced in geometry
- * stage we do not add '[0]'.
- *
- * Note, that TCS outputs and TES inputs should not have index appended
- * either.
  */
 static bool
 add_index_to_name(struct gl_program_resource *res)
 {
-   bool add_index = !((res->Type == GL_PROGRAM_INPUT &&
-                       res->StageReferences & (1 << MESA_SHADER_GEOMETRY |
-                                               1 << MESA_SHADER_TESS_CTRL |
-                                               1 << MESA_SHADER_TESS_EVAL)) ||
-                      (res->Type == GL_PROGRAM_OUTPUT &&
-                       res->StageReferences & 1 << MESA_SHADER_TESS_CTRL));
-
    /* Transform feedback varyings have array index already appended
     * in their names.
     */
-   if (res->Type == GL_TRANSFORM_FEEDBACK_VARYING)
-      add_index = false;
-
-   return add_index;
+   return res->Type != GL_TRANSFORM_FEEDBACK_VARYING;
 }
 
 /* Get name length of a program resource. This consists of
@@ -1401,9 +1384,6 @@ validate_io(struct gl_shader_program *producer,
 
    bool valid = true;
 
-   void *name_buffer = NULL;
-   size_t name_buffer_size = 0;
-
    gl_shader_variable const **outputs =
       (gl_shader_variable const **) calloc(producer->NumProgramResourceList,
                                            sizeof(gl_shader_variable *));
@@ -1475,52 +1455,11 @@ validate_io(struct gl_shader_program *producer,
             }
          }
       } else {
-         char *consumer_name = consumer_var->name;
-
-         if (nonarray_stage_to_array_stage &&
-             consumer_var->interface_type != NULL &&
-             consumer_var->interface_type->is_array() &&
-             !is_gl_identifier(consumer_var->name)) {
-            const size_t name_len = strlen(consumer_var->name);
-
-            if (name_len >= name_buffer_size) {
-               free(name_buffer);
-
-               name_buffer_size = name_len + 1;
-               name_buffer = malloc(name_buffer_size);
-               if (name_buffer == NULL) {
-                  valid = false;
-                  goto out;
-               }
-            }
-
-            consumer_name = (char *) name_buffer;
-
-            char *s = strchr(consumer_var->name, '[');
-            if (s == NULL) {
-               valid = false;
-               goto out;
-            }
-
-            char *t = strchr(s, ']');
-            if (t == NULL) {
-               valid = false;
-               goto out;
-            }
-
-            assert(t[1] == '.' || t[1] == '[');
-
-            const ptrdiff_t base_name_len = s - consumer_var->name;
-
-            memcpy(consumer_name, consumer_var->name, base_name_len);
-            strcpy(consumer_name + base_name_len, t + 1);
-         }
-
          for (unsigned j = 0; j < num_outputs; j++) {
             const gl_shader_variable *const var = outputs[j];
 
             if (!var->explicit_location &&
-                strcmp(consumer_name, var->name) == 0) {
+                strcmp(consumer_var->name, var->name) == 0) {
                producer_var = var;
                match_index = j;
                break;
@@ -1583,21 +1522,29 @@ validate_io(struct gl_shader_program *producer,
        * find the producer variable that goes with the consumer variable.
        */
       if (nonarray_stage_to_array_stage) {
-         if (!consumer_var->type->is_array() ||
-             consumer_var->type->fields.array != producer_var->type) {
-            valid = false;
-            goto out;
-         }
-
          if (consumer_var->interface_type != NULL) {
+            /* the interface is the array; underlying types should match */
+            if (producer_var->type != consumer_var->type) {
+               valid = false;
+               goto out;
+            }
+
             if (!consumer_var->interface_type->is_array() ||
                 consumer_var->interface_type->fields.array != producer_var->interface_type) {
                valid = false;
                goto out;
             }
-         } else if (producer_var->interface_type != NULL) {
-            valid = false;
-            goto out;
+         } else {
+            if (producer_var->interface_type != NULL) {
+               valid = false;
+               goto out;
+            }
+
+            if (!consumer_var->type->is_array() ||
+                consumer_var->type->fields.array != producer_var->type) {
+               valid = false;
+               goto out;
+            }
          }
       } else {
          if (producer_var->type != consumer_var->type) {
@@ -1628,7 +1575,6 @@ validate_io(struct gl_shader_program *producer,
    }
 
  out:
-   free(name_buffer);
    free(outputs);
    return valid && num_outputs == 0;
 }