[Mesa-dev] [PATCH 1/2] glsl: Fix varying struct locations when varying packing is disabled.

[Timothy Arceri]

On Wed, 2015-12-30 at 08:49 -0500, Ilia Mirkin wrote:
> On Wed, Dec 30, 2015 at 6:26 AM, Kenneth Graunke <
> kenneth at whitecape.org> wrote:
> > varying_matches::record tries to compute the number of components
> > in
> > each varying, which varying_matches::assign_locations uses to
> > assign
> > locations.  With varying packing, it uses
> > glsl_type::component_slots()
> > to come up with a reasonable value.
> > 
> > Without varying packing, it fell back to an open-coded computation
> > that didn't bother to handle structs at all.  I believe we can
> > simply
> > use 4 * glsl_type::count_attribute_slots(false), which already
> > handles
> > these cases correctly.
> > 
> > Partially fixes rendering in GFXBench 4.0's tessellation benchmark.
> > (NVE0 is almost right after this, but i965 is still mostly
> > garbage.)
> > 
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > Cc: mesa-stable at lists.freedesktop.org
> > Cc: Timothy Arceri <timothy.arceri at collabora.com>
> > Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> 
> That seems right... but 11.0/11.1 didn't have the
> count_attribute_slots(true/false) thing -- only true :(
> 
> I'd be fine with nominating commit 5dc22cadb for 11.0/11.1 stable
> though and then including this patch as well. Either way, this one is

Yeah I'd like to see that change in at least 11.1 too so I could
nominate this varying packing fix:

http://patchwork.freedesktop.org/patch/69126/
http://patchwork.freedesktop.org/patch/69127/


Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>

> 
> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
> 
> > ---
> >  src/glsl/link_varyings.cpp | 15 ++-------------
> >  1 file changed, 2 insertions(+), 13 deletions(-)
> > 
> > diff --git a/src/glsl/link_varyings.cpp
> > b/src/glsl/link_varyings.cpp
> > index 9cc77fe..1ff25b8 100644
> > --- a/src/glsl/link_varyings.cpp
> > +++ b/src/glsl/link_varyings.cpp
> > @@ -959,19 +959,8 @@ varying_matches::record(ir_variable
> > *producer_var, ir_variable *consumer_var)
> >           type = type->fields.array;
> >        }
> > 
> > -      if (type->is_array()) {
> > -         slots = 1;
> > -         while (type->is_array()) {
> > -            slots *= type->length;
> > -            type = type->fields.array;
> > -         }
> > -         slots *= type->matrix_columns;
> > -      } else {
> > -         slots = type->matrix_columns;
> > -      }
> > -      if (type->without_array()->is_dual_slot_double())
> > -         slots *= 2;
> > -      this->matches[this->num_matches].num_components = 4 * slots;
> > +      slots = type->count_attribute_slots(false);
> > +      this->matches[this->num_matches].num_components = slots * 4;
> >     } else {
> >        this->matches[this->num_matches].num_components
> >           = var->type->component_slots();
> > --
> > 2.6.4
> > 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev