[Mesa-dev] [PATCH V3 1/6] glsl: order indices for samplers inside a struct array
Timothy Arceri
t_arceri at yahoo.com.au
Mon Sep 7 15:09:55 PDT 2015
On Mon, 2015-09-07 at 11:24 -0700, Jason Ekstrand wrote:
> On Tue, Sep 1, 2015 at 7:44 PM, Timothy Arceri <t_arceri at yahoo.com.au>
> wrote:
> > This allows the correct offset to be easily calculated for indirect
> > indexing when a struct array contains multiple samplers, or any crazy
> > nesting.
> >
> > The indices for the folling struct will now look like this:
> > Sampler index: 0 Name: s[0].tex
> > Sampler index: 1 Name: s[1].tex
> > Sampler index: 2 Name: s[0].si.tex
> > Sampler index: 3 Name: s[1].si.tex
> > Sampler index: 4 Name: s[0].si.tex2
> > Sampler index: 5 Name: s[1].si.tex2
> >
> > Before this change it looked like this:
> > Sampler index: 0 Name: s[0].tex
> > Sampler index: 3 Name: s[1].tex
> > Sampler index: 1 Name: s[0].si.tex
> > Sampler index: 4 Name: s[1].si.tex
> > Sampler index: 2 Name: s[0].si.tex2
> > Sampler index: 5 Name: s[1].si.tex2
> >
> > struct S_inner {
> > sampler2D tex;
> > sampler2D tex2;
> > };
> >
> > struct S {
> > sampler2D tex;
> > S_inner si;
> > };
> >
> > uniform S s[2];
> >
> > V2: rename struct array counter to have better name
> > ---
> > src/glsl/link_uniforms.cpp | 112 ++++++++++++++++++++++++++++++++++++++--
> > -----
> > src/glsl/linker.h | 4 +-
> > 2 files changed, 98 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> > index 254086d..5402c99 100644
> > --- a/src/glsl/link_uniforms.cpp
> > +++ b/src/glsl/link_uniforms.cpp
> > @@ -28,6 +28,7 @@
> > #include "glsl_symbol_table.h"
> > #include "program/hash_table.h"
> > #include "program.h"
> > +#include "util/hash_table.h"
> >
> > /**
> > * \file link_uniforms.cpp
> > @@ -63,14 +64,17 @@ program_resource_visitor::process(const glsl_type
> > *type, const char *name)
> > assert(type->without_array()->is_record()
> > || type->without_array()->is_interface());
> >
> > + unsigned record_array_count = 1;
> > char *name_copy = ralloc_strdup(NULL, name);
> > - recursion(type, &name_copy, strlen(name), false, NULL, false);
> > + recursion(type, &name_copy, strlen(name), false, NULL, false,
> > + record_array_count);
> > ralloc_free(name_copy);
> > }
> >
> > void
> > program_resource_visitor::process(ir_variable *var)
> > {
> > + unsigned record_array_count = 1;
> > const glsl_type *t = var->type;
> > const bool row_major =
> > var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
> > @@ -111,7 +115,8 @@ program_resource_visitor::process(ir_variable *var)
> > * lowering is only applied to non-uniform interface blocks, so
> > we
> > * can safely pass false for row_major.
> > */
> > - recursion(var->type, &name, new_length, row_major, NULL, false);
> > + recursion(var->type, &name, new_length, row_major, NULL, false,
> > + record_array_count);
> > }
> > ralloc_free(name);
> > } else if (var->data.from_named_ifc_block_nonarray) {
> > @@ -135,19 +140,23 @@ program_resource_visitor::process(ir_variable *var)
> > * is only applied to non-uniform interface blocks, so we can
> > safely
> > * pass false for row_major.
> > */
> > - recursion(var->type, &name, strlen(name), row_major, NULL, false);
> > + recursion(var->type, &name, strlen(name), row_major, NULL, false,
> > + record_array_count);
> > ralloc_free(name);
> > } else if (t->without_array()->is_record()) {
> > char *name = ralloc_strdup(NULL, var->name);
> > - recursion(var->type, &name, strlen(name), row_major, NULL, false);
> > + recursion(var->type, &name, strlen(name), row_major, NULL, false,
> > + record_array_count);
> > 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);
> > + recursion(var->type, &name, strlen(name), row_major, NULL, false,
> > + record_array_count);
> > ralloc_free(name);
> > } else if (t->is_array() && t->fields.array->is_interface()) {
> > char *name = ralloc_strdup(NULL, var->type->fields.array->name);
> > - recursion(var->type, &name, strlen(name), row_major, NULL, false);
> > + recursion(var->type, &name, strlen(name), row_major, NULL, false,
> > + record_array_count);
> > ralloc_free(name);
> > } else {
> > this->visit_field(t, var->name, row_major, NULL, false);
> > @@ -158,7 +167,8 @@ void
> > program_resource_visitor::recursion(const glsl_type *t, char **name,
> > size_t name_length, bool row_major,
> > const glsl_type *record_type,
> > - bool last_field)
> > + bool last_field,
> > + unsigned record_array_count)
> > {
> > /* Records need to have each field processed individually.
> > *
> > @@ -205,7 +215,7 @@ program_resource_visitor::recursion(const glsl_type
> > *t, char **name,
> > recursion(t->fields.structure[i].type, name, new_length,
> > field_row_major,
> > record_type,
> > - (i + 1) == t->length);
> > + (i + 1) == t->length, record_array_count);
> >
> > /* Only the first leaf-field of the record gets called with the
> > * record type pointer.
> > @@ -222,6 +232,8 @@ program_resource_visitor::recursion(const glsl_type
> > *t, char **name,
> > if (record_type == NULL && t->fields.array->is_record())
> > record_type = t->fields.array;
> >
> > + record_array_count *= t->length;
>
> It's possible that I'm not understanding something, but I don't think
> this recursion does what you think it does. The only time you ever
> manipulate record_array_count is to multiply it by an array length so
> the final record_array_count will be the product of the lengths of all
> arrays encountered. If you have the following structure:
>
> struct S {
> sampler2D A[5];
> sampler2D B[3];
> sampler3D C[2];
> };
>
> Then, by the time you get done with B and on to C, the
> record_array_count will be 15, not 8 like is what I think you want.
> Also, nowhere in here do you predicate that on is_sampler() so if I
> replaced B with an array of floats, you'd still get 15 but I think you
> would want 5. Am I missing something?
Yes you are ;)
This happens within
if (t->without_array()->is_record() ||
t->without_array()->is_interface() ||
(t->is_array() && t->fields.array->is_array()))
So record_array_count (maybe could have a better name) only gets increased if
the struct is an array, or AoA or the member is an AoA.
In your example above record_array_count would remain at the default of 1.
For something a little more complex it would end up like this:
struct S [2] {
sampler2D A[5]; // record_array_count == 2
sampler2D B[2][3]; // record_array_count == 4
sampler3D C[2]; // record_array_count == 2
};
It ends up back at 2 because the recursion is doing its job as we pass the
value of record_array_count rather than a pointer to it in the next recusive
call.
There is no need to predicate on is_sampler() as record_array_count is
essentially an individualised outter array dimension count for each struct
member. Does that make sense?
As a result AoA will also use this count to allocate there space later on
although they don't really need to as we don't need to worry about these kind
of offsets but its just extra code to stop it for no real gain.
We are doing the count for interface too but we just don't use it for those.
Does that all make sense?
> --Jason
>
> > +
> > for (unsigned i = 0; i < t->length; i++) {
> > size_t new_length = name_length;
> >
> > @@ -230,7 +242,7 @@ program_resource_visitor::recursion(const glsl_type
> > *t, char **name,
> >
> > recursion(t->fields.array, name, new_length, row_major,
> > record_type,
> > - (i + 1) == t->length);
> > + (i + 1) == t->length, record_array_count);
> >
> > /* Only the first leaf-field of the record gets called with the
> > * record type pointer.
> > @@ -238,6 +250,7 @@ program_resource_visitor::recursion(const glsl_type
> > *t, char **name,
> > record_type = NULL;
> > }
> > } else {
> > + this->set_record_array_count(record_array_count);
> > this->visit_field(t, *name, row_major, record_type, last_field);
> > }
> > }
> > @@ -268,6 +281,11 @@ program_resource_visitor::leave_record(const
> > glsl_type *, const char *, bool)
> > {
> > }
> >
> > +void
> > +program_resource_visitor::set_record_array_count(unsigned)
> > +{
> > +}
> > +
> > namespace {
> >
> > /**
> > @@ -432,6 +450,7 @@ public:
> > this->next_sampler = 0;
> > this->next_image = 0;
> > this->next_subroutine = 0;
> > + this->record_array_count = 1;
> > memset(this->targets, 0, sizeof(this->targets));
> > }
> >
> > @@ -440,6 +459,7 @@ public:
> > {
> > current_var = var;
> > field_counter = 0;
> > + this->record_next_sampler = new string_to_uint_map;
> >
> > ubo_block_index = -1;
> > if (var->is_in_buffer_block()) {
> > @@ -493,6 +513,8 @@ public:
> > process(var);
> > } else
> > process(var);
> > + }
> > + delete this->record_next_sampler;
> > }
> >
> > int ubo_block_index;
> > @@ -501,17 +523,60 @@ public:
> >
> > private:
> > void handle_samplers(const glsl_type *base_type,
> > - struct gl_uniform_storage *uniform)
> > + struct gl_uniform_storage *uniform, const char
> > *name)
> > {
> > if (base_type->is_sampler()) {
> > - uniform->sampler[shader_type].index = this->next_sampler;
> > uniform->sampler[shader_type].active = true;
> >
> > - /* Increment the sampler by 1 for non-arrays and by the number
> > of
> > - * array elements for arrays.
> > - */
> > - this->next_sampler +=
> > - MAX2(1, uniform->array_elements);
> > + /* Handle multiple samplers inside struct arrays */
> > + if (this->record_array_count > 1) {
> > + unsigned inner_array_size = MAX2(1, uniform->array_elements);
> > + char *name_copy = ralloc_strdup(NULL, name);
> > +
> > + /* Remove all array subscripts from the sampler name */
> > + char *str_start;
> > + const char *str_end;
> > + while((str_start = strchr(name_copy, '[')) &&
> > + (str_end = strchr(name_copy, ']'))) {
> > + memmove(str_start, str_end + 1, 1 + strlen(str_end));
> > + }
> > +
> > + unsigned index = 0;
> > + if (this->record_next_sampler->get(index, name_copy)) {
> > + /* Set the base sampler index for the uniform, increment
> > the
> > + * next index and return as everything else has already
> > been
> > + * initialised.
> > + */
> > + uniform->sampler[shader_type].index = index;
> > + index = inner_array_size + uniform
> > ->sampler[shader_type].index;
> > + this->record_next_sampler->put(index, name_copy);
> > +
> > + ralloc_free(name_copy);
> > + return;
> > + } else {
> > + /* Nested struct arrays behave like arrays of arrays so we
> > need
> > + * to increase the index by the total number of elements
> > of the
> > + * sampler in case there is more than one sampler inside
> > the
> > + * structs. This allows the offset to be easily calculated
> > for
> > + * indirect indexing.
> > + */
> > + uniform->sampler[shader_type].index = this->next_sampler;
> > + this->next_sampler +=
> > + inner_array_size * this->record_array_count;
> > +
> > + /* Store the next index for future passes over the struct
> > array
> > + */
> > + index = uniform->sampler[shader_type].index +
> > inner_array_size;
> > + this->record_next_sampler->put(index, name_copy);
> > + ralloc_free(name_copy);
> > + }
> > + } else {
> > + /* Increment the sampler by 1 for non-arrays and by the
> > number of
> > + * array elements for arrays.
> > + */
> > + uniform->sampler[shader_type].index = this->next_sampler;
> > + this->next_sampler += MAX2(1, uniform->array_elements);
> > + }
> >
> > const gl_texture_index target = base_type->sampler_index();
> > const unsigned shadow = base_type->sampler_shadow;
> > @@ -564,6 +629,11 @@ private:
> > }
> > }
> >
> > + virtual void set_record_array_count(unsigned record_array_count)
> > + {
> > + this->record_array_count = record_array_count;
> > + }
> > +
> > virtual void visit_field(const glsl_type *type, const char *name,
> > bool row_major)
> > {
> > @@ -615,7 +685,7 @@ private:
> > }
> >
> > /* This assigns uniform indices to sampler and image uniforms. */
> > - handle_samplers(base_type, &this->uniforms[id]);
> > + handle_samplers(base_type, &this->uniforms[id], name);
> > handle_images(base_type, &this->uniforms[id]);
> > handle_subroutines(base_type, &this->uniforms[id]);
> >
> > @@ -704,6 +774,14 @@ private:
> > unsigned next_image;
> > unsigned next_subroutine;
> >
> > + /* Stores total struct array elements including nested structs */
> > + unsigned record_array_count;
> > +
> > + /* Map for temporarily storing next sampler index when handling
> > samplers in
> > + * struct arrays.
> > + */
> > + struct string_to_uint_map *record_next_sampler;
> > +
> > public:
> > union gl_constant_value *values;
> >
> > diff --git a/src/glsl/linker.h b/src/glsl/linker.h
> > index ce3dc32..40e6c73 100644
> > --- a/src/glsl/linker.h
> > +++ b/src/glsl/linker.h
> > @@ -181,6 +181,8 @@ protected:
> > virtual void leave_record(const glsl_type *type, const char *name,
> > bool row_major);
> >
> > + virtual void set_record_array_count(unsigned record_array_count);
> > +
> > private:
> > /**
> > * \param name_length Length of the current name \b not including the
> > @@ -191,7 +193,7 @@ private:
> > */
> > void recursion(const glsl_type *t, char **name, size_t name_length,
> > bool row_major, const glsl_type *record_type,
> > - bool last_field);
> > + bool last_field, unsigned record_array_count);
> > };
> >
> > void
> > --
> > 2.4.3
> >
> > _______________________________________________
> > 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