Perhaps I'm blind, but I don't see where that array var is used beyond just being incremented. Mind pointing it out? <div class="gmail_quote">On Mar 21, 2015 11:31 PM, "Timothy Arceri" <<a href="mailto:t_arceri@yahoo.com.au">t_arceri@yahoo.com.au</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Sat, 2015-03-21 at 19:46 -0400, Ilia Mirkin wrote: > On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri <<a href="mailto:t_arceri@yahoo.com.au">t_arceri@yahoo.com.au</a>> wrote: > > --- > > src/glsl/link_uniform_initializers.cpp | 51 ++++++++++++++++++++++++---------- > > src/glsl/link_uniforms.cpp | 28 +++++++++++-------- > > 2 files changed, 52 insertions(+), 27 deletions(-) > > > > diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp > > index 6907384..7817314 100644 > > --- a/src/glsl/link_uniform_initializers.cpp > > +++ b/src/glsl/link_uniform_initializers.cpp > > @@ -100,6 +100,38 @@ copy_constant_to_storage(union gl_constant_value *storage, > > } > > > > void > > +copy_constant_array_to_storage(struct gl_uniform_storage *const storage, > > + const ir_constant *val, > > + unsigned int *idx, > > + unsigned int *array_elements, > > + unsigned int boolean_true) > > +{ > > + if (val->type->fields.array->is_array()) { > > + for (unsigned int i = 0; i < val->type->length; i++) { > > + copy_constant_array_to_storage(storage, val->array_elements[i], > > + idx, array_elements, boolean_true); > > + } > > + } else { > > + const enum glsl_base_type base_type = > > + val->array_elements[0]->type->base_type; > > + const unsigned int elements = val->array_elements[0]->type->components(); > > + unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; > > + unsigned int length = MIN2(val->type->length, > > + (storage->array_elements - *array_elements)); > > + > > + for (unsigned int i = 0; i < length; i++) { > > + copy_constant_to_storage(& storage->storage[*idx], > > + val->array_elements[i], > > + base_type, > > + elements, > > + boolean_true); > > + *idx += elements * dmul; > > + *array_elements = *array_elements + 1; > > + } > > + } > > +} > > + > > +void > > set_sampler_binding(gl_shader_program *prog, const char *name, int binding) > > { > > struct gl_uniform_storage *const storage = > > @@ -178,7 +210,7 @@ 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 (type->without_array()->is_record()) { > > That's not what the old code looked for... it looked for array && > array-of-record. You changed it to record ||-array-of-record || > array-of-array-of-record || ... . You've done this several times > throughout this change. Are all of these changes safe? Yeah it's safe. In each case you will see that the non array version would be found in a preceding if statement so all we are really checking for is array-of-record || array-of-array-of-record || ... > > > const glsl_type *const element_type = type->fields.array; > > > > for (unsigned int i = 0; i < type->length; i++) { > > @@ -201,22 +233,11 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, > > } > > > > if (val->type->is_array()) { > > - const enum glsl_base_type base_type = > > - val->array_elements[0]->type->base_type; > > - const unsigned int elements = val->array_elements[0]->type->components(); > > unsigned int idx = 0; > > - unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; > > + unsigned int array_elements = 0; > > What is this used for? I don't see it used here or in the function (as > temp shared storage). It's used in the function to calculate the amount of room left in storage so we don't overflow if the array is to big. Maybe it should be renamed to something better storage_count? used_storage_elements? total_array_elements? Or maybe I could just add a comment to the function: /* Used to calculate the space left in storage so we don't * overflow if the array is to big. */ > > > > > - assert(val->type->length >= storage->array_elements); > > - for (unsigned int i = 0; i < storage->array_elements; i++) { > > - copy_constant_to_storage(& storage->storage[idx], > > - val->array_elements[i], > > - base_type, > > - elements, > > - boolean_true); > > - > > - idx += elements * dmul; > > - } > > + copy_constant_array_to_storage(storage, val, &idx, > > + &array_elements, boolean_true); > > } else { > > copy_constant_to_storage(storage->storage, > > val, > > diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp > > index 799c74b..654a1ca 100644 > > --- a/src/glsl/link_uniforms.cpp > > +++ b/src/glsl/link_uniforms.cpp > > @@ -49,8 +49,8 @@ values_for_type(const glsl_type *type) > > { > > if (type->is_sampler()) { > > return 1; > > - } else if (type->is_array() && type->fields.array->is_sampler()) { > > - return type->array_size(); > > + } else if (type->is_array()) { > > + return type->array_size() * values_for_type(type->fields.array); > > } else { > > return type->component_slots(); > > } > > @@ -71,6 +71,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; > > > > @@ -136,7 +137,7 @@ program_resource_visitor::process(ir_variable *var) > > */ > > recursion(var->type, &name, strlen(name), row_major, NULL, false); > > ralloc_free(name); > > - } else if (t->without_array()->is_record()) { > > + } else if (t_without_array->is_record()) { > > char *name = ralloc_strdup(NULL, var->name); > > recursion(var->type, &name, strlen(name), row_major, NULL, false); > > ralloc_free(name); > > @@ -144,8 +145,8 @@ program_resource_visitor::process(ir_variable *var) > > 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 { > > @@ -216,8 +217,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; > > > > @@ -574,8 +575,12 @@ private: > > > > const glsl_type *base_type; > > if (type->is_array()) { > > - this->uniforms[id].array_elements = type->length; > > - base_type = type->fields.array; > > + this->uniforms[id].array_elements = type->length; > > + base_type = type->fields.array; > > + while (base_type->is_array()) { > > + this->uniforms[id].array_elements *= base_type->length; > > + base_type = base_type->fields.array; > > + } > > } else { > > this->uniforms[id].array_elements = 0; > > base_type = type; > > @@ -629,7 +634,7 @@ private: > > > > if (type->is_array()) { > > 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; > > } > > @@ -768,8 +773,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 = '['; > > } > > > > -- > > 2.1.0 > > > > _______________________________________________ > > mesa-dev mailing list > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div>