[Mesa-dev] [PATCH 03/19] glsl: Track the linearized array index for each UBO instance array element

[Ian Romanick]

On 12/18/2016 09:54 PM, Timothy Arceri wrote:
> On Thu, 2016-12-15 at 20:10 -0800, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Cc: mesa-stable at lists.freedesktop.org
>> ---
>>  src/compiler/glsl/link_uniform_blocks.cpp | 17 ++++++++++++++---
>>  src/mesa/main/mtypes.h                    | 15 +++++++++++++++
>>  2 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/compiler/glsl/link_uniform_blocks.cpp
>> b/src/compiler/glsl/link_uniform_blocks.cpp
>> index 41b26e7..9adfbd5 100644
>> --- a/src/compiler/glsl/link_uniform_blocks.cpp
>> +++ b/src/compiler/glsl/link_uniform_blocks.cpp
>> @@ -209,13 +209,19 @@ static void process_block_array_leaf(char
>> **name, gl_uniform_block *blocks,
>>                                       struct gl_context *ctx,
>>                                       struct gl_shader_program
>> *prog);
>>  
>> +/**
>> + *
>> + * \param first_index Value of \c block_index for the first element
>> of the
>> + *                    array.
>> + */
>>  static void
>>  process_block_array(struct uniform_block_array_elements *ub_array,
>> char **name,
>>                      size_t name_length, gl_uniform_block *blocks,
>>                      ubo_visitor *parcel, gl_uniform_buffer_variable
>> *variables,
>>                      const struct link_uniform_block_active *const b,
>>                      unsigned *block_index, unsigned *binding_offset,
>> -                    struct gl_context *ctx, struct gl_shader_program
>> *prog)
>> +                    struct gl_context *ctx, struct gl_shader_program
>> *prog,
>> +                    unsigned first_index)
>>  {
>>     for (unsigned j = 0; j < ub_array->num_array_elements; j++) {
>>        size_t new_length = name_length;
>> @@ -227,11 +233,15 @@ process_block_array(struct
>> uniform_block_array_elements *ub_array, char **name,
>>        if (ub_array->array) {
>>           process_block_array(ub_array->array, name, new_length,
>> blocks,
>>                               parcel, variables, b, block_index,
>> -                             binding_offset, ctx, prog);
>> +                             binding_offset, ctx, prog,
>> first_index);
>>        } else {
>> +         const unsigned i = *block_index;
>> +
>>           process_block_array_leaf(name, blocks,
>>                                    parcel, variables, b, block_index,
>>                                    binding_offset, ctx, prog);
>> +
>> +         blocks[i].linearized_array_index = i - first_index;
> 
> Shouldn't this go in the new process_block_array_leaf() too?

It only needs to be one place.  *block_index is modified by
process_block_array_leaf, but the linearized index is the pre-modified
(that's a word... I'm sure of it!) value.  It could go in
process_block_array_leaf instead, but then I'd also need to pass either
first_index or (i - first_index).

I originally had a patch that removed the open-coded copy of
process_block_array_leaf which does not need to set
linearized_array_index.  I dropped that patch because there was a subtle
difference between the two implementations.  I will probably revisit the
idea, however.

> Otherwise this patch is:
> 
> Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>
> 
> 
>>        }
>>     }
>>  }
>> @@ -359,7 +369,8 @@ create_buffer_blocks(void *mem_ctx, struct
>> gl_context *ctx,
>>  
>>              assert(b->has_instance_name);
>>              process_block_array(b->array, &name, name_length,
>> blocks, &parcel,
>> -                                variables, b, &i, &binding_offset,
>> ctx, prog);
>> +                                variables, b, &i, &binding_offset,
>> ctx, prog,
>> +                                i);
>>              ralloc_free(name);
>>           } else {
>>              blocks[i].Name = ralloc_strdup(blocks, block_type-
>>> name);
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 36d48e2..ac4cac0 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -2493,6 +2493,21 @@ struct gl_uniform_block
>>     uint8_t stageref;
>>  
>>     /**
>> +    * Linearized array index for uniform block instance arrays
>> +    *
>> +    * Given a uniform block instance array declared with size
>> +    * blk[s_0][s_1]..[s_m], the block referenced by
>> blk[i_0][i_1]..[i_m] will
>> +    * have the linearized array index
>> +    *
>> +    *           m-1       m
>> +    *     i_m + ∑   i_j * ∏     s_k
>> +    *           j=0       k=j+1
>> +    *
>> +    * For a uniform block instance that is not an array, this is
>> always 0.
>> +    */
>> +   uint8_t linearized_array_index;
>> +
>> +   /**
>>      * Layout specified in the shader
>>      *
>>      * This isn't accessible through the API, but it is used while