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

[Jason Ekstrand]

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.

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.

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.  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