[Mesa-dev] [RFC PATCH 07/12] glsl: Add support for linking uniform arrays of arrays
Ilia Mirkin
imirkin at alum.mit.edu
Sat Mar 21 20:35:23 PDT 2015
Perhaps I'm blind, but I don't see where that array var is used beyond just
being incremented. Mind pointing it out?
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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150321/534cf5e1/attachment-0001.html>
More information about the mesa-dev
mailing list