[Mesa-dev] [PATCH V4 5/6] glsl: add helper for calculating offsets for struct members

[Timothy Arceri]

On Tue, 2015-09-15 at 11:51 -0700, Jason Ekstrand wrote:
> On Tue, Sep 15, 2015 at 12:51 AM, Timothy Arceri <
> t_arceri at yahoo.com.au> wrote:
> > From: Timothy <t_arceri at yahoo.com.au>
> > 
> > Cc: Jason Ekstrand <jason.ekstrand at intel.com>
> > ---
> >  src/glsl/glsl_types.cpp | 20 ++++++++++++++++++++
> >  src/glsl/glsl_types.h   |  7 +++++++
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> > index 755618a..38b1660 100644
> > --- a/src/glsl/glsl_types.cpp
> > +++ b/src/glsl/glsl_types.cpp
> > @@ -1040,6 +1040,26 @@ glsl_type::component_slots() const
> >  }
> > 
> >  unsigned
> > +glsl_type::record_location_offset(unsigned length) const
> > +{
> > +   unsigned offset = 0;
> > +   const glsl_type *t = this->without_array();
> > +   if (t->is_record()) {
> > +      for (unsigned i = 0; i < length; i++) {
> > +         const glsl_type *st = t->fields.structure[i].type;
> > +         const glsl_type *wa = st->without_array();
> > +         if (wa->is_record()) {
> > +            unsigned r_offset = wa->record_location_offset(wa
> > ->length);
> > +            offset += st->is_array() ? st->length * r_offset :
> > r_offset;
> > +         } else {
> > +            offset += 1;
> > +         }
> > +      }
> > +   }
> > +   return offset;
> > +}
> 
> I'm not sure how I feel about having this as a general thin in
> glsl_type.  It's really just for samplers, so I think I'd rather just
> add it as a helper in nir_lower_samplers.

Sure I can see what you mean I guess I put it here for non-nir drivers.
Also there is also no reason this needs to be sampler specific
(although there are obiviously no uses elsewhere currently).

I'll move it.

> 
> Also, IMHO, this would be easier to read if it were written more like
> this:
> 
> unsigned
> get_struct_field_offset(glsl_type *type, unsigned length)
> {
>    switch (type->base_type) {
>    case SAMPLER:
>       return 1;
>    case ARRAY:
>       return get_struct_field_offset(type->array_type,
> type->array_type->length) * type->array_length;
>    case STRUCT: {
>       unsigned offset = 0;
>       for (stuff) {
>          offset += get_struct_field_offset(field_type, field_type
> ->length)
>       }
>    default:
>       return 0;
>    }
> }
> 
> The way you wrote it is very GLSL IR style which is fine for what it
> is.  I simply find explicitly listing all the cases easier to follow
> than the without_array() stuff.

I see what you mean but the code above won't work the way we want it
to. We are getting the offset for the uniform not the sampler, so we do
care about and want to count all members not just samplers.

Also we only want to add arrays to the count if its an array of a
struct (or array of array but I will add that support later) as arrays
of other members only take up a single uniform location, this is why
without_array() is important. I'll have a think about a better way to
implement this but I'm not sure the alternative will be much better.

> 
> Also, note the default case in my example.  When we're figuring out
> where we want to put samplers, we don't want to count in struct
> members that aren't samplers. 

As above we what the uniform index here not the sampler index.

>  Again, this is why this is a
> sampler-specific thing and should go in nir_lower_samplers.
> --Jason
> 
> > +
> > +unsigned
> >  glsl_type::uniform_locations() const
> >  {
> >     unsigned size = 0;
> > diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
> > index 02a398f..8ee8968 100644
> > --- a/src/glsl/glsl_types.h
> > +++ b/src/glsl/glsl_types.h
> > @@ -292,6 +292,13 @@ struct glsl_type {
> >     unsigned component_slots() const;
> > 
> >     /**
> > +    * Calculate offset between the base location and this struct
> > member.
> > +    * For the initial call length is the index of the member to
> > find the
> > +    * offset for.
> > +    */
> > +   unsigned record_location_offset(unsigned length) const;
> > +
> > +   /**
> >      * Calculate the number of unique values from
> > glGetUniformLocation for the
> >      * elements of the type.
> >      *
> > --
> > 2.4.3
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev