On Wed, 2017-10-25 at 08:35 -0400, Ilia Mirkin wrote:
> Not your fault, but this diff is hard to read. On the bright side, 20
> lines removed... Can you confirm that
>
> layout(location = 1, component = 2) int foo;
> layout(location = 0) dvec3 bar;
>
> fails due to the int vs double conflicts? (I'm specifically
> interested
> in the case where the int is specified first and dvec3 is specified
> second, since it'll be up to the processing of the dvec3 to determine
> the conflict on the second slot.)
>
> Assuming that all checks out,
Yeah, I have checked this (and a few other variations) and everything
seems to work as expected.
> Acked-by: Ilia Mirkin <imir...@alum.mit.edu>
Thanks to you and Timothy for all the reviews!
Iago
>
> On Wed, Oct 25, 2017 at 5:15 AM, Iago Toral Quiroga <ito...@igalia.co
> m> wrote:
> > Mostly, this merges the type checks with all the other checks so
> > we only have a single loop for this.
> > ---
> > src/compiler/glsl/link_varyings.cpp | 110 +++++++++++++++---------
> > ------------
> > 1 file changed, 46 insertions(+), 64 deletions(-)
> >
> > diff --git a/src/compiler/glsl/link_varyings.cpp
> > b/src/compiler/glsl/link_varyings.cpp
> > index e80736fb38..8767f00976 100644
> > --- a/src/compiler/glsl/link_varyings.cpp
> > +++ b/src/compiler/glsl/link_varyings.cpp
> > @@ -438,92 +438,74 @@ check_location_aliasing(struct
> > explicit_location_info explicit_locations[][4],
> > }
> >
> > while (location < location_limit) {
> > - unsigned i = component;
> > -
> > - /* If there are other outputs assigned to the same location
> > - * they must have the same interpolation
> > - */
> > unsigned comp = 0;
> > while (comp < 4) {
> > - /* Skip the components used by this output, we only care
> > about
> > - * other outputs in the same location
> > - */
> > - if (comp == i) {
> > - comp = last_comp;
> > - continue;
> > - }
> > -
> > struct explicit_location_info *info =
> > &explicit_locations[location][comp];
> >
> > if (info->var) {
> > - if (info->interpolation != interpolation) {
> > + /* Component aliasing is not alloed */
> > + if (comp >= component && comp < last_comp) {
> > linker_error(prog,
> > - "%s shader has multiple outputs at
> > explicit "
> > - "location %u with different
> > interpolation "
> > - "settings\n",
> > - _mesa_shader_stage_to_string(stage),
> > location);
> > + "%s shader has multiple outputs
> > explicitly "
> > + "assigned to location %d and component
> > %d\n",
> > + _mesa_shader_stage_to_string(stage),
> > + location, component);
> > return false;
> > - }
> > -
> > - if (info->centroid != centroid ||
> > - info->sample != sample ||
> > - info->patch != patch) {
> > - linker_error(prog,
> > - "%s shader has multiple outputs at
> > explicit "
> > - "location %u with different aux
> > storage\n",
> > - _mesa_shader_stage_to_string(stage),
> > location);
> > - return false;
> > - }
> > - }
> > -
> > - comp++;
> > - }
> > + } else {
> > + /* For all other used components we need to have
> > matching
> > + * types, interpolation and auxiliary storage
> > + */
> > + if (info->base_type != 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",
> > + location, component);
> > + return false;
> > + }
> >
> > - /* Component aliasing is not allowed */
> > - while (i < last_comp) {
> > - if (explicit_locations[location][i].var != NULL) {
> > - linker_error(prog,
> > - "%s shader has multiple outputs
> > explicitly "
> > - "assigned to location %d and component
> > %d\n",
> > - _mesa_shader_stage_to_string(stage),
> > - location, component);
> > - return false;
> > - }
> > + if (info->interpolation != interpolation) {
> > + linker_error(prog,
> > + "%s shader has multiple outputs at
> > explicit "
> > + "location %u with different
> > interpolation "
> > + "settings\n",
> > + _mesa_shader_stage_to_string(stage)
> > , location);
> > + return false;
> > + }
> >
> > - /* Make sure all component at this location have the same
> > type.
> > - */
> > - for (unsigned j = 0; j < 4; j++) {
> > - if (explicit_locations[location][j].var &&
> > - explicit_locations[location][j].base_type !=
> > - 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",
> > location, component);
> > - return false;
> > + if (info->centroid != centroid ||
> > + info->sample != sample ||
> > + info->patch != patch) {
> > + linker_error(prog,
> > + "%s shader has multiple outputs at
> > explicit "
> > + "location %u with different aux
> > storage\n",
> > + _mesa_shader_stage_to_string(stage)
> > , location);
> > + return false;
> > + }
> > }
> > + } else if (comp >= component && comp < last_comp) {
> > + info->var = var;
> > + info->base_type = type->without_array()->base_type;
> > + info->interpolation = interpolation;
> > + info->centroid = centroid;
> > + info->sample = sample;
> > + info->patch = patch;
> > }
> >
> > - explicit_locations[location][i].var = var;
> > - explicit_locations[location][i].base_type =
> > - type->without_array()->base_type;
> > - explicit_locations[location][i].interpolation =
> > interpolation;
> > - explicit_locations[location][i].centroid = centroid;
> > - explicit_locations[location][i].sample = sample;
> > - explicit_locations[location][i].patch = patch;
> > - i++;
> > + comp++;
> >
> > /* 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 (i == 4 && last_comp > 4) {
> > + if (comp == 4 && last_comp > 4) {
> > last_comp = last_comp - 4;
> > /* Bump location index and reset the component index
> > */
> > location++;
> > - i = 0;
> > + comp = 0;
> > + component = 0;
> > }
> > }
> >
> > --
> > 2.11.0
> >
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
