[Mesa-dev] [PATCH] i915: Always emit W on gen3

[Ville Syrjälä]

On Tue, Jun 20, 2017 at 11:00:52AM -0700, Ian Romanick wrote:
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

Thanks. Pushed to master.

> 
> On 06/20/2017 10:22 AM, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Unlike the older gen2 hardware, gen3 performs perspective
> > correct interpolation even for the primary/secondary colors.
> > To do that it naturally needs us to emit W for the vertices.
> > 
> > Currently we emit W only when at least one texture coordinate
> > set gets emitted. This means the interpolation of color will
> > change depending on whether texcoords/varyings are used or not.
> > That's probably not what anyone would expect, so let's just
> > always emit W to get consistent behaviour. Trying to avoid
> > emitting W seems like more hassle than it's worth, especially
> > as bspec seems to suggest that the hardware will perform the
> > perspective division anyway.
> > 
> > This used to be broken until it was accidentally fixed it in
> > commit c349031c27b7 ("i915: Fix texcoord vs. varying collision
> > in fragment programs") by introducing a bug that made the driver
> > always emit W. After fixing that bug in commit c1eedb43f32f
> > ("i915: Fix wpos_tex vs. -1 comparison") we went back to the
> > old behaviour and caused an apparent regression.
> > 
> > Fixes: c1eedb43f32f ("i915: Fix wpos_tex vs. -1 comparison")
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101451
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  src/mesa/drivers/dri/i915/i915_fragprog.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c b/src/mesa/drivers/dri/i915/i915_fragprog.c
> > index e19ca60bd334..3657b2d82565 100644
> > --- a/src/mesa/drivers/dri/i915/i915_fragprog.c
> > +++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
> > @@ -1252,12 +1252,10 @@ i915ValidateFragmentProgram(struct i915_context *i915)
> >     intel->coloroffset = 0;
> >     intel->specoffset = 0;
> >  
> > -   if (inputsRead & VARYING_BITS_TEX_ANY || p->wpos_tex != I915_WPOS_TEX_INVALID) {
> > -      EMIT_ATTR(_TNL_ATTRIB_POS, EMIT_4F_VIEWPORT, S4_VFMT_XYZW, 16);
> > -   }
> > -   else {
> > -      EMIT_ATTR(_TNL_ATTRIB_POS, EMIT_3F_VIEWPORT, S4_VFMT_XYZ, 12);
> > -   }
> > +   /* Always emit W to get consistent perspective
> > +    * correct interpolation of primary/secondary colors.
> > +    */
> > +   EMIT_ATTR(_TNL_ATTRIB_POS, EMIT_4F_VIEWPORT, S4_VFMT_XYZW, 16);
> >  
> >     /* Handle gl_PointSize builtin var here */
> >     if (ctx->Point._Attenuated || ctx->VertexProgram.PointSizeEnabled)
> > 

-- 
Ville Syrjälä
Intel OTC