[Mesa-dev] [PATCH] glsl: add varying resources for arrays of complex types

[Juan A. Suarez Romero]

On Fri, 2017-10-27 at 10:27 -0400, Ilia Mirkin wrote:
> On Fri, Oct 27, 2017 at 10:03 AM, Juan A. Suarez Romero
> <jasuarez at igalia.com> wrote:
> > This patch is mostly a patch done by Ilia Mirkin.
> > 
> > It fixes KHR-GL45.enhanced_layouts.varying_structure_locations.
> > 
> > CC: Ilia Mirkin <imirkin at alum.mit.edu>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103098
> > Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> > ---
> >  src/compiler/glsl/linker.cpp | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> > index f827b68555..7305b7740b 100644
> > --- a/src/compiler/glsl/linker.cpp
> > +++ b/src/compiler/glsl/linker.cpp
> > @@ -3882,6 +3882,42 @@ add_shader_variable(const struct gl_context *ctx,
> >        return true;
> >     }
> > 
> > +   case GLSL_TYPE_ARRAY: {
> > +      /* The ARB_program_interface_query spec says:
> > +       *
> > +       *     "For an active variable declared as an array of basic types, a
> > +       *      single entry will be generated, with its name string formed by
> > +       *      concatenating the name of the array and the string "[0]"."
> > +       *
> > +       *     "For an active variable declared as an array of an aggregate data
> > +       *      type (structures or arrays), a separate entry will be generated
> > +       *      for each active array element, unless noted immediately below.
> > +       *      The name of each entry is formed by concatenating the name of
> > +       *      the array, the "[" character, an integer identifying the element
> > +       *      number, and the "]" character.  These enumeration rules are
> > +       *      applied recursively, treating each enumerated array element as a
> > +       *      separate active variable."
> > +       */
> 
> So if you have a TCS or TES or GS with
> 
> in struct { vec4 foo; } bar[4];
> 
> Wouldn't this add bar[0].foo, bar[1].foo, bar[2].foo, and bar[3].foo?

According to the spec, yes. Unless I'm missing something...


Looking for other examples, in

https://www.khronos.org/opengl/wiki/Program_Introspection#Arrays

it shows a similar example, and also do as you say.

> With the latter ones getting bogus locations? What is it supposed to
> do in this case?

Why it would get bogus locations?


> 
> > +      const struct glsl_type *array_type = type->fields.array;
> > +      if (array_type->base_type == GLSL_TYPE_STRUCT ||
> > +          array_type->base_type == GLSL_TYPE_ARRAY) {
> > +         unsigned elem_location = location;
> > +         unsigned stride = array_type->count_attribute_slots(false);
> > +         for (unsigned i = 0; i < type->length; i++) {
> > +            char *elem = ralloc_asprintf(shProg, "%s[%d]", name, i);
> > +            if (!add_shader_variable(ctx, shProg, resource_set,
> > +                                     stage_mask, programInterface,
> > +                                     var, elem, array_type,
> > +                                     use_implicit_location, elem_location,
> > +                                     outermost_struct_type))
> > +               return false;
> > +            elem_location += stride;
> > +         }
> > +         return true;
> > +      }
> > +      /* fallthrough */
> > +   }
> > +
> >     default: {
> >        /* The ARB_program_interface_query spec says:
> >         *
> > --
> > 2.13.6
> > 
> 
>