llvmpipe: Fix perspective interpolation for point sprites.

Once a fragment is generated with LP_INTERP_PERSPECTIVE set for an input,
it will do a divide by w for that input. Therefore it's not OK to treat LP_INTERP_PERSPECTIVE as
LP_INTERP_LINEAR or vice-versa, even if the attribute is known to not
vary.

A better strategy would be to take the primitive in consideration when
generating the fragment shader key, and therefore avoid the per-fragment
perspective divide.

1 files changed, 54 insertions, 17 deletions

diff --git a/src/gallium/drivers/llvmpipe/lp_setup_point.c b/src/gallium/drivers/llvmpipe/lp_setup_point.c
index 3b217f95447..c91e85f9159 100644
--- a/src/gallium/drivers/llvmpipe/lp_setup_point.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup_point.c
@@ -64,12 +64,37 @@ constant_coef(struct lp_setup_context *setup,
 }
 
 
+static void
+point_persp_coeff(struct lp_setup_context *setup,
+                  struct lp_rast_triangle *point,
+                  const struct point_info *info,
+                  unsigned slot,
+                  unsigned i)
+{
+   /*
+    * Fragment shader expects pre-multiplied w for LP_INTERP_PERSPECTIVE. A
+    * better stratergy would be to take the primitive in consideration when
+    * generating the fragment shader key, and therefore avoid the per-fragment
+    * perspective divide.
+    */
+
+   float w0 = info->v0[0][3];
+
+   assert(i < 4);
+
+   point->inputs.a0[slot][i] = info->v0[slot][i]*w0;
+   point->inputs.dadx[slot][i] = 0.0f;
+   point->inputs.dady[slot][i] = 0.0f;
+}
+
+
 /**
  * Setup automatic texcoord coefficients (for sprite rendering).
  * \param slot  the vertex attribute slot to setup
  * \param i  the attribute channel in [0,3]
  * \param sprite_coord_origin  one of PIPE_SPRITE_COORD_x
- * \param perspective_proj  will the TEX instruction do a divide by Q?
+ * \param perspective  does the shader expects pre-multiplied w, i.e.,
+ *    LP_INTERP_PERSPECTIVE is specified in the shader key
  */
 static void
 texcoord_coef(struct lp_setup_context *setup,
@@ -78,7 +103,7 @@ texcoord_coef(struct lp_setup_context *setup,
               unsigned slot,
               unsigned i,
               unsigned sprite_coord_origin,
-              boolean perspective_proj)
+              boolean perspective)
 {
    assert(i < 4);
 
@@ -92,7 +117,7 @@ texcoord_coef(struct lp_setup_context *setup,
       point->inputs.dady[slot][0] = dady;
       point->inputs.a0[slot][0] = 0.5 - (dadx * x0 + dady * y0);
 
-      if (!perspective_proj) {
+      if (perspective) {
          /* Divide coefficients by vertex.w here.
           *
           * It would be clearer to always multiply by w0 above and
@@ -119,7 +144,7 @@ texcoord_coef(struct lp_setup_context *setup,
       point->inputs.dady[slot][1] = dady;
       point->inputs.a0[slot][1] = 0.5 - (dadx * x0 + dady * y0);
 
-      if (!perspective_proj) {
+      if (perspective) {
          float w0 = info->v0[0][3];
          point->inputs.dadx[slot][1] *= w0;
          point->inputs.dady[slot][1] *= w0;
@@ -193,11 +218,17 @@ setup_point_coefficients( struct lp_setup_context *setup,
    /* setup interpolation for all the remaining attributes:
     */
    for (slot = 0; slot < setup->fs.nr_inputs; slot++) {
+      enum lp_interp interp = setup->fs.input[slot].interp;
+      boolean perspective = !!(interp == LP_INTERP_PERSPECTIVE);
       unsigned vert_attr = setup->fs.input[slot].src_index;
       unsigned usage_mask = setup->fs.input[slot].usage_mask;
       unsigned i;
+
+      if (perspective & usage_mask) {
+         fragcoord_usage_mask |= TGSI_WRITEMASK_W;
+      }
       
-      switch (setup->fs.input[slot].interp) {
+      switch (interp) {
       case LP_INTERP_POSITION:
          /*
           * The generated pixel interpolators will pick up the coeffs from
@@ -210,32 +241,38 @@ setup_point_coefficients( struct lp_setup_context *setup,
       case LP_INTERP_LINEAR:
          /* Sprite tex coords may use linear interpolation someday */
          /* fall-through */
-
       case LP_INTERP_PERSPECTIVE:
          /* check if the sprite coord flag is set for this attribute.
           * If so, set it up so it up so x and y vary from 0 to 1.
           */
          if (shader->info.input_semantic_name[slot] == TGSI_SEMANTIC_GENERIC) {
-            const int index = shader->info.input_semantic_index[slot];
+            unsigned semantic_index = shader->info.input_semantic_index[slot];
             /* Note that sprite_coord enable is a bitfield of
              * PIPE_MAX_SHADER_OUTPUTS bits.
              */
-            if (index < PIPE_MAX_SHADER_OUTPUTS &&
-                (setup->sprite_coord_enable & (1 << index))) {
-               for (i = 0; i < NUM_CHANNELS; i++)
-                  if (usage_mask & (1 << i))
+            if (semantic_index < PIPE_MAX_SHADER_OUTPUTS &&
+                (setup->sprite_coord_enable & (1 << semantic_index))) {
+               for (i = 0; i < NUM_CHANNELS; i++) {
+                  if (usage_mask & (1 << i)) {
                      texcoord_coef(setup, point, info, slot + 1, i,
                                    setup->sprite_coord_origin,
-                                   (usage_mask & TGSI_WRITEMASK_W));
-               fragcoord_usage_mask |= TGSI_WRITEMASK_W;
-               break;                     
+                                   perspective);
+                  }
+               }
+               break;
             }
          }
-         /* FALLTHROUGH */
+         /* fall-through */
       case LP_INTERP_CONSTANT:
          for (i = 0; i < NUM_CHANNELS; i++) {
-            if (usage_mask & (1 << i))
-               constant_coef(setup, point, slot+1, info->v0[vert_attr][i], i);
+            if (usage_mask & (1 << i)) {
+               if (perspective) {
+                  point_persp_coeff(setup, point, info, slot+1, i);
+               }
+               else {
+                  constant_coef(setup, point, slot+1, info->v0[vert_attr][i], i);
+               }
+            }
          }
          break;