[Mesa-dev] [PATCH 03/25] glsl: clean-up link uniform code

[Ian Romanick]

On 08/20/2015 10:31 AM, Ian Romanick wrote:
> On 08/19/2015 09:37 PM, Timothy Arceri wrote:
>> These changes are also needed to allow linking of
>> struct and interface arrays of arrays.
>> ---
>>  src/glsl/link_uniforms.cpp | 16 ++++++----------
>>  1 file changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
>> index 254086d..42f75e9 100644
>> --- a/src/glsl/link_uniforms.cpp
>> +++ b/src/glsl/link_uniforms.cpp
>> @@ -72,6 +72,7 @@ void
>>  program_resource_visitor::process(ir_variable *var)
>>  {
>>     const glsl_type *t = var->type;
>> +   const glsl_type *t_without_array = var->type->without_array();
>>     const bool row_major =
>>        var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
>>  
>> @@ -141,12 +142,8 @@ program_resource_visitor::process(ir_variable *var)
>>        char *name = ralloc_strdup(NULL, var->name);
>>        recursion(var->type, &name, strlen(name), row_major, NULL, false);
>>        ralloc_free(name);
>> -   } else if (t->is_interface()) {
>> -      char *name = ralloc_strdup(NULL, var->type->name);
>> -      recursion(var->type, &name, strlen(name), row_major, NULL, false);
>> -      ralloc_free(name);
>> -   } else if (t->is_array() && t->fields.array->is_interface()) {
>> -      char *name = ralloc_strdup(NULL, var->type->fields.array->name);
>> +   } else if (t_without_array->is_interface()) {
>> +      char *name = ralloc_strdup(NULL, t_without_array->name);
>>        recursion(var->type, &name, strlen(name), row_major, NULL, false);
>>        ralloc_free(name);
>>     } else {
>> @@ -217,8 +214,8 @@ program_resource_visitor::recursion(const glsl_type *t, char **name,
>>           (*name)[name_length] = '\0';
>>           this->leave_record(t, *name, row_major);
>>        }
>> -   } else if (t->is_array() && (t->fields.array->is_record()
>> -                                || t->fields.array->is_interface())) {
>> +   } else if (t->without_array()->is_record()
>> +              || t->without_array()->is_interface()) {
>>        if (record_type == NULL && t->fields.array->is_record())
>>           record_type = t->fields.array;
> 
> Is the part after the change still correct?  If t is an array of array
> of struct, record_type will still point at NULL.  I guess this might be
> addressed in a later patch...

It looks like it's somewhat addressed in the next patch.  It's not clear
why this is better than just adding a "|| t->fields.array->is_array()"
clause to the right side of the &&.  Is that just the way it turned out
(which is fine), or am I missing something subtle (which should probably
be documented)?

>> @@ -810,8 +807,7 @@ link_update_uniform_buffer_variables(struct gl_shader *shader)
>>  
>>        if (var->type->is_record()) {
>>           sentinel = '.';
>> -      } else if (var->type->is_array()
>> -                 && var->type->fields.array->is_record()) {
>> +      } else if (var->type->without_array()->is_record()) {
>>           sentinel = '[';
>>        }
>>  
>>
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>