On Thu, 2016-01-07 at 13:47 -0800, Anuj Phogat wrote:
> On Mon, Dec 28, 2015 at 9:00 PM, Timothy Arceri
> <timothy.arc...@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?
Nice spotting, I've sent a new version of the patch which makes double
handling much nicer and fixes this issue.
There is also a new piglit to test this case:
http://patchwork.freedesktop.org/patch/69671/
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
