[Intel-gfx] [PATCH 1/5] intel gen4/5: fix GL_VERTEX_PROGRAM_TWO_SIDE.

Olivier Galibert galibert at pobox.com
Sat Jun 30 20:50:11 CEST 2012


There was... confusion about which register goes where.  With that
patch urb_setup is in line with the vue setup, even when these
annoying backcolor slots are used.  And in addition the stray mov into
lalaland is avoided when only one of the front/back slots is used and
the backface is looking at you.  The code instead picks whatever slot
was written to by the vertex shader.  That makes most of the generated
piglit tests useless to test the backface selection though.

Signed-off-by: Olivier Galibert <galibert at pobox.com>
---
 src/mesa/drivers/dri/i965/brw_fs.cpp     |   18 +++++-
 src/mesa/drivers/dri/i965/brw_sf.c       |    3 +-
 src/mesa/drivers/dri/i965/brw_sf_emit.c  |   93 +++++++++++++++++-------------
 src/mesa/drivers/dri/i965/brw_wm_pass2.c |   19 +++++-
 4 files changed, 89 insertions(+), 44 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 6cef08a..710f2ff 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -721,8 +721,24 @@ fs_visitor::calculate_urb_setup()
 	 if (c->key.vp_outputs_written & BITFIELD64_BIT(i)) {
 	    int fp_index = _mesa_vert_result_to_frag_attrib((gl_vert_result) i);
 
+            /* Special case: two-sided vertex option, vertex program
+             * only writes to the back color.  Map it to the
+             * associated front color location.
+             */
+            if (i >= VERT_RESULT_BFC0 && i <= VERT_RESULT_BFC1 &&
+                ctx->VertexProgram._TwoSideEnabled &&
+                urb_setup[i - VERT_RESULT_BFC0 + FRAG_ATTRIB_COL0] == -1)
+               fp_index = i - VERT_RESULT_BFC0 + FRAG_ATTRIB_COL0;
+
+	    /* The back color slot is skipped when the front color is
+	     * also written to.  In addition, some slots can be
+	     * written in the vertex shader and not read in the
+	     * fragment shader.  So the register number must always be
+	     * incremented, mapped or not.
+	     */
 	    if (fp_index >= 0)
-	       urb_setup[fp_index] = urb_next++;
+	       urb_setup[fp_index] = urb_next;
+	    urb_next++;
 	 }
       }
 
diff --git a/src/mesa/drivers/dri/i965/brw_sf.c b/src/mesa/drivers/dri/i965/brw_sf.c
index 23a874a..7867ab5 100644
--- a/src/mesa/drivers/dri/i965/brw_sf.c
+++ b/src/mesa/drivers/dri/i965/brw_sf.c
@@ -192,7 +192,8 @@ brw_upload_sf_prog(struct brw_context *brw)
 
    /* _NEW_LIGHT */
    key.do_flat_shading = (ctx->Light.ShadeModel == GL_FLAT);
-   key.do_twoside_color = (ctx->Light.Enabled && ctx->Light.Model.TwoSide);
+   key.do_twoside_color = (ctx->Light.Enabled && ctx->Light.Model.TwoSide) ||
+     ctx->VertexProgram._TwoSideEnabled;
 
    /* _NEW_POLYGON */
    if (key.do_twoside_color) {
diff --git a/src/mesa/drivers/dri/i965/brw_sf_emit.c b/src/mesa/drivers/dri/i965/brw_sf_emit.c
index ff6383b..9d8aa38 100644
--- a/src/mesa/drivers/dri/i965/brw_sf_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_sf_emit.c
@@ -79,24 +79,9 @@ have_attr(struct brw_sf_compile *c, GLuint attr)
 /*********************************************************************** 
  * Twoside lighting
  */
-static void copy_bfc( struct brw_sf_compile *c,
-		      struct brw_reg vert )
-{
-   struct brw_compile *p = &c->func;
-   GLuint i;
-
-   for (i = 0; i < 2; i++) {
-      if (have_attr(c, VERT_RESULT_COL0+i) &&
-	  have_attr(c, VERT_RESULT_BFC0+i))
-	 brw_MOV(p, 
-		 get_vert_result(c, vert, VERT_RESULT_COL0+i),
-		 get_vert_result(c, vert, VERT_RESULT_BFC0+i));
-   }
-}
-
-
 static void do_twoside_color( struct brw_sf_compile *c )
 {
+   GLuint i, need_0, need_1;
    struct brw_compile *p = &c->func;
    GLuint backface_conditional = c->key.frontface_ccw ? BRW_CONDITIONAL_G : BRW_CONDITIONAL_L;
 
@@ -105,12 +90,14 @@ static void do_twoside_color( struct brw_sf_compile *c )
    if (c->key.primitive == SF_UNFILLED_TRIS)
       return;
 
-   /* XXX: What happens if BFC isn't present?  This could only happen
-    * for user-supplied vertex programs, as t_vp_build.c always does
-    * the right thing.
+   /* If the vertex shader provides both front and backface color, do
+    * the selection.  Otherwise the generated code will pick up
+    * whichever there is.
     */
-   if (!(have_attr(c, VERT_RESULT_COL0) && have_attr(c, VERT_RESULT_BFC0)) &&
-       !(have_attr(c, VERT_RESULT_COL1) && have_attr(c, VERT_RESULT_BFC1)))
+   need_0 = have_attr(c, VERT_RESULT_COL0) && have_attr(c, VERT_RESULT_BFC0);
+   need_1 = have_attr(c, VERT_RESULT_COL1) && have_attr(c, VERT_RESULT_BFC1);
+
+   if (!need_0 && !need_1)
       return;
    
    /* Need to use BRW_EXECUTE_4 and also do an 4-wide compare in order
@@ -121,12 +108,15 @@ static void do_twoside_color( struct brw_sf_compile *c )
    brw_push_insn_state(p);
    brw_CMP(p, vec4(brw_null_reg()), backface_conditional, c->det, brw_imm_f(0));
    brw_IF(p, BRW_EXECUTE_4);
-   {
-      switch (c->nr_verts) {
-      case 3: copy_bfc(c, c->vert[2]);
-      case 2: copy_bfc(c, c->vert[1]);
-      case 1: copy_bfc(c, c->vert[0]);
-      }
+   for (i=0; i<c->nr_verts; i++) {
+      if (need_0)
+	 brw_MOV(p, 
+		 get_vert_result(c, c->vert[i], VERT_RESULT_COL0),
+		 get_vert_result(c, c->vert[i], VERT_RESULT_BFC0));
+      if (need_1)
+	 brw_MOV(p, 
+		 get_vert_result(c, c->vert[i], VERT_RESULT_COL1),
+		 get_vert_result(c, c->vert[i], VERT_RESULT_BFC1));
    }
    brw_ENDIF(p);
    brw_pop_insn_state(p);
@@ -139,20 +129,27 @@ static void do_twoside_color( struct brw_sf_compile *c )
  */
 
 #define VERT_RESULT_COLOR_BITS (BITFIELD64_BIT(VERT_RESULT_COL0) | \
-				BITFIELD64_BIT(VERT_RESULT_COL1))
+                                BITFIELD64_BIT(VERT_RESULT_COL1))
 
 static void copy_colors( struct brw_sf_compile *c,
 		     struct brw_reg dst,
-		     struct brw_reg src)
+                     struct brw_reg src,
+                     int allow_twoside)
 {
    struct brw_compile *p = &c->func;
    GLuint i;
 
    for (i = VERT_RESULT_COL0; i <= VERT_RESULT_COL1; i++) {
-      if (have_attr(c,i))
+      if (have_attr(c,i)) {
 	 brw_MOV(p, 
 		 get_vert_result(c, dst, i),
 		 get_vert_result(c, src, i));
+
+      } else if(allow_twoside && have_attr(c, i - VERT_RESULT_COL0 + VERT_RESULT_BFC0)) {
+	 brw_MOV(p, 
+		 get_vert_result(c, dst, i - VERT_RESULT_COL0 + VERT_RESULT_BFC0),
+		 get_vert_result(c, src, i - VERT_RESULT_COL0 + VERT_RESULT_BFC0));
+      }
    }
 }
 
@@ -167,9 +164,19 @@ static void do_flatshade_triangle( struct brw_sf_compile *c )
    struct brw_compile *p = &c->func;
    struct intel_context *intel = &p->brw->intel;
    struct brw_reg ip = brw_ip_reg();
-   GLuint nr = _mesa_bitcount_64(c->key.attrs & VERT_RESULT_COLOR_BITS);
    GLuint jmpi = 1;
 
+   GLuint nr;
+
+   if (c->key.do_twoside_color) {
+      nr = ((c->key.attrs & (BITFIELD64_BIT(VERT_RESULT_COL0) | BITFIELD64_BIT(VERT_RESULT_BFC0))) != 0) +
+         ((c->key.attrs & (BITFIELD64_BIT(VERT_RESULT_COL1) | BITFIELD64_BIT(VERT_RESULT_BFC1))) != 0);
+
+   } else {
+      nr = ((c->key.attrs & BITFIELD64_BIT(VERT_RESULT_COL0)) != 0) +
+         ((c->key.attrs & BITFIELD64_BIT(VERT_RESULT_COL1)) != 0);
+   }
+
    if (!nr)
       return;
 
@@ -186,16 +193,16 @@ static void do_flatshade_triangle( struct brw_sf_compile *c )
    brw_MUL(p, c->pv, c->pv, brw_imm_d(jmpi*(nr*2+1)));
    brw_JMPI(p, ip, ip, c->pv);
 
-   copy_colors(c, c->vert[1], c->vert[0]);
-   copy_colors(c, c->vert[2], c->vert[0]);
+   copy_colors(c, c->vert[1], c->vert[0], c->key.do_twoside_color);
+   copy_colors(c, c->vert[2], c->vert[0], c->key.do_twoside_color);
    brw_JMPI(p, ip, ip, brw_imm_d(jmpi*(nr*4+1)));
 
-   copy_colors(c, c->vert[0], c->vert[1]);
-   copy_colors(c, c->vert[2], c->vert[1]);
+   copy_colors(c, c->vert[0], c->vert[1], c->key.do_twoside_color);
+   copy_colors(c, c->vert[2], c->vert[1], c->key.do_twoside_color);
    brw_JMPI(p, ip, ip, brw_imm_d(jmpi*nr*2));
 
-   copy_colors(c, c->vert[0], c->vert[2]);
-   copy_colors(c, c->vert[1], c->vert[2]);
+   copy_colors(c, c->vert[0], c->vert[2], c->key.do_twoside_color);
+   copy_colors(c, c->vert[1], c->vert[2], c->key.do_twoside_color);
 
    brw_pop_insn_state(p);
 }
@@ -224,10 +231,10 @@ static void do_flatshade_line( struct brw_sf_compile *c )
    
    brw_MUL(p, c->pv, c->pv, brw_imm_d(jmpi*(nr+1)));
    brw_JMPI(p, ip, ip, c->pv);
-   copy_colors(c, c->vert[1], c->vert[0]);
+   copy_colors(c, c->vert[1], c->vert[0], 0);
 
    brw_JMPI(p, ip, ip, brw_imm_ud(jmpi*nr));
-   copy_colors(c, c->vert[0], c->vert[1]);
+   copy_colors(c, c->vert[0], c->vert[1], 0);
 
    brw_pop_insn_state(p);
 }
@@ -337,13 +344,17 @@ calculate_masks(struct brw_sf_compile *c,
    if (c->key.do_flat_shading)
       persp_mask = c->key.attrs & ~(BITFIELD64_BIT(VERT_RESULT_HPOS) |
                                     BITFIELD64_BIT(VERT_RESULT_COL0) |
-                                    BITFIELD64_BIT(VERT_RESULT_COL1));
+                                    BITFIELD64_BIT(VERT_RESULT_COL1) |
+                                    BITFIELD64_BIT(VERT_RESULT_BFC0) |
+                                    BITFIELD64_BIT(VERT_RESULT_BFC1));
    else
       persp_mask = c->key.attrs & ~(BITFIELD64_BIT(VERT_RESULT_HPOS));
 
    if (c->key.do_flat_shading)
       linear_mask = c->key.attrs & ~(BITFIELD64_BIT(VERT_RESULT_COL0) |
-                                     BITFIELD64_BIT(VERT_RESULT_COL1));
+                                     BITFIELD64_BIT(VERT_RESULT_COL1) |
+                                     BITFIELD64_BIT(VERT_RESULT_BFC0) |
+                                     BITFIELD64_BIT(VERT_RESULT_BFC1));
    else
       linear_mask = c->key.attrs;
 
diff --git a/src/mesa/drivers/dri/i965/brw_wm_pass2.c b/src/mesa/drivers/dri/i965/brw_wm_pass2.c
index 27c0a94..48143f3 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_pass2.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_pass2.c
@@ -96,9 +96,26 @@ static void init_registers( struct brw_wm_compile *c )
 	 if (c->key.vp_outputs_written & BITFIELD64_BIT(j)) {
 	    int fp_index = _mesa_vert_result_to_frag_attrib(j);
 
+            /* Special case: two-sided vertex option, vertex program
+             * only writes to the back color.  Map it to the
+             * associated front color location.
+             */
+            if (j >= VERT_RESULT_BFC0 && j <= VERT_RESULT_BFC1 &&
+                intel->ctx.VertexProgram._TwoSideEnabled &&
+                !(c->key.vp_outputs_written & BITFIELD64_BIT(j - VERT_RESULT_BFC0 + VERT_RESULT_COL0)))
+               fp_index = j - VERT_RESULT_BFC0 + FRAG_ATTRIB_COL0;
+
 	    nr_interp_regs++;
+
+	    /* The back color slot is skipped when the front color is
+	     * also written to.  In addition, some slots can be
+	     * written in the vertex shader and not read in the
+	     * fragment shader.  So the register number must always be
+	     * incremented, mapped or not.
+	     */
 	    if (fp_index >= 0)
-	       prealloc_reg(c, &c->payload.input_interp[fp_index], i++);
+	       prealloc_reg(c, &c->payload.input_interp[fp_index], i);
+            i++;
 	 }
       }
       assert(nr_interp_regs >= 1);
-- 
1.7.10.rc3.1.gb306




More information about the Intel-gfx mailing list