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_interpol > > 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(produc > > 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(produc > > 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. 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