[Mesa-dev] [PATCH 3/3] draw: fix clipping of layer/vp index outputs

sroland at vmware.com sroland at vmware.com
Wed Dec 2 16:35:10 PST 2015


From: Roland Scheidegger <sroland at vmware.com>

This was just plain broken. It used always the value from v0 (for vp_index)
but would pass the value from the provoking vertex to later stages - but only
if there was a corresponding fs input, otherwise the layer/vp index would get
lost completely (as it would try to interpolate the (unsigned) values as
floats).
So, make it obey provoking vertex rules (drivers relying on draw will need to
do the same). And make sure that the default interpolation mode (when no
corresponding fs input is found) for them is constant.
Also, change the code a bit so constant inputs aren't interpolated then
copied over later.

No piglit change (but I'm writing one...).
---
 src/gallium/auxiliary/draw/draw_pipe_clip.c | 207 +++++++++++++++++-----------
 1 file changed, 130 insertions(+), 77 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_pipe_clip.c b/src/gallium/auxiliary/draw/draw_pipe_clip.c
index c22758b..7d9ea5e 100644
--- a/src/gallium/auxiliary/draw/draw_pipe_clip.c
+++ b/src/gallium/auxiliary/draw/draw_pipe_clip.c
@@ -58,12 +58,17 @@
 struct clip_stage {
    struct draw_stage stage;      /**< base class */
 
-   /* List of the attributes to be flatshaded. */
-   uint num_flat_attribs;
-   uint flat_attribs[PIPE_MAX_SHADER_OUTPUTS];
-
-   /* Mask of attributes in noperspective mode */
-   boolean noperspective_attribs[PIPE_MAX_SHADER_OUTPUTS];
+   unsigned pos_attr;
+
+   /* List of the attributes to be constant interpolated. */
+   uint num_const_attribs;
+   uint8_t const_attribs[PIPE_MAX_SHADER_OUTPUTS];
+   /* List of the attributes to be linear interpolated. */
+   uint num_linear_attribs;
+   uint8_t linear_attribs[PIPE_MAX_SHADER_OUTPUTS];
+   /* List of the attributes to be perspective interpolated. */
+   uint num_perspect_attribs;
+   uint8_t perspect_attribs[PIPE_MAX_SHADER_OUTPUTS];
 
    float (*plane)[4];
 };
@@ -97,9 +102,9 @@ draw_viewport_index(struct draw_context *draw,
 /* All attributes are float[4], so this is easy:
  */
 static void interp_attr( float dst[4],
-			 float t,
-			 const float in[4],
-			 const float out[4] )
+                         float t,
+                         const float in[4],
+                         const float out[4] )
 {
    dst[0] = LINTERP( t, out[0], in[0] );
    dst[1] = LINTERP( t, out[1], in[1] );
@@ -117,8 +122,8 @@ static void copy_flat( struct draw_stage *stage,
 {
    const struct clip_stage *clipper = clip_stage(stage);
    uint i;
-   for (i = 0; i < clipper->num_flat_attribs; i++) {
-      const uint attr = clipper->flat_attribs[i];
+   for (i = 0; i < clipper->num_const_attribs; i++) {
+      const uint attr = clipper->const_attribs[i];
       COPY_4FV(dst->data[attr], src->data[attr]);
    }
 }
@@ -126,15 +131,13 @@ static void copy_flat( struct draw_stage *stage,
 /* Interpolate between two vertices to produce a third.  
  */
 static void interp( const struct clip_stage *clip,
-		    struct vertex_header *dst,
-		    float t,
-		    const struct vertex_header *out, 
-		    const struct vertex_header *in,
+                    struct vertex_header *dst,
+                    float t,
+                    const struct vertex_header *out,
+                    const struct vertex_header *in,
                     unsigned viewport_index )
 {
-   const unsigned nr_attrs = draw_num_shader_outputs(clip->stage.draw);
-   const unsigned pos_attr = draw_current_shader_position_output(clip->stage.draw);
-   const unsigned clip_attr = draw_current_shader_clipvertex_output(clip->stage.draw);
+   const unsigned pos_attr = clip->pos_attr;
    unsigned j;
    float t_nopersp;
 
@@ -168,6 +171,13 @@ static void interp( const struct clip_stage *clip,
       dst->data[pos_attr][3] = oow;
    }
    
+
+   /* interp perspective attribs */
+   for (j = 0; j < clip->num_perspect_attribs; j++) {
+      const unsigned attr = clip->perspect_attribs[j];
+      interp_attr(dst->data[attr], t, in->data[attr], out->data[attr]);
+   }
+
    /**
     * Compute the t in screen-space instead of 3d space to use
     * for noperspective interpolation.
@@ -177,7 +187,7 @@ static void interp( const struct clip_stage *clip,
     * pick whatever value (the interpolated point won't be in front
     * anyway), so just use the 3d t.
     */
-   {
+   if (clip->num_linear_attribs){
       int k;
       t_nopersp = t;
       /* find either in.x != out.x or in.y != out.y */
@@ -191,24 +201,17 @@ static void interp( const struct clip_stage *clip,
             break;
          }
       }
-   }
-
-   /* Other attributes
-    */
-   for (j = 0; j < nr_attrs; j++) {
-      if (j != pos_attr && j != clip_attr) {
-         if (clip->noperspective_attribs[j])
-            interp_attr(dst->data[j], t_nopersp, in->data[j], out->data[j]);
-         else
-            interp_attr(dst->data[j], t, in->data[j], out->data[j]);
+      for (j = 0; j < clip->num_linear_attribs; j++) {
+         const unsigned attr = clip->linear_attribs[j];
+         interp_attr(dst->data[attr], t_nopersp, in->data[attr], out->data[attr]);
       }
    }
 }
 
 /**
- * Checks whether the specifed triangle is empty and if it is returns
+ * Checks whether the specified triangle is empty and if it is returns
  * true, otherwise returns false.
- * Triangle is considered null/empty if it's area is qual to zero.
+ * Triangle is considered null/empty if it's area is equal to zero.
  */
 static inline boolean
 is_tri_null(struct draw_context *draw, const struct prim_header *header)
@@ -234,10 +237,10 @@ is_tri_null(struct draw_context *draw, const struct prim_header *header)
  * will be convex and the provoking vertex will always be vertex[0].
  */
 static void emit_poly( struct draw_stage *stage,
-		       struct vertex_header **inlist,
+                       struct vertex_header **inlist,
                        const boolean *edgeflags,
-		       unsigned n,
-		       const struct prim_header *origPrim)
+                       unsigned n,
+                       const struct prim_header *origPrim)
 {
    struct prim_header header;
    unsigned i;
@@ -359,14 +362,15 @@ static inline float getclipdist(const struct clip_stage *clipper,
  */
 static void
 do_clip_tri( struct draw_stage *stage, 
-	     struct prim_header *header,
-	     unsigned clipmask )
+             struct prim_header *header,
+             unsigned clipmask )
 {
    struct clip_stage *clipper = clip_stage( stage );
    struct vertex_header *a[MAX_CLIPPED_VERTICES];
    struct vertex_header *b[MAX_CLIPPED_VERTICES];
    struct vertex_header **inlist = a;
    struct vertex_header **outlist = b;
+   struct vertex_header *prov_vertex;
    unsigned tmpnr = 0;
    unsigned n = 3;
    unsigned i;
@@ -380,7 +384,20 @@ do_clip_tri( struct draw_stage *stage,
    inlist[1] = header->v[1];
    inlist[2] = header->v[2];
 
-   viewport_index = draw_viewport_index(clipper->stage.draw, inlist[0]);
+   /*
+    * For d3d10, we need to take this from the leading (first) vertex.
+    * For GL, we could do anything (as long as we advertize
+    * GL_UNDEFINED_VERTEX for the VIEWPORT_INDEX_PROVOKING_VERTEX query),
+    * but it needs to be consistent with what other parts (i.e. driver)
+    * will do, and that seems easier with GL_PROVOKING_VERTEX logic.
+    */
+   if (stage->draw->rasterizer->flatshade_first) {
+      prov_vertex = inlist[0];
+   }
+   else {
+      prov_vertex = inlist[2];
+   }
+   viewport_index = draw_viewport_index(clipper->stage.draw, prov_vertex);
 
    if (DEBUG_CLIP) {
       const float *v0 = header->v[0]->clip;
@@ -512,10 +529,10 @@ do_clip_tri( struct draw_stage *stage,
 
    }
 
-   /* If flat-shading, copy provoking vertex color to polygon vertex[0]
+   /* If constant interpolated, copy provoking vertex attrib to polygon vertex[0]
     */
    if (n >= 3) {
-      if (clipper->num_flat_attribs) {
+      if (clipper->num_const_attribs) {
          if (stage->draw->rasterizer->flatshade_first) {
             if (inlist[0] != header->v[0]) {
                assert(tmpnr < MAX_CLIPPED_VERTICES + 1);
@@ -547,16 +564,25 @@ do_clip_tri( struct draw_stage *stage,
  */
 static void
 do_clip_line( struct draw_stage *stage,
-	      struct prim_header *header,
-	      unsigned clipmask )
+              struct prim_header *header,
+              unsigned clipmask )
 {
    const struct clip_stage *clipper = clip_stage( stage );
    struct vertex_header *v0 = header->v[0];
    struct vertex_header *v1 = header->v[1];
+   struct vertex_header *prov_vertex;
    float t0 = 0.0F;
    float t1 = 0.0F;
    struct prim_header newprim;
-   int viewport_index = draw_viewport_index(clipper->stage.draw, v0);
+   int viewport_index;
+
+   if (stage->draw->rasterizer->flatshade_first) {
+      prov_vertex = v0;
+   }
+   else {
+      prov_vertex = v1;
+   }
+   viewport_index = draw_viewport_index(clipper->stage.draw, prov_vertex);
 
    while (clipmask) {
       const unsigned plane_idx = ffs(clipmask)-1;
@@ -567,17 +593,17 @@ do_clip_line( struct draw_stage *stage,
          return; //discard nan
 
       if (dp1 < 0.0F) {
-	 float t = dp1 / (dp1 - dp0);
+         float t = dp1 / (dp1 - dp0);
          t1 = MAX2(t1, t);
       } 
 
       if (dp0 < 0.0F) {
-	 float t = dp0 / (dp0 - dp1);
+         float t = dp0 / (dp0 - dp1);
          t0 = MAX2(t0, t);
       }
 
       if (t0 + t1 >= 1.0F)
-	 return; /* discard */
+         return; /* discard */
 
       clipmask &= ~(1 << plane_idx);  /* turn off this plane's bit */
    }
@@ -668,7 +694,7 @@ clip_first_point( struct draw_stage *stage,
 
 static void
 clip_line( struct draw_stage *stage,
-	   struct prim_header *header )
+           struct prim_header *header )
 {
    unsigned clipmask = (header->v[0]->clipmask | 
                         header->v[1]->clipmask);
@@ -715,12 +741,24 @@ find_interp(const struct draw_fragment_shader *fs, int *indexed_interp,
    if (semantic_name == TGSI_SEMANTIC_COLOR ||
        semantic_name == TGSI_SEMANTIC_BCOLOR) {
       interp = indexed_interp[semantic_index];
+   } else if (semantic_name == TGSI_SEMANTIC_POSITION ||
+              semantic_name == TGSI_SEMANTIC_CLIPVERTEX) {
+      /* these inputs are handled specially always */
+      return -1;
    } else {
       /* Otherwise, search in the FS inputs, with a decent default
        * if we don't find it.
+       * This probably only matters for layer, vpindex, culldist, maybe
+       * front_face.
        */
       uint j;
-      interp = TGSI_INTERPOLATE_PERSPECTIVE;
+      if (semantic_name == TGSI_SEMANTIC_LAYER ||
+          semantic_name == TGSI_SEMANTIC_VIEWPORT_INDEX) {
+         interp = TGSI_INTERPOLATE_CONSTANT;
+      }
+      else {
+         interp = TGSI_INTERPOLATE_PERSPECTIVE;
+      }
       if (fs) {
          for (j = 0; j < fs->info.num_inputs; j++) {
             if (semantic_name == fs->info.input_semantic_name[j] &&
@@ -745,6 +783,9 @@ clip_init_state( struct draw_stage *stage )
    const struct draw_fragment_shader *fs = draw->fs.fragment_shader;
    const struct tgsi_shader_info *info = draw_get_shader_info(draw);
    uint i, j;
+   int indexed_interp[2];
+
+   clipper->pos_attr = draw_current_shader_position_output(draw);
 
    /* We need to know for each attribute what kind of interpolation is
     * done on it (flat, smooth or noperspective).  But the information
@@ -765,7 +806,6 @@ clip_init_state( struct draw_stage *stage )
    /* First pick up the interpolation mode for
     * gl_Color/gl_SecondaryColor, with the correct default.
     */
-   int indexed_interp[2];
    indexed_interp[0] = indexed_interp[1] = draw->rasterizer->flatshade ?
       TGSI_INTERPOLATE_CONSTANT : TGSI_INTERPOLATE_PERSPECTIVE;
 
@@ -778,29 +818,33 @@ clip_init_state( struct draw_stage *stage )
       }
    }
 
-   /* Then resolve the interpolation mode for every output attribute.
-    *
-    * Given how the rest of the code, the most efficient way is to
-    * have a vector of flat-mode attributes, and a mask for
-    * noperspective attributes.
-    */
+   /* Then resolve the interpolation mode for every output attribute. */
 
-   clipper->num_flat_attribs = 0;
-   memset(clipper->noperspective_attribs, 0, sizeof(clipper->noperspective_attribs));
+   clipper->num_const_attribs = 0;
+   clipper->num_linear_attribs = 0;
+   clipper->num_perspect_attribs = 0;
    for (i = 0; i < info->num_outputs; i++) {
       /* Find the interpolation mode for a specific attribute */
       int interp = find_interp(fs, indexed_interp,
                                info->output_semantic_name[i],
                                info->output_semantic_index[i]);
-      /* If it's flat, add it to the flat vector.  Otherwise update
-       * the noperspective mask.
-       */
-
-      if (interp == TGSI_INTERPOLATE_CONSTANT) {
-         clipper->flat_attribs[clipper->num_flat_attribs] = i;
-         clipper->num_flat_attribs++;
-      } else
-         clipper->noperspective_attribs[i] = interp == TGSI_INTERPOLATE_LINEAR;
+      switch (interp) {
+      case TGSI_INTERPOLATE_CONSTANT:
+         clipper->const_attribs[clipper->num_const_attribs] = i;
+         clipper->num_const_attribs++;
+         break;
+      case TGSI_INTERPOLATE_LINEAR:
+         clipper->linear_attribs[clipper->num_linear_attribs] = i;
+         clipper->num_linear_attribs++;
+         break;
+      case TGSI_INTERPOLATE_PERSPECTIVE:
+         clipper->perspect_attribs[clipper->num_perspect_attribs] = i;
+         clipper->num_perspect_attribs++;
+         break;
+      default:
+         assert(interp == -1);
+         break;
+      }
    }
    /* Search the extra vertex attributes */
    for (j = 0; j < draw->extra_shader_outputs.num; j++) {
@@ -808,16 +852,25 @@ clip_init_state( struct draw_stage *stage )
       int interp = find_interp(fs, indexed_interp,
                                draw->extra_shader_outputs.semantic_name[j],
                                draw->extra_shader_outputs.semantic_index[j]);
-      /* If it's flat, add it to the flat vector.  Otherwise update
-       * the noperspective mask.
-       */
-      if (interp == TGSI_INTERPOLATE_CONSTANT) {
-         clipper->flat_attribs[clipper->num_flat_attribs] = i + j;
-         clipper->num_flat_attribs++;
-      } else
-         clipper->noperspective_attribs[i + j] = interp == TGSI_INTERPOLATE_LINEAR;
+      switch (interp) {
+      case TGSI_INTERPOLATE_CONSTANT:
+         clipper->const_attribs[clipper->num_const_attribs] = i + j;
+         clipper->num_const_attribs++;
+         break;
+      case TGSI_INTERPOLATE_LINEAR:
+         clipper->linear_attribs[clipper->num_linear_attribs] = i + j;
+         clipper->num_linear_attribs++;
+         break;
+      case TGSI_INTERPOLATE_PERSPECTIVE:
+         clipper->perspect_attribs[clipper->num_perspect_attribs] = i + j;
+         clipper->num_perspect_attribs++;
+         break;
+      default:
+         assert(interp == -1);
+         break;
+      }
    }
-   
+
    stage->tri = clip_tri;
    stage->line = clip_line;
 }
@@ -825,14 +878,14 @@ clip_init_state( struct draw_stage *stage )
 
 
 static void clip_first_tri( struct draw_stage *stage,
-			    struct prim_header *header )
+                            struct prim_header *header )
 {
    clip_init_state( stage );
    stage->tri( stage, header );
 }
 
 static void clip_first_line( struct draw_stage *stage,
-			     struct prim_header *header )
+                             struct prim_header *header )
 {
    clip_init_state( stage );
    stage->line( stage, header );
@@ -840,7 +893,7 @@ static void clip_first_line( struct draw_stage *stage,
 
 
 static void clip_flush( struct draw_stage *stage, 
-			     unsigned flags )
+                        unsigned flags )
 {
    stage->tri = clip_first_tri;
    stage->line = clip_first_line;
-- 
2.1.4



More information about the mesa-dev mailing list