[Mesa-dev] [PATCH 11/28] glsl: cross validate varyings with a component qualifier

[Anuj Phogat]

On Mon, Dec 28, 2015 at 9:00 PM, Timothy Arceri
<timothy.arceri at collabora.com> wrote:
> This change checks for component overlap, including handling overlap of
> locations and components by doubles. Previously there was no validation
> for assigning explicit locations to a location used by the second half
> of a double.
>
> V2: fix component matching for matricies
> ---
>  src/glsl/link_varyings.cpp | 71 +++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index dea8741..496b987 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -222,7 +222,7 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
>                                  gl_shader *producer, gl_shader *consumer)
>  {
>     glsl_symbol_table parameters;
> -   ir_variable *explicit_locations[MAX_VARYING] = { NULL, };
> +   ir_variable *explicit_locations[MAX_VARYING][4] = { {NULL, NULL} };
>
>     /* Find all shader outputs in the "producer" stage.
>      */
> @@ -243,18 +243,67 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
>           unsigned num_elements = type->count_attribute_slots(false);
>           unsigned idx = var->data.location - VARYING_SLOT_VAR0;
>           unsigned slot_limit = idx + num_elements;
> +         unsigned last_comp;
> +
> +         if (var->type->without_array()->is_record()) {
> +            /* The componet qualifier can't be used on structs so just treat
> +             * all component slots as used.
> +             */
> +            last_comp = 4;
> +         } else {
> +            last_comp = var->data.location_frac +
> +               var->type->without_array()->vector_elements;
> +         }
>
>           while(idx < slot_limit) {
> -            if (explicit_locations[idx] != NULL) {
> -               linker_error(prog,
> -                         "%s shader has multiple outputs explicitly "
> -                         "assigned to location %d\n",
> -                         _mesa_shader_stage_to_string(producer->Stage),
> -                         idx);
> -               return;
> -            }
> +            bool d_second_location = false;
> +            for (unsigned i = var->data.location_frac; i < last_comp; i++) {
> +               if (explicit_locations[idx][i] != NULL) {
> +                  linker_error(prog,
> +                               "%s shader has multiple outputs explicitly "
> +                               "assigned to location %d and component %d\n",
> +                               _mesa_shader_stage_to_string(producer->Stage),
> +                               idx, var->data.location_frac);
> +                  return;
> +               }
>
> -            explicit_locations[idx] = var;
> +               /* Make sure all component at this location have the same type.
> +                */
> +               for (unsigned j = 0; j < 4; j++) {
> +                  if (explicit_locations[idx][j] &&
> +                      (explicit_locations[idx][j]->type->without_array()
> +                       ->base_type != var->type->without_array()->base_type)) {
> +                     linker_error(prog,
> +                                  "Varyings sharing the same location must "
> +                                  "have the same underlying numerical type. "
> +                                  "Location %u component %u\n", idx,
> +                                  var->data.location_frac);
> +                     return;
> +                  }
> +               }
> +
> +               explicit_locations[idx][i] = var;
With this code we end up setting only components 0 and 1 for dvec2. My
understanding is dvec2 will take all 4 components?
> +
> +               /* We need to do some special handling for doubles as dvec3 and
> +                * dvec4 consume two consecutive locations. We don't need to
> +                * worry about components beginning at anything other than 0 as
> +                * the spec does not allow this for dvec3 and dvec4.
> +                */
> +               if (type->without_array()->is_dual_slot_double() &&
> +                   i == last_comp - 1 && !d_second_location) {
> +                  /* For dvec3 mark last slot in this location as used and
> +                   * change the last slot for the next location to 2.
> +                   */
> +                  if (last_comp == 3) {
> +                     explicit_locations[idx][i+1] = var;
> +                     last_comp = 2;
> +                  }
> +                  /* Bump location index and reset the component index */
> +                  idx++;
> +                  i = 0;
> +                  d_second_location = true;
> +               }
> +            }
>              idx++;
>           }
>        }
> @@ -311,7 +360,7 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
>              unsigned slot_limit = idx + num_elements;
>
>              while(idx < slot_limit) {
> -               output = explicit_locations[idx];
> +               output = explicit_locations[idx][input->data.location_frac];
>
>                 if (output == NULL ||
>                     input->data.location != output->data.location) {
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev