[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 22:02:45 PDT 2015


On Sat, 2015-03-21 at 23:35 -0400, Ilia Mirkin wrote:
> Perhaps I'm blind, but I don't see where that array var is used beyond
> just being incremented. Mind pointing it out?

No problem.

unsigned int length = MIN2(val->type->length, (storage->array_elements -
*array_elements));
> 
> On Mar 21, 2015 11:31 PM, "Timothy Arceri" <t_arceri at yahoo.com.au>
> wrote:
>         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