[Mesa-dev] [PATCH V6 01/27] glsl: Add support for linking uniform arrays of arrays

[Ian Romanick]

On 09/28/2015 07:42 PM, Timothy Arceri wrote:
> V3: Fix setting of data.location for struct AoA UBO members
> 
> V2: Handle arrays of arrays in the same way structures are handled
> 
> The ARB_arrays_of_arrays spec doesn't give very many details on how
> AoA uniforms are intended to be implemented. However in the
> ARB_program_interface_query spec there are details that show AoA are
> intended to be handled in a similar way to structs.
> 
> Issues 7 from the ARB_program_interface_query spec:
> 
>  We define rules consistent with our enumeration rules for
>  other complex types.  For existing one-dimensional arrays, we enumerate
>  a single entry if the array is an array of basic types, or separate
>  entries for each array element if the array is an array of structures.
>  We follow similar rules here.  For a uniform array such as:
> 
>    uniform vec4 a[5][4][3];
> 
>  we enumerate twenty different entries ("a[0][0][0]" through
>  "a[4][3][0]"), each of which is treated as an array with three elements.
>  This is morally equivalent to what you'd get if you worked around the
>  limitation in current GLSL via:
> 
>     struct ArrayBottom { vec4 c[3]; };
>     struct ArrayMid    { ArrayBottom b[3]; };
>     uniform ArrayMid   a[5];
> 
>  which would enumerate "a[0].b[0].c[0]" through "a[4].b[3].c[0]".
> 
> Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>  src/glsl/link_uniform_initializers.cpp |  4 +++-
>  src/glsl/link_uniforms.cpp             | 16 +++++++++++-----
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp
> index 05000fc..f64ba1b 100644
> --- a/src/glsl/link_uniform_initializers.cpp
> +++ b/src/glsl/link_uniform_initializers.cpp
> @@ -179,6 +179,7 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog,
>  			const char *name, const glsl_type *type,
>                          ir_constant *val, unsigned int boolean_true)
>  {
> +   const glsl_type *t_without_array = type->without_array();
>     if (type->is_record()) {
>        ir_constant *field_constant;
>  
> @@ -193,7 +194,8 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog,
>  	 field_constant = (ir_constant *)field_constant->next;
>        }
>        return;
> -   } else if (type->is_array() && type->fields.array->is_record()) {
> +   } else if (t_without_array->is_record() ||
> +              (type->is_array() && type->fields.array->is_array())) {

I think a good follow-up patch would be to add an is_array_of_array
predicate that encapsulates this check... I'm not going to make you do a
v7 just for that. :)

This patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

>        const glsl_type *const element_type = type->fields.array;
>  
>        for (unsigned int i = 0; i < type->length; i++) {
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> index 94d7287..f3508a3 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -149,7 +149,8 @@ program_resource_visitor::process(ir_variable *var)
>        recursion(var->type, &name, strlen(name), row_major, NULL, packing,
>                  false, record_array_count);
>        ralloc_free(name);
> -   } else if (t->without_array()->is_record()) {
> +   } else if (t_without_array->is_record() ||
> +              (t->is_array() && t->fields.array->is_array())) {
>        char *name = ralloc_strdup(NULL, var->name);
>        recursion(var->type, &name, strlen(name), row_major, NULL, packing,
>                  false, record_array_count);
> @@ -231,7 +232,8 @@ program_resource_visitor::recursion(const glsl_type *t, char **name,
>           this->leave_record(t, *name, row_major, packing);
>        }
>     } else if (t->without_array()->is_record() ||
> -              t->without_array()->is_interface()) {
> +              t->without_array()->is_interface() ||
> +              (t->is_array() && t->fields.array->is_array())) {
>        if (record_type == NULL && t->fields.array->is_record())
>           record_type = t->fields.array;
>  
> @@ -387,6 +389,7 @@ private:
>     {
>        assert(!type->without_array()->is_record());
>        assert(!type->without_array()->is_interface());
> +      assert(!(type->is_array() && type->fields.array->is_array()));
>  
>        (void) row_major;
>  
> @@ -721,6 +724,7 @@ private:
>     {
>        assert(!type->without_array()->is_record());
>        assert(!type->without_array()->is_interface());
> +      assert(!(type->is_array() && type->fields.array->is_array()));
>  
>        unsigned id;
>        bool found = this->map->get(id, name);
> @@ -809,10 +813,11 @@ private:
>           if (type->is_array()) {
>              if (packing == GLSL_INTERFACE_PACKING_STD430)
>                 this->uniforms[id].array_stride =
> -                  type->fields.array->std430_array_stride(row_major);
> +                  type->without_array()->std430_array_stride(row_major);
>              else
>                 this->uniforms[id].array_stride =
> -                  glsl_align(type->fields.array->std140_size(row_major), 16);
> +                  glsl_align(type->without_array()->std140_size(row_major),
> +                             16);
>  	 } else {
>  	    this->uniforms[id].array_stride = 0;
>  	 }
> @@ -970,7 +975,8 @@ link_update_uniform_buffer_variables(struct gl_shader *shader)
>  
>        if (var->type->is_record()) {
>           sentinel = '.';
> -      } else if (var->type->without_array()->is_record()) {
> +      } else if (var->type->is_array() && (var->type->fields.array->is_array()
> +                 || var->type->without_array()->is_record())) {
>           sentinel = '[';
>        }
>  
>