On Fri, Mar 09, 2012 at 10:35:33AM -0800, Eric Anholt wrote:
> On Thu, 8 Mar 2012 19:21:23 +0800, Yuanhan Liu <yuanhan....@linux.intel.com>
> wrote:
> > From ddd1a9d8f0d82c2f5fcb78a471608a005a6a077c Mon Sep 17 00:00:00 2001
> > From: Yuanhan Liu <yuanhan....@linux.intel.com>
> > Date: Thu, 8 Mar 2012 18:48:54 +0800
> > Subject: [PATCH] i915: set SPRITE_POINT_ENABLE bit just when we need do
> > coord
> > replace
> >
> > When SPRITE_POINT_ENABLE bit is set, the texture coord would be
> > replaced, and this is only needed when we called something like
> > glTexEnvi(GL_POINT_SPRITE, GL_COORD_REPLACE, GL_TRUE).
> >
> > Since we currently handling varying inputs as tex coord, we would be
> > careful when setting this bit and set it just when needed, or you will
> > find the value of varying input is not right and changed.
> >
> > With handling the bit setup at i915ValidateFragmentProgram, we don't
> > need the code at i915Enable then.
> >
> > This patch would _really_ fix the webglc point-size.html test case and
> > of course, not regress piglit point-sprite and glean-pointSprite testcase.
> >
> > NOTE: This is a candidate for stable release branches.
> >
> > Signed-off-by: Yuanhan Liu <yuanhan....@linux.intel.com>
> > ---
> > src/mesa/drivers/dri/i915/i915_fragprog.c | 5 +++++
> > src/mesa/drivers/dri/i915/i915_state.c | 13 +------------
> > 2 files changed, 6 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c
> > b/src/mesa/drivers/dri/i915/i915_fragprog.c
> > index 5b7e93e..8829e8d 100644
> > --- a/src/mesa/drivers/dri/i915/i915_fragprog.c
> > +++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
> > @@ -1379,7 +1379,12 @@ i915ValidateFragmentProgram(struct i915_context
> > *i915)
> > EMIT_ATTR(_TNL_ATTRIB_FOG, EMIT_1F, S4_VFMT_FOG_PARAM, 4);
> > }
> >
> > + s4 &= ~S4_SPRITE_POINT_ENABLE;
> > for (i = 0; i < p->ctx->Const.MaxTextureCoordUnits; i++) {
> > + /* Do set SPRITE_POINT_ENABLE bit when we need do coord replace */
> > + if (ctx->Point.CoordReplace[i] && ctx->Point.PointSprite)
> > + s4 |= S4_SPRITE_POINT_ENABLE;
> > +
> > if (inputsRead & FRAG_BIT_TEX(i)) {
> > int sz = VB->AttribPtr[_TNL_ATTRIB_TEX0 + i]->size;
>
> I don't think you've done anything to guarantee that this code is called
> when CoordReplace changes.
Yes, you are right. Maybe we can do it at the final _draw_ time, say
i915_reduced_primitive_state?
>
> A more general problem: you're turning on point sprite if coord replace
> is set on any texcoord. i915 replaces all texcoords with (0,0)-(1,1)
> when point sprite is enabled, which breaks any non-point-sprite
> coordinates, plus varyings as you noted.
Well, I guess that's also what the current code do. Since
SPRITE_POINT_ENABLE is just _one_ bit instead of a set of mask bits.
> If you need point sprite
> coordinates and actual texcoords, a fallback should be done.
Yes, agreed acoording to the above state.
>
> (Well, if we did better compiling, we could route a couple of varyings
> through color/secondarycolor while still getting point sprite
> coordinates on the texcoords)
That's a good hint. But I'd like to do the fallback first. Since I don't
know how many works should be done to get the better compiling.
Thanks,
Yuanhan Liu
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
