[Mesa-dev] [PATCH 07/17] glsl: Mark as active all elements of shared/std140 block arrays

[Samuel Iglesias Gonsalvez]

From: Antia Puentes <apuentes at igalia.com>

Commit 1ca25ab (glsl: Do not eliminate 'shared' or 'std140' blocks
or block members) considered as active 'shared' and 'std140' uniform
blocks and uniform block arrays, but did not include the block array
elements. Because of that, it was possible to have an active uniform
block array without any elements marked as used, making the assertion
   ((b->num_array_elements > 0) == b->type->is_array())
in link_uniform_blocks() fail.

Fixes the following 5 dEQP tests:

 * dEQP-GLES3.functional.ubo.random.nested_structs_instance_arrays.18
 * dEQP-GLES3.functional.ubo.random.nested_structs_instance_arrays.24
 * dEQP-GLES3.functional.ubo.random.nested_structs_arrays_instance_arrays.19
 * dEQP-GLES3.functional.ubo.random.all_per_block_buffers.49
 * dEQP-GLES3.functional.ubo.random.all_shared_buffer.36

Fixes bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83508
---

Section 2.11.6 (Uniform Variables) of the OpenGL ES 3.0.3 spec says:
    
"All members of a named uniform block declared with a shared or
std140 layout qualifier are considered active, even if they are not
referenced in any shader in the program. The uniform block itself is
also considered active, even if no member of the block is
referenced."

Although this behaviour is not explicitly defined for OpenGL, I have not
added _mesa_is_gles3() checks in the patch, because first, commit
"1ca25ab" marks the 'shared' or 'std140' blocks as active and avoids
eliminating them regardless of the OpenGL used, and second, to introduce
those checks makes Piglit test: "bin/shader_runner
tests/spec/arb_uniform_buffer_object/example_from_spec.shader_test"
fail, which has a comment in its shader that suggests that we expect the
same behaviour in GLES 3.0 and OpenGL:

/* std140 (or shared) layout prevents any fields or blocks from being
 * eliminted.  Section 2.11.6 of the OpenGL ES 3.0 spec makes this
 * explicit, but desktop GL specs only say it implicitly.  Either way,
 * there is no need to reference any field of the std140 block.
 */

 src/glsl/link_uniform_block_active_visitor.cpp | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/glsl/link_uniform_block_active_visitor.cpp b/src/glsl/link_uniform_block_active_visitor.cpp
index 5102947..fbe79de 100644
--- a/src/glsl/link_uniform_block_active_visitor.cpp
+++ b/src/glsl/link_uniform_block_active_visitor.cpp
@@ -106,6 +106,22 @@ link_uniform_block_active_visitor::visit(ir_variable *var)
    assert(b->num_array_elements == 0);
    assert(b->array_elements == NULL);
    assert(b->type != NULL);
+   assert(!b->type->is_array() || b->has_instance_name);
+
+   /* For uniform block arrays declared with a shared or std140 layout
+    * qualifier, mark all its instances as used.
+    */
+   if (b->type->is_array() && b->type->length > 0) {
+      b->num_array_elements = b->type->length;
+      b->array_elements = reralloc(this->mem_ctx,
+                                   b->array_elements,
+                                   unsigned,
+                                   b->num_array_elements);
+
+      for (unsigned i = 0; i < b->num_array_elements; i++) {
+         b->array_elements[i] = i;
+      }
+   }
 
    return visit_continue;
 }
@@ -147,6 +163,13 @@ link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir)
    assert((b->num_array_elements == 0) == (b->array_elements == NULL));
    assert(b->type != NULL);
 
+   /* If the block array was declared with a shared or
+    * std140 layout qualifier, all its instances have been already marked
+    * as used in link_uniform_block_active_visitor::visit(ir_variable *).
+    */
+   if (var->type->interface_packing != GLSL_INTERFACE_PACKING_PACKED)
+      return visit_continue_with_parent;
+
    ir_constant *c = ir->array_index->as_constant();
 
    if (c) {
-- 
2.4.3