[Mesa-dev] [PATCH v2] i965/sbe: fix number of inputs for active components

[Iago Toral]

On Wed, 2018-02-28 at 15:39 -0800, Kenneth Graunke wrote:
> On Monday, February 26, 2018 11:02:08 PM PST Iago Toral Quiroga
> wrote:
> > In 16631ca30ea6 we fixed gen9 active components to account for
> > padded
> > inputs in the URB, which we can have with SSO programs. To do that,
> > instead of going through the bitfield of inputs (which doesn't
> > include
> > padding information), we compute the number of inputs from the size
> > of the URB entry.
> > 
> > Unfortunately, there are some special inputs that are not stored in
> > the URB and that we also need to account for. These special inputs
> > are identified and handled during calculate_attr_overrides(), so
> > this
> > patch modifies this function to return a value with the total
> > number
> > of inputs, including the ones that are not stored in the URB, so we
> > can use that number to program the correct number of active
> > components.
> > 
> > This fixes a regression in a WebGL program that uses Point Sprite
> > functionality (specifically, VARYING_SLOT_PNTC).
> > 
> > v2:
> >  - Add 'Fixes' tag (Mark Janes)
> >  - make no_vue_inputs int instead of uint32_t, and add const
> > qualifier
> >    to num_inputs variable (Ian)
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105224
> > Fixes: 16631ca30ea6 (i965/sbe: fix active components for SSO
> > programs with over 16 inputs)
> 
> Or you could just steal the code from anv and do:
> 
>    for (unsigned i = 0; i < 32; i++)
>       sbe.AttributeActiveComponentFormat[i] = ACF_XYZW;
> 
> instead of trying to count correctly.


Yeah, I was wondering if that would have any performance impact, so I
have tried this approach with a couple of WebGL demos and I haven't
seen much of a difference. I have also tried a terrain demo that I have
around and gives me more precise frame timing and I seem to get ~0.03
more fps on average with the original patch, so I would say that
performance impact is negligible, if it exists at all.

Seeing how performance-wise there is not a clear difference I'll push
the patch with your recommendation if Jenkins doesn't report anything
weird, which I don't expect it to.

Iago