[Mesa-dev] [PATCH 1/2] i915: set SPRITE_POINT_ENABLE bit correctly

[Yuanhan Liu]

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).

And more,  we currently handle varying inputs as texture 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.

Thus we do set SPRITE_POINT_ENABLE bit only when all enabled tex
coord units need do CoordReplace. Or fallback is needed to make
sure the rendering is right.

As we need guarantee the CoordReplace changes handled well and
be able to fallback when finding something wrong, I added another
function to handle it at intelRunPipepline, where the drawing
happened here and tnl pipeline hasn't started yet.

With handling the bit setup at intel_validate_sprite_point_enable(),
we don't need the relative 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.

v2: fallback just when all enabled tex coord units need do
    CoordReplace(Eric).

Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
---
 src/mesa/drivers/dri/i915/i915_context.h |    1 +
 src/mesa/drivers/dri/i915/i915_state.c   |   13 +-------
 src/mesa/drivers/dri/i915/intel_tris.c   |   52 ++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/src/mesa/drivers/dri/i915/i915_context.h b/src/mesa/drivers/dri/i915/i915_context.h
index 8167137..59eeb6e 100644
--- a/src/mesa/drivers/dri/i915/i915_context.h
+++ b/src/mesa/drivers/dri/i915/i915_context.h
@@ -40,6 +40,7 @@
 #define I915_FALLBACK_POINT_SMOOTH	 0x80000
 #define I915_FALLBACK_POINT_SPRITE_COORD_ORIGIN	 0x100000
 #define I915_FALLBACK_DRAW_OFFSET	 0x200000
+#define I915_FALLBACK_COORD_REPLACE	 0x400000
 
 #define I915_UPLOAD_CTX              0x1
 #define I915_UPLOAD_BUFFERS          0x2
diff --git a/src/mesa/drivers/dri/i915/i915_state.c b/src/mesa/drivers/dri/i915/i915_state.c
index 756001f..3c751e4 100644
--- a/src/mesa/drivers/dri/i915/i915_state.c
+++ b/src/mesa/drivers/dri/i915/i915_state.c
@@ -869,18 +869,7 @@ i915Enable(struct gl_context * ctx, GLenum cap, GLboolean state)
       break;
 
    case GL_POINT_SPRITE:
-      /* This state change is handled in i915_reduced_primitive_state because
-       * the hardware bit should only be set when rendering points.
-       */
-	 dw = i915->state.Ctx[I915_CTXREG_LIS4];
-      if (state)
-	 dw |= S4_SPRITE_POINT_ENABLE;
-      else
-	 dw &= ~S4_SPRITE_POINT_ENABLE;
-      if (dw != i915->state.Ctx[I915_CTXREG_LIS4]) {
-	 i915->state.Ctx[I915_CTXREG_LIS4] = dw;
-	 I915_STATECHANGE(i915, I915_UPLOAD_CTX);
-      }
+      /* Handle it at intel_validate_sprite_point_enable() */
       break;
 
    case GL_POINT_SMOOTH:
diff --git a/src/mesa/drivers/dri/i915/intel_tris.c b/src/mesa/drivers/dri/i915/intel_tris.c
index a36011a..58f6a59 100644
--- a/src/mesa/drivers/dri/i915/intel_tris.c
+++ b/src/mesa/drivers/dri/i915/intel_tris.c
@@ -1052,6 +1052,48 @@ static const GLenum reduced_prim[GL_POLYGON + 1] = {
    GL_TRIANGLES
 };
 
+static void
+intel_validate_sprite_point_enable(struct intel_context *intel)
+{
+   struct gl_context *ctx = &intel->ctx;
+   struct i915_fragment_program *p =
+      (struct i915_fragment_program *) ctx->FragmentProgram._Current;
+   const GLbitfield64 inputsRead = p->FragProg.Base.InputsRead;
+   struct i915_context *i915 = i915_context(ctx);
+   GLuint s4 = i915->state.Ctx[I915_CTXREG_LIS4] & ~S4_VFMT_MASK;
+   int i;
+   GLuint coord_replace_bits = 0x0;
+   GLuint tex_coord_unit_bits = 0x0;
+
+   for (i = 0; i < ctx->Const.MaxTextureCoordUnits; i++) {
+      if (ctx->Point.CoordReplace[i] && ctx->Point.PointSprite)
+         coord_replace_bits |= (1 << i);
+      if (inputsRead & FRAG_BIT_TEX(i))
+         tex_coord_unit_bits |= (1 << i);
+   }
+
+   s4 &= ~S4_SPRITE_POINT_ENABLE;
+   if (coord_replace_bits) {
+      if (coord_replace_bits != tex_coord_unit_bits) {
+         /*
+          * Here we can't enable the SPRITE_POINT_ENABLE bit due to the
+          * mis-match of tex_coord_unit_bits and coord_replace_bits, or
+          * this will make all the other non-point-sprite coords be
+          * replaced to value (0, 0)-(1, 1).
+          *
+          * Thus, a fallback is needed.
+          */
+         FALLBACK(intel, I915_FALLBACK_COORD_REPLACE, true);
+      } else {
+         s4 |= S4_SPRITE_POINT_ENABLE;
+      }
+   }
+
+   if (s4 != i915->state.Ctx[I915_CTXREG_LIS4]) {
+      i915->state.Ctx[I915_CTXREG_LIS4] = s4;
+      I915_STATECHANGE(i915, I915_UPLOAD_CTX);
+   }
+}
 
 /**********************************************************************/
 /*                 High level hooks for t_vb_render.c                 */
@@ -1070,6 +1112,15 @@ intelRunPipeline(struct gl_context * ctx)
    if (ctx->NewState)
       _mesa_update_state_locked(ctx);
 
+   /*
+    * Enable POINT_SPRITE_ENABLE bit when needed here
+    *
+    * Handle it at _draw_ time so that we can guarantee the CoordReplace
+    * changes handled well. And we must do it before the tnl pipeline is
+    * running so that we can fallback when finding something goes wrong.
+    */
+   intel_validate_sprite_point_enable(intel);
+
    /* We need to get this done before we start the pipeline, or a
     * change in the INTEL_FALLBACK() of its intel_draw_buffers() call
     * while the pipeline is running will result in mismatched swrast
@@ -1198,6 +1249,7 @@ static char *fallbackStrings[] = {
    [19] = "Smooth point",
    [20] = "point sprite coord origin",
    [21] = "depth/color drawing offset",
+   [22] = "coord replace(SPRITE POINT ENABLE)",
 };
 
 
-- 
1.7.7