[Mesa-dev] [PATCH 1/3] glsl/linker: fix varying packing for non-flat integer varyings.

[Jordan Justen]

On Sun, Apr 7, 2013 at 11:42 AM, Paul Berry <stereotype441 at gmail.com> wrote:
> On 7 April 2013 11:12, Jordan Justen <jljusten at gmail.com> wrote:
>>
>> On Sat, Apr 6, 2013 at 7:49 PM, Paul Berry <stereotype441 at gmail.com>
>> wrote:
>> > +   if (consumer_var == NULL) {
>> > +      /* Since there is no consumer_var, the interpolation type of this
>> > +       * varying cannot possibly affect rendering.  Also, since the GL
>> > spec
>> > +       * only requires integer varyings to be "flat" when they are
>> > fragment
>> > +       * shader inputs, it is possible that this variable is non-flat
>> > and is
>> > +       * (or contains) an integer.
>> > +       *
>> > +       * lower_packed_varyings requires all integer varyings to flat,
>> > +       * regardless of where they appear.  We can trivially satisfy
>> > that
>> > +       * requirement by changing the interpolation type to flat here.
>> > +       */
>> > +      producer_var->centroid = false;
>> > +      producer_var->interpolation = INTERP_QUALIFIER_FLAT;
>>
>> Should we check if producer_var's type is an integer?
>
> Yeah, that's a good point.  It shouldn't affect correctness,

It isn't possible to have a non-integer scenario of this where we
wouldn't want to override the interpolation type?

> but it will
> make the patch less invasive, and that seems worthwhile since I'm hoping
> this can be cherry-picked back to 9.1.  I'll follow up with a v2 of the
> series.
>
>> Could this cause issues if the producer stage is later linked to
>> another consumer?
>
> That shouldn't be a problem, since at this stage of linking, we are
> operating on a copy of the IR that is private to this particular linked
> program.

Okay, well, based on that and adding the int check above, for the series:
Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>