[Mesa-dev] [PATCH 1/9] i915: Only use TEXCOORDTYPE_VECTOR with cube maps on gen2

[Ville Syrjälä]

On Thu, Nov 20, 2014 at 09:59:00AM -0800, Ian Romanick wrote:
> On 08/06/2014 11:56 AM, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Check that the target is GL_TEXTURE_CUBE_MAP before emitting
> > TEXCOORDTYPE_VECTOR texture coordinates.
> > 
> > I'm not sure if the hardware would like CARTESIAN coordinates
> > with cube maps, and as I'm too lazy to find out just emit the
> > VECTOR coordinates for cube maps always. For other targets use
> > CARTESIAN or HOMOGENOUS depending on the number of texture
> > coordinates provided.
> > 
> > Fixes rendering of the "electric" background texture in chromium-bsu
> > main menu. We appear to be provided with three texture coordinates
> > there (I'm guessing due to the funky texture matrix rotation it does).
> > So the code would decide to use TEXCOORDTYPE_VECTOR instead of
> > TEXCOORDTYPE_CARTESIAN even though we're dealing with a 2D texure.
> > The results weren't what one might expect.
> > 
> > demos/cubemap still works, which hopefully indicates that this doesn't
> > break things.
> 
> I won't dare ask about a full piglit run on that hardware,

I did actually try to run piglit on the 830, but it always
seemed to die in some X blit batch after a while :(

There have been some recent blt TLB workaround fixes in the
kernel though, so perhaps things are better now. And if not,
I do have a 855 that ought to be a bit less fragile. So
maybe I'll give it another go just for kicks :P

> but how about
> 
>     bin/glean -o -v -v -v -t +texCube --quick
> 
> and
> 
>     bin/cubemap -auto
> 
> from piglit?

pass->pass on both counts.

> 
> These changes seem reasonable, and assuming those tests aren't made worse,
> 
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
> 
> If you're excited about GEN2 bugs, wanna take a look at 79841? >:)

I'm thinking it should be fixed by:

commit dafae910d4fc791ba49f20e937cb918669f42944
Author: Ville Syrjälä <ville.syrjala at linux.intel.com>
Date:   Thu Jul 3 15:38:07 2014 +0300

    i915: Accept GL_DEPTH_STENCIL GL_DEPTH_COMPONENT formats for
    renderbuffers

I'll note it in the bug.

> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  src/mesa/drivers/dri/i915/i830_vtbl.c | 37 ++++++++++++++++++-----------------
> >  1 file changed, 19 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i915/i830_vtbl.c b/src/mesa/drivers/dri/i915/i830_vtbl.c
> > index 53d408b..0f22d86 100644
> > --- a/src/mesa/drivers/dri/i915/i830_vtbl.c
> > +++ b/src/mesa/drivers/dri/i915/i830_vtbl.c
> > @@ -134,27 +134,28 @@ i830_render_start(struct intel_context *intel)
> >              GLuint mcs = (i830->state.Tex[i][I830_TEXREG_MCS] &
> >                            ~TEXCOORDTYPE_MASK);
> >  
> > -            switch (sz) {
> > -            case 1:
> > -            case 2:
> > -               emit = EMIT_2F;
> > -               sz = 2;
> > -               mcs |= TEXCOORDTYPE_CARTESIAN;
> > -               break;
> > -            case 3:
> > +            if (intel->ctx.Texture.Unit[i]._Current->Target == GL_TEXTURE_CUBE_MAP) {
> >                 emit = EMIT_3F;
> >                 sz = 3;
> >                 mcs |= TEXCOORDTYPE_VECTOR;
> > -               break;
> > -            case 4:
> > -               emit = EMIT_3F_XYW;
> > -               sz = 3;
> > -               mcs |= TEXCOORDTYPE_HOMOGENEOUS;
> > -               break;
> > -            default:
> > -               continue;
> > -            };
> > -
> > +            } else {
> > +               switch (sz) {
> > +               case 1:
> > +               case 2:
> > +               case 3:
> > +                  emit = EMIT_2F;
> > +                  sz = 2;
> > +                  mcs |= TEXCOORDTYPE_CARTESIAN;
> > +                  break;
> > +               case 4:
> > +                  emit = EMIT_3F_XYW;
> > +                  sz = 3;
> > +                  mcs |= TEXCOORDTYPE_HOMOGENEOUS;
> > +                  break;
> > +               default:
> > +                  continue;
> > +               }
> > +            }
> >  
> >              EMIT_ATTR(_TNL_ATTRIB_TEX0 + i, emit, 0);
> >              v2 |= VRTX_TEX_SET_FMT(count, SZ_TO_HW(sz));
> > 

-- 
Ville Syrjälä
Intel OTC