On Thu, 2017-10-19 at 08:29 +0200, Iago Toral wrote: > On Thu, 2017-10-19 at 17:14 +1100, Timothy Arceri wrote: > > > > On 19/10/17 16:57, Iago Toral Quiroga wrote: > > > From ARB_enhanced_layouts: > > > > > > "[...]when location aliasing, the aliases sharing the location > > > must have the same underlying numerical type (floating-point or > > > integer) and the same auxiliary storage and > > > interpolation qualification.[...]" > > > > > > Add code to the linker to validate that aliased locations do > > > have the same interpolation. > > > > > > Fixes: > > > KHR- > > > GL45.enhanced_layouts.varying_location_aliasing_with_mixed_interp > > > ol > > > ation > > > --- > > > src/compiler/glsl/link_varyings.cpp | 35 > > > +++++++++++++++++++++++++++++++++++ > > > 1 file changed, 35 insertions(+) > > > > > > diff --git a/src/compiler/glsl/link_varyings.cpp > > > b/src/compiler/glsl/link_varyings.cpp > > > index 69c92bf53b..c888635e82 100644 > > > --- a/src/compiler/glsl/link_varyings.cpp > > > +++ b/src/compiler/glsl/link_varyings.cpp > > > @@ -459,6 +459,41 @@ cross_validate_outputs_to_inputs(struct > > > gl_context *ctx, > > > > > > while (idx < slot_limit) { > > > unsigned i = var->data.location_frac; > > > + > > > + /* If there are other outputs assigned to the same > > > location > > > + * they must have the same interpolation > > > + */ > > > + unsigned comp = 0; > > > + while (comp < i) { > > > + ir_variable *tmp = explicit_locations[idx][comp]; > > > + if (tmp && tmp->data.interpolation != var- > > > > data.interpolation) { > > > > > > + linker_error(prog, > > > + "%s shader has multiple outputs > > > at > > > explicit " > > > + "location %u with different > > > interpolation " > > > + "settings\n", > > > + _mesa_shader_stage_to_string(prod > > > uc > > > er->Stage), > > > + idx); > > > + return; > > > + } > > > + comp++; > > > + } > > > + > > > + comp = last_comp + 1; > > I just noticed that this should be: > > comp = last_comp; > > since that is the first component not used by this variable. > > > > + while (comp < 4) { > > > + ir_variable *tmp = explicit_locations[idx][comp]; > > > + if (tmp && tmp->data.interpolation != var- > > > > data.interpolation) { > > > > > > + linker_error(prog, > > > + "%s shader has multiple outputs > > > at > > > explicit " > > > + "location %u with different > > > interpolation " > > > + "settings\n", > > > + _mesa_shader_stage_to_string(prod > > > uc > > > er->Stage), > > > + idx); > > > + return; > > > + } > > > + comp++; > > > + } > > > > Can't we just put this check in the loop below? It should allow > > doubles > > to be handled without the duplication if my quick skim over the > > code > > is > > correct. > > Mmm...I don't think so. The loop below checks the components used by > the current variable to see if there is aliasing in the range used by > the variable: [i, last_comp). We need to check if there are other > variables assigned to any other components in the same location to > check if there is a different interpolation for them, so that means > that we need to check all the components in the location that are > _not_ > used by this variable, so basically, all the components not covered > by > the loop below. This means the regions [0...i) and [last_comp...4]. > > We could make the loop below go up to 4 instead of last_comp and the > nhave ifs inside to do one thing in the range [i...last_comp) and > another one in the range [last_comp, 4] but that would only make > things > more confusing IMHO, so I'd rather not do that.
However, we can do a single loop and skip the components in [1, last_comp) for these checks so we only have one loop instead of two. I'll send a v2 with this change. I have another patch to add also checks for the auxiliary storage and this way we don't have to duplicate those too. Iago > > > > > + > > > + /* Component aliasing is not allowed */ > > > while (i < last_comp) { > > > if (explicit_locations[idx][i] != NULL) { > > > linker_error(prog, > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev