[Mesa-dev] [PATCH] i965: fix the constant interp bitmask for flat mode

[Yuanhan Liu]

On Mon, Sep 05, 2011 at 10:41:03AM -0700, Eric Anholt wrote:
> On Mon,  5 Sep 2011 15:59:32 +0800, Yuanhan Liu <yuanhan.liu at linux.intel.com> wrote:
> > Fix the constant interpolation enable bit mask for flat light mode.
> > FRAG_BIT_COL0 attribute bit might be 0, in which case we need to
> > shift one more bit right.
> > 
> > This would fix the oglc specularColor test fail on both Sandybridge and
> > Ivybridge.
> > 
> > Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> > Signed-off-by: Xiang, Haihao <haihao.xiang at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/gen6_sf_state.c |   14 ++++++++++++--
> >  src/mesa/drivers/dri/i965/gen7_sf_state.c |   14 ++++++++++++--
> >  2 files changed, 24 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> > index 5bb731d..2c954d8 100644
> > --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> > @@ -233,8 +233,18 @@ upload_sf_state(struct brw_context *brw)
> >  
> >     /* flat shading */
> >     if (ctx->Light.ShadeModel == GL_FLAT) {
> > -       dw17 |= ((brw->fragment_program->Base.InputsRead & (FRAG_BIT_COL0 | FRAG_BIT_COL1)) >>
> > -                ((brw->fragment_program->Base.InputsRead & FRAG_BIT_WPOS) ? 0 : 1));
> > +      /*
> > +       * Setup the Constant Interpolation Enable bit mask for each
> > +       * corresponding attribute(currently, we only care two attrs:
> > +       * FRAG_BIT_COL0 and FRAG_BIT_COL1).
> > +       *
> > +       * FIXME: should we care other attributes?
> > +       */
> > +      dw17 = brw->fragment_program->Base.InputsRead & (FRAG_BIT_COL0 | FRAG_BIT_COL1);
> > +      if ((brw->fragment_program->Base.InputsRead & FRAG_BIT_WPOS) == 0)
> > +         dw17 >>= 1;
> > +      if ((brw->fragment_program->Base.InputsRead & FRAG_BIT_COL0) == 0)
> > +         dw17 >>= 1;
> >     }
> 
> Seems like the code would be a lot more understandable if constant
> interpolation was set up in the "for (attr = 0; attr < FRAG_ATTRIB_MAX;
> attr++)" loop.  Plus then it would be obviously extendable when we do
> the flat keyword support.

Nice suggestion. New patch sent.

Thanks,
Yuanhan Liu