[Mesa-dev] [PATCH] glsl: fix varying slot allocation for blocks and structs with explicit locations

[Ilia Mirkin]

On Wed, Jan 6, 2016 at 4:32 AM, Timothy Arceri
<timothy.arceri at collabora.com> wrote:
> Previously each member was being counted as using a single slot,
> count_attribute_slots() fixes the count for array and struct members.
>
> Also don't assign a negitive to the unsigned expl_location variable.
> ---
>
>  Fixes these new piglit tests:
>    http://patchwork.freedesktop.org/patch/69531/
>
>  src/glsl/ast_to_hir.cpp | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 0197cdc..50d5e22 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -6408,12 +6408,13 @@ ast_process_struct_or_iface_block_members(exec_list *instructions,
>              if (process_qualifier_constant(state, &loc, "location",
>                                             qual->location, &qual_location)) {
>                 fields[i].location = VARYING_SLOT_VAR0 + qual_location;
> -               expl_location = fields[i].location + 1;
> +               expl_location = fields[i].location +
> +                  fields[i].type->count_attribute_slots(false);
>              }
>           } else {
>              if (layout && layout->flags.q.explicit_location) {
>                 fields[i].location = expl_location;
> -               expl_location = expl_location + 1;
> +               expl_location += fields[i].type->count_attribute_slots(false);
>              } else {
>                 fields[i].location = -1;
>              }
> @@ -6570,7 +6571,7 @@ ast_struct_specifier::hir(exec_list *instructions,
>
>     state->struct_specifier_depth++;
>
> -   unsigned expl_location = -1;
> +   unsigned expl_location = 0;
>     if (layout && layout->flags.q.explicit_location) {
>        if (!process_qualifier_constant(state, &loc, "location",
>                                        layout->location, &expl_location)) {
> @@ -6763,7 +6764,7 @@ ast_interface_block::hir(exec_list *instructions,
>        return NULL;
>     }
>
> -   unsigned expl_location = -1;
> +   unsigned expl_location = 0;

There are a number of places that check for location != -1 as a sanity
check... won't this defeat that?

>     if (layout.flags.q.explicit_location) {
>        if (!process_qualifier_constant(state, &loc, "location",
>                                        layout.location, &expl_location)) {
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev