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

[Ilia Mirkin]

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?
With the latter ones getting bogus locations? What is it supposed to
do in this case?

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