[Mesa-dev] [RFC PATCH 07/12] glsl: Add support for linking uniform arrays of arrays

Timothy Arceri t_arceri at yahoo.com.au
Sat Mar 21 20:31:25 PDT 2015


On Sat, 2015-03-21 at 19:46 -0400, Ilia Mirkin wrote:
> On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri <t_arceri at yahoo.com.au> 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
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev




More information about the mesa-dev mailing list