[Mesa-dev] [PATCH] i915: fallback when point sprite is enabled while handling varying inputs

[Eric Anholt]

On Thu, 8 Mar 2012 19:21:23 +0800, Yuanhan Liu <yuanhan.liu at linux.intel.com> wrote:
> From ddd1a9d8f0d82c2f5fcb78a471608a005a6a077c Mon Sep 17 00:00:00 2001
> From: Yuanhan Liu <yuanhan.liu at 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.liu at 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.

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.  If you need point sprite
coordinates and actual texcoords, a fallback should be done.

(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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120309/016ffbfa/attachment.pgp>