[Mesa-dev] [PATCH v2 19/28] nir/linker: use only the array element type for array of ssbo/ubo

[Alejandro Piñeiro]

For this interfaces, the inner members are added only once as uniforms
or resources, in opposite to other cases, like a uniform array of
structs.

For those guessing why a issue (16) from ARB_program_interface_query
was used, instead of a quote of the core spec: The core spec is not
really clear about how members of arrays of blocks should be
enumerated.

On GLSL this was also problematic, specially when we were trying to
pass the 4.5 CTS tests. See commit "glsl: Fix program interface
queries relating to interface blocks"
(4c4d9e4f032d5753034361ee70aa88d16d3a04b4), as a reference. That one
also needed to rely on issue (16) to justify the change, pointing that
the core spec needs to be clarified.
---

As mentioned on the commit message, I needed to quote a issue of a
specific extension spec, instead of the core spec. Quoting from the
commit "glsl: Fix program interface queries relating to interface
blocks", mentioned on the commit message:

    "
    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."

That "clarify the spec in the future" didn't happen. Rules for member
of arrays of blocks are not clear with the core spec. This/next week I
plan to create a spec issue in order to try to clarify this, even if
the solution is just copy what issue 16 says on the core spec. At that
point I could send a new commit in order to replace the spec quote.


 src/compiler/glsl/gl_nir_link_uniforms.c | 44 ++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/gl_nir_link_uniforms.c b/src/compiler/glsl/gl_nir_link_uniforms.c
index 00995fb3f76..ac445c8560a 100644
--- a/src/compiler/glsl/gl_nir_link_uniforms.c
+++ b/src/compiler/glsl/gl_nir_link_uniforms.c
@@ -498,11 +498,51 @@ gl_nir_link_uniforms(struct gl_context *ctx,
 
          state.current_var = var;
 
+         /*
+          * From ARB_program_interface spec, issue (16):
+          *
+          * "RESOLVED: We will follow the default rule for enumerating block
+          *  members in the OpenGL API, which is:
+          *
+          *  * If a variable is a member of an interface block without an
+          *    instance name, it is enumerated using just the variable name.
+          *
+          *  * If a variable is a member of an interface block with an
+          *    instance name, it is enumerated as "BlockName.Member", where
+          *    "BlockName" is the name of the interface block (not the
+          *    instance name) and "Member" is the name of the variable.
+          *
+          * For example, in the following code:
+          *
+          * uniform Block1 {
+          *   int member1;
+          * };
+          * uniform Block2 {
+          *   int member2;
+          * } instance2;
+          * uniform Block3 {
+          *  int member3;
+          * } instance3[2];  // uses two separate buffer bindings
+          *
+          * the three uniforms (if active) are enumerated as "member1",
+          * "Block2.member2", and "Block3.member3"."
+          *
+          * Note that in the last example, with an array of ubo, only one
+          * uniform is generated. For that reason, while unrolling the
+          * uniforms of a ubo, or the variables of a ssbo, we need to treat
+          * arrays of instance as a single block.
+          */
+         const struct glsl_type *type = var->type;
+         if (nir_variable_is_in_block(var) &&
+             glsl_type_is_array(type)) {
+            type = glsl_without_array(type);
+         }
+
          struct type_tree_entry *type_tree =
-            build_type_tree_for_type(var->type);
+            build_type_tree_for_type(type);
          state.current_type = type_tree;
 
-         int res = nir_link_uniform(ctx, prog, sh->Program, shader_type, var->type,
+         int res = nir_link_uniform(ctx, prog, sh->Program, shader_type, type,
                                     location, &state);
 
          free_type_tree(type_tree);
-- 
2.14.1