(no subject)

(no subject)

[Olivier Galibert]

  Hi,

This is the second verion of the clipping/interpolation patches.

Main differences:
- I tried to take all of Paul's remarks into account
- I exploded the first patch in 4 independant ones
- I've added a patch to ensure that integers pass through unscathed

Patch 4/9 is (slightly) controversial.  There may be better ways to do
it, or at least more general ones.  But it's simple, it works, and it
allows to validate the other 8.  It's an easy one to revert if we
build an alternative.

Best,

  OG.
 
[PATCH 1/9] intel gen4-5: fix the vue view in the fs.
[PATCH 2/9] intel gen4-5: simplify the bfc copy in the sf.
[PATCH 3/9] intel gen4-5: fix GL_VERTEX_PROGRAM_TWO_SIDE selection.
[PATCH 4/9] intel gen4-5: Fix backface/frontface selection when one
[PATCH 5/9] intel gen4-5: Compute the interpolation status for every
[PATCH 6/9] intel gen4-5: Correctly setup the parameters in the sf.
[PATCH 7/9] intel gen4-5: Correctly handle flat vs. non-flat in the
[PATCH 8/9] intel gen4-5: Make noperspective clipping work.
[PATCH 9/9] intel gen4-5: Don't touch flatshaded values when

[PATCH 1/9] intel gen4-5: fix the vue view in the fs.

[Olivier Galibert]

In some cases the fragment shader view of the vue registers was out of
sync with the builder.  This fixes it.

Signed-off-by: Olivier Galibert <galibert@pobox.com>
---
 src/mesa/drivers/dri/i965/brw_fs.cpp     |    9 ++++++++-
 src/mesa/drivers/dri/i965/brw_wm_pass2.c |   10 +++++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index b3b25cc..3f98137 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -972,8 +972,15 @@ 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);
 
+	    /* 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_wm_pass2.c b/src/mesa/drivers/dri/i965/brw_wm_pass2.c
index 27c0a94..eacf7c0 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_pass2.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_pass2.c
@@ -97,8 +97,16 @@ static void init_registers( struct brw_wm_compile *c )
 	    int fp_index = _mesa_vert_result_to_frag_attrib(j);
 
 	    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.280.gaa39

Re: [Mesa-dev] [PATCH 1/9] intel gen4-5: fix the vue view in the fs.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 408 bytes --]

Olivier Galibert <galibert@pobox.com> writes:

> In some cases the fragment shader view of the vue registers was out of
> sync with the builder.  This fixes it.

s/builder/SF outputs/ ?

I'd love to see the pre-gen6 code get rearranged so the FS walked the
bitfield of FS inputs from SF and chose the urb offset for each.  But
this does look like the minimal fix.

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Mesa-dev] [PATCH 1/9] intel gen4-5: fix the vue view in the fs.

[Olivier Galibert]

On Thu, Jul 26, 2012 at 10:18:01AM -0700, Eric Anholt wrote:
> Olivier Galibert <galibert@pobox.com> writes:
> 
> > In some cases the fragment shader view of the vue registers was out of
> > sync with the builder.  This fixes it.
> 
> s/builder/SF outputs/ ?
> 
> I'd love to see the pre-gen6 code get rearranged so the FS walked the
> bitfield of FS inputs from SF and chose the urb offset for each.  But
> this does look like the minimal fix.

In other words, an explicit linking pass?  That could be useful with
geometry shaders, too.

  OG.

[PATCH 2/9] intel gen4-5: simplify the bfc copy in the sf.

[Olivier Galibert]

This patch is mostly designed to make followup patches simpler, but
it's a simplification by itself.

Signed-off-by: Olivier Galibert <galibert@pobox.com>
---
 src/mesa/drivers/dri/i965/brw_sf_emit.c |   93 +++++++++++++++++--------------
 1 file changed, 52 insertions(+), 41 deletions(-)

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;
 
-- 
1.7.10.280.gaa39

Re: [PATCH 2/9] intel gen4-5: simplify the bfc copy in the sf.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 961 bytes --]

Olivier Galibert <galibert@pobox.com> writes:
> @@ -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++) {

We tend to put spaces around our binary operators.

> +      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));

trim trailing whitespace.

Other than that,

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/9] intel gen4-5: fix GL_VERTEX_PROGRAM_TWO_SIDE selection.

[Olivier Galibert]

Previous code only selected two side in pure fixed-function setups.
This version also activates it when needed with shaders programs.

Signed-off-by: Olivier Galibert <galibert@pobox.com>
---
 src/mesa/drivers/dri/i965/brw_sf.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_sf.c b/src/mesa/drivers/dri/i965/brw_sf.c
index 23a874a..791210f 100644
--- a/src/mesa/drivers/dri/i965/brw_sf.c
+++ b/src/mesa/drivers/dri/i965/brw_sf.c
@@ -192,7 +192,7 @@ 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->VertexProgram._TwoSideEnabled;
 
    /* _NEW_POLYGON */
    if (key.do_twoside_color) {
-- 
1.7.10.280.gaa39

Re: [PATCH 3/9] intel gen4-5: fix GL_VERTEX_PROGRAM_TWO_SIDE selection.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1051 bytes --]

Olivier Galibert <galibert@pobox.com> writes:

> Previous code only selected two side in pure fixed-function setups.
> This version also activates it when needed with shaders programs.
>
> Signed-off-by: Olivier Galibert <galibert@pobox.com>
> ---
>  src/mesa/drivers/dri/i965/brw_sf.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_sf.c b/src/mesa/drivers/dri/i965/brw_sf.c
> index 23a874a..791210f 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf.c
> @@ -192,7 +192,7 @@ 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->VertexProgram._TwoSideEnabled;

ctx->VertexProgram._TwoSideEnabled is also changed when _NEW_PROGRAM is
set, so that should be noted in the _NEW_LIGHT comment above and
included in brw_sf_prog.dirty.mesa.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/9] intel gen4-5: Fix backface/frontface selection when one one color is written to.

[Olivier Galibert]

Shaders, piglit test ones in particular, may write only to one of
gl_FrontColor/gl_BackColor.  The standard is unclear on whether the
behaviour is defined in that case, but it seems reasonable to support
it.

The choice done there to pick up whichever color was actually written
to.  That makes most of the generated piglit tests useless to test the
backface selection, but it's simple and it works.

Signed-off-by: Olivier Galibert <galibert@pobox.com>
---
 src/mesa/drivers/dri/i965/brw_fs.cpp     |    9 +++++++++
 src/mesa/drivers/dri/i965/brw_wm_pass2.c |    9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 3f98137..3b62952 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -972,6 +972,15 @@ 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
diff --git a/src/mesa/drivers/dri/i965/brw_wm_pass2.c b/src/mesa/drivers/dri/i965/brw_wm_pass2.c
index eacf7c0..48143f3 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_pass2.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_pass2.c
@@ -96,6 +96,15 @@ 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
-- 
1.7.10.280.gaa39

Re: [PATCH 4/9] intel gen4-5: Fix backface/frontface selection when one one color is written to.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1951 bytes --]

Olivier Galibert <galibert@pobox.com> writes:

> Shaders, piglit test ones in particular, may write only to one of
> gl_FrontColor/gl_BackColor.  The standard is unclear on whether the
> behaviour is defined in that case, but it seems reasonable to support
> it.
>
> The choice done there to pick up whichever color was actually written
> to.  That makes most of the generated piglit tests useless to test the
> backface selection, but it's simple and it works.
>
> Signed-off-by: Olivier Galibert <galibert@pobox.com>
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp     |    9 +++++++++
>  src/mesa/drivers/dri/i965/brw_wm_pass2.c |    9 +++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 3f98137..3b62952 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -972,6 +972,15 @@ 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;

In the fs_visitor (and brw_wm_pass*), you don't get to look at ctx->
state like that -- you're getting called once with some set of ctx
state, but the program will get reused even if the ctx state changes.
You'd have to get that state into the wm prog key, and use that, which
would guarantee that you have the appropriate program code.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/9] intel gen4-5: Fix backface/frontface selection when one one color is written to.

[Olivier Galibert]

On Fri, Jul 20, 2012 at 10:01:03AM -0700, Eric Anholt wrote:
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > index 3f98137..3b62952 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > @@ -972,6 +972,15 @@ 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;
> 
> In the fs_visitor (and brw_wm_pass*), you don't get to look at ctx->
> state like that -- you're getting called once with some set of ctx
> state, but the program will get reused even if the ctx state changes.
> You'd have to get that state into the wm prog key, and use that, which
> would guarantee that you have the appropriate program code.

Ok.  OTOH, we don't actually *need* to look at TwoSideEnabled.  If the
rest of the condition triggers it's either correct or undefined
behaviour.  So we can do it systematically.

  OG.

[PATCH 5/9] intel gen4-5: Compute the interpolation status for every variable in one place.

[Olivier Galibert]

The program keys are updated accordingly, but the values are not used
yet.

Signed-off-by: Olivier Galibert <galibert@pobox.com>
---
 src/mesa/drivers/dri/i965/brw_clip.c    |   90 ++++++++++++++++++++++++++++++-
 src/mesa/drivers/dri/i965/brw_clip.h    |    1 +
 src/mesa/drivers/dri/i965/brw_context.h |   11 ++++
 src/mesa/drivers/dri/i965/brw_sf.c      |    5 +-
 src/mesa/drivers/dri/i965/brw_sf.h      |    1 +
 src/mesa/drivers/dri/i965/brw_wm.c      |    2 +
 src/mesa/drivers/dri/i965/brw_wm.h      |    1 +
 7 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_clip.c b/src/mesa/drivers/dri/i965/brw_clip.c
index d411208..b4a2e0a 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.c
+++ b/src/mesa/drivers/dri/i965/brw_clip.c
@@ -47,6 +47,86 @@
 #define FRONT_UNFILLED_BIT  0x1
 #define BACK_UNFILLED_BIT   0x2
 
+/**
+ * Lookup the interpolation mode information for every element in the
+ * vue.
+ */
+static void
+brw_lookup_interpolation(struct brw_context *brw)
+{
+   /* pprog means "previous program", i.e. the last program before the
+    * fragment shader.  It can only be the vertex shader for now, but
+    * it may be a geometry shader in the future.
+    */
+   const struct gl_program *pprog = &brw->vertex_program->Base;
+   const struct gl_fragment_program *fprog = brw->fragment_program;
+   struct brw_vue_map *vue_map = &brw->vs.prog_data->vue_map;
+
+   /* Default everything to INTERP_QUALIFIER_NONE */
+   memset(brw->interpolation_mode, INTERP_QUALIFIER_NONE, BRW_VERT_RESULT_MAX);
+
+   /* If there is no fragment shader, interpolation won't be needed,
+    * so defaulting to none is good.
+    */
+   if (!fprog)
+      return;
+
+   for (int i = 0; i < vue_map->num_slots; i++) {
+      /* First lookup the vert result, skip if there isn't one */
+      int vert_result = vue_map->slot_to_vert_result[i];
+      if (vert_result == BRW_VERT_RESULT_MAX)
+         continue;
+
+      /* HPOS is special.  In the clipper, it is handled specifically,
+       * so its value is irrelevant.  In the sf, it's forced to
+       * linear.  In the wm, it's special cased, irrelevant again.  So
+       * force linear to remove the sf special case.
+       */
+      if (vert_result == VERT_RESULT_HPOS) {
+         brw->interpolation_mode[i] = INTERP_QUALIFIER_NOPERSPECTIVE;
+         continue;
+      }
+
+      /* There is a 1-1 mapping of vert result to frag attrib except
+       * for BackColor and vars
+       */
+      int frag_attrib = vert_result;
+      if (vert_result >= VERT_RESULT_BFC0 && vert_result <= VERT_RESULT_BFC1)
+         frag_attrib = vert_result - VERT_RESULT_BFC0 + FRAG_ATTRIB_COL0;
+      else if(vert_result >= VERT_RESULT_VAR0)
+         frag_attrib = vert_result - VERT_RESULT_VAR0 + FRAG_ATTRIB_VAR0;
+
+      /* If the output is not used by the fragment shader, skip it. */
+      if (!(fprog->Base.InputsRead & BITFIELD64_BIT(frag_attrib)))
+         continue;
+
+      /* Lookup the interpolation mode */
+      enum glsl_interp_qualifier interpolation_mode = fprog->InterpQualifier[frag_attrib];
+
+      /* If the mode is not specified, then the default varies.  Color
+       * values follow the shader model, while all the rest uses
+       * smooth.
+       */
+      if (interpolation_mode == INTERP_QUALIFIER_NONE) {
+         if (frag_attrib >= FRAG_ATTRIB_COL0 && frag_attrib <= FRAG_ATTRIB_COL1)
+            interpolation_mode = brw->intel.ctx.Light.ShadeModel == GL_FLAT ? INTERP_QUALIFIER_FLAT : INTERP_QUALIFIER_SMOOTH;
+         else
+            interpolation_mode = INTERP_QUALIFIER_SMOOTH;
+      }
+
+      /* Finally, if we have both a front color and a back color for
+       * the same channel, the selection will be done before
+       * interpolation and the back color copied over the front color
+       * if necessary.  So interpolating the back color is
+       * unnecessary.
+       */
+      if (vert_result >= VERT_RESULT_BFC0 && vert_result <= VERT_RESULT_BFC1)
+         if (pprog->OutputsWritten & BITFIELD64_BIT(vert_result - VERT_RESULT_BFC0 + VERT_RESULT_COL0))
+            interpolation_mode = INTERP_QUALIFIER_NONE;
+
+      brw->interpolation_mode[i] = interpolation_mode;
+   }
+}
 
 static void compile_clip_prog( struct brw_context *brw,
 			     struct brw_clip_prog_key *key )
@@ -143,6 +223,10 @@ brw_upload_clip_prog(struct brw_context *brw)
 
    /* Populate the key:
     */
+
+   /* BRW_NEW_FRAGMENT_PROGRAM, _NEW_LIGHT */
+   brw_lookup_interpolation(brw);
+
    /* BRW_NEW_REDUCED_PRIMITIVE */
    key.primitive = brw->intel.reduced_primitive;
    /* CACHE_NEW_VS_PROG (also part of VUE map) */
@@ -150,6 +234,10 @@ brw_upload_clip_prog(struct brw_context *brw)
    /* _NEW_LIGHT */
    key.do_flat_shading = (ctx->Light.ShadeModel == GL_FLAT);
    key.pv_first = (ctx->Light.ProvokingVertex == GL_FIRST_VERTEX_CONVENTION);
+
+   /* BRW_NEW_FRAGMENT_PROGRAM, _NEW_LIGHT */
+   memcpy(key.interpolation_mode, brw->interpolation_mode, BRW_VERT_RESULT_MAX);
+
    /* _NEW_TRANSFORM (also part of VUE map)*/
    key.nr_userclip = _mesa_bitcount_64(ctx->Transform.ClipPlanesEnabled);
 
@@ -258,7 +346,7 @@ const struct brw_tracked_state brw_clip_prog = {
 		_NEW_TRANSFORM |
 		_NEW_POLYGON | 
 		_NEW_BUFFERS),
-      .brw   = (BRW_NEW_REDUCED_PRIMITIVE),
+      .brw   = (BRW_NEW_FRAGMENT_PROGRAM|BRW_NEW_REDUCED_PRIMITIVE),
       .cache = CACHE_NEW_VS_PROG
    },
    .emit = brw_upload_clip_prog
diff --git a/src/mesa/drivers/dri/i965/brw_clip.h b/src/mesa/drivers/dri/i965/brw_clip.h
index 9185651..e78d074 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.h
+++ b/src/mesa/drivers/dri/i965/brw_clip.h
@@ -43,6 +43,7 @@
  */
 struct brw_clip_prog_key {
    GLbitfield64 attrs;
+   unsigned char interpolation_mode[BRW_VERT_RESULT_MAX]; /* copy of the main context */
    GLuint primitive:4;
    GLuint nr_userclip:4;
    GLuint do_flat_shading:1;
diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index b4868fe..afafa47 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1054,6 +1054,17 @@ struct brw_context
    uint32_t render_target_format[MESA_FORMAT_COUNT];
    bool format_supported_as_render_target[MESA_FORMAT_COUNT];
 
+   /* Interpolation modes, one byte per vue slot, values equal to
+    * glsl_interp_qualifier.
+    *
+    * Used on gen4/5 by the clipper, sf and wm stages.  Given the
+    * update order, the clipper is responsible to update it.
+    *
+    * Ignored on gen 6+
+    */
+
+   unsigned char interpolation_mode[BRW_VERT_RESULT_MAX];
+
    /* PrimitiveRestart */
    struct {
       bool in_progress;
diff --git a/src/mesa/drivers/dri/i965/brw_sf.c b/src/mesa/drivers/dri/i965/brw_sf.c
index 791210f..26cbaf7 100644
--- a/src/mesa/drivers/dri/i965/brw_sf.c
+++ b/src/mesa/drivers/dri/i965/brw_sf.c
@@ -194,6 +194,9 @@ brw_upload_sf_prog(struct brw_context *brw)
    key.do_flat_shading = (ctx->Light.ShadeModel == GL_FLAT);
    key.do_twoside_color = ctx->VertexProgram._TwoSideEnabled;
 
+   /* BRW_NEW_FRAGMENT_PROGRAM, _NEW_LIGHT */
+   memcpy(key.interpolation_mode, brw->interpolation_mode, BRW_VERT_RESULT_MAX);
+
    /* _NEW_POLYGON */
    if (key.do_twoside_color) {
       /* If we're rendering to a FBO, we have to invert the polygon
@@ -215,7 +218,7 @@ const struct brw_tracked_state brw_sf_prog = {
    .dirty = {
       .mesa  = (_NEW_HINT | _NEW_LIGHT | _NEW_POLYGON | _NEW_POINT |
                 _NEW_TRANSFORM | _NEW_BUFFERS),
-      .brw   = (BRW_NEW_REDUCED_PRIMITIVE),
+      .brw   = (BRW_NEW_FRAGMENT_PROGRAM|BRW_NEW_REDUCED_PRIMITIVE),
       .cache = CACHE_NEW_VS_PROG
    },
    .emit = brw_upload_sf_prog
diff --git a/src/mesa/drivers/dri/i965/brw_sf.h b/src/mesa/drivers/dri/i965/brw_sf.h
index f908fc0..5e261fb 100644
--- a/src/mesa/drivers/dri/i965/brw_sf.h
+++ b/src/mesa/drivers/dri/i965/brw_sf.h
@@ -46,6 +46,7 @@
 
 struct brw_sf_prog_key {
    GLbitfield64 attrs;
+   unsigned char interpolation_mode[BRW_VERT_RESULT_MAX]; /* copy of the main context */
    uint8_t point_sprite_coord_replace;
    GLuint primitive:2;
    GLuint do_twoside_color:1;
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
index 587cc35..b54f4b1 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -513,6 +513,8 @@ static void brw_wm_populate_key( struct brw_context *brw,
 
    /* _NEW_LIGHT */
    key->flat_shade = (ctx->Light.ShadeModel == GL_FLAT);
+   if (intel->gen < 6)
+      memcpy(key->interpolation_mode, brw->interpolation_mode, BRW_VERT_RESULT_MAX);
 
    /* _NEW_FRAG_CLAMP | _NEW_BUFFERS */
    key->clamp_fragment_color = ctx->Color._ClampFragmentColor;
diff --git a/src/mesa/drivers/dri/i965/brw_wm.h b/src/mesa/drivers/dri/i965/brw_wm.h
index b976a60..add9dd6 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.h
+++ b/src/mesa/drivers/dri/i965/brw_wm.h
@@ -60,6 +60,7 @@
 #define AA_ALWAYS    2
 
 struct brw_wm_prog_key {
+   unsigned char interpolation_mode[BRW_VERT_RESULT_MAX]; /* copy of the main context */
    uint8_t iz_lookup;
    GLuint stats_wm:1;
    GLuint flat_shade:1;
-- 
1.7.10.280.gaa39

Re: [Mesa-dev] [PATCH 5/9] intel gen4-5: Compute the interpolation status for every variable in one place.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1841 bytes --]

Olivier Galibert <galibert@pobox.com> writes:

> The program keys are updated accordingly, but the values are not used
> yet.
>
> Signed-off-by: Olivier Galibert <galibert@pobox.com>
> ---
>  src/mesa/drivers/dri/i965/brw_clip.c    |   90 ++++++++++++++++++++++++++++++-
>  src/mesa/drivers/dri/i965/brw_clip.h    |    1 +
>  src/mesa/drivers/dri/i965/brw_context.h |   11 ++++
>  src/mesa/drivers/dri/i965/brw_sf.c      |    5 +-
>  src/mesa/drivers/dri/i965/brw_sf.h      |    1 +
>  src/mesa/drivers/dri/i965/brw_wm.c      |    2 +
>  src/mesa/drivers/dri/i965/brw_wm.h      |    1 +
>  7 files changed, 109 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.c b/src/mesa/drivers/dri/i965/brw_clip.c
> index d411208..b4a2e0a 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip.c
> @@ -47,6 +47,86 @@
>  #define FRONT_UNFILLED_BIT  0x1
>  #define BACK_UNFILLED_BIT   0x2
>  
> +/**
> + * Lookup the interpolation mode information for every element in the
> + * vue.
> + */
> +static void
> +brw_lookup_interpolation(struct brw_context *brw)
> +{
> +   /* pprog means "previous program", i.e. the last program before the
> +    * fragment shader.  It can only be the vertex shader for now, but
> +    * it may be a geometry shader in the future.
> +    */
> +   const struct gl_program *pprog = &brw->vertex_program->Base;
> +   const struct gl_fragment_program *fprog = brw->fragment_program;
> +   struct brw_vue_map *vue_map = &brw->vs.prog_data->vue_map;
> +
> +   /* Default everything to INTERP_QUALIFIER_NONE */
> +   memset(brw->interpolation_mode, INTERP_QUALIFIER_NONE, BRW_VERT_RESULT_MAX);

I don't like seeing this data that should be referenced out of the
program cache key being communicated through brw->.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Mesa-dev] [PATCH 5/9] intel gen4-5: Compute the interpolation status for every variable in one place.

[Olivier Galibert]

On Thu, Jul 26, 2012 at 10:22:26AM -0700, Eric Anholt wrote:
> I don't like seeing this data that should be referenced out of the
> program cache key being communicated through brw->.

What would you like it being communicated through?

  OG.

[PATCH 6/9] intel gen4-5: Correctly setup the parameters in the sf.

[Olivier Galibert]

This patch also correct a couple of problems with noperspective
interpolation.

At that point all the glsl 1.1/1.3 interpolation tests that do not
clip pass (the -none ones).

The fs code does not use the pre-resolved interpolation modes in order
not to mess with gen6+.  Sharing the resolution would require putting
brw_wm_prog before brw_clip_prog and brw_sf_prog.  This may be a good
thing, but it could have unexpected consequences, so it's better be
done independently in any case.

Signed-off-by: Olivier Galibert <galibert@pobox.com>
Reviewed-by: Paul Berry <stereotype441@gmail.com>
---
 src/mesa/drivers/dri/i965/brw_fs.cpp         |    2 +-
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   15 +++
 src/mesa/drivers/dri/i965/brw_sf.c           |   12 +-
 src/mesa/drivers/dri/i965/brw_sf.h           |    2 +-
 src/mesa/drivers/dri/i965/brw_sf_emit.c      |  164 +++++++++++++-------------
 5 files changed, 106 insertions(+), 89 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 3b62952..4734a5d 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -757,7 +757,7 @@ fs_visitor::emit_general_interpolation(ir_variable *ir)
                      inst->predicated = true;
                      inst->predicate_inverse = true;
                   }
-		  if (intel->gen < 6) {
+		  if (intel->gen < 6 && interpolation_mode == INTERP_QUALIFIER_SMOOTH) {
 		     emit(BRW_OPCODE_MUL, attr, attr, this->pixel_w);
 		  }
 	       }
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 08c0130..c6dc265 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -1872,6 +1872,21 @@ fs_visitor::emit_interpolation_setup_gen4()
    emit(BRW_OPCODE_ADD, this->delta_y[BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC],
 	this->pixel_y, fs_reg(negate(brw_vec1_grf(1, 1))));
 
+   /*
+    * On Gen4-5, we accomplish perspective-correct interpolation by
+    * dividing the attribute values by w in the sf shader,
+    * interpolating the result linearly in screen space, and then
+    * multiplying by w in the fragment shader.  So the interpolation
+    * step is always linear in screen space, regardless of whether the
+    * attribute is perspective or non-perspective.  Accordingly, we
+    * use the same delta_x and delta_y values for both kinds of
+    * interpolation.
+    */
+   this->delta_x[BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC] =
+     this->delta_x[BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC];
+   this->delta_y[BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC] =
+     this->delta_y[BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC];
+
    this->current_annotation = "compute pos.w and 1/pos.w";
    /* Compute wpos.w.  It's always in our setup, since it's needed to
     * interpolate the other attributes.
diff --git a/src/mesa/drivers/dri/i965/brw_sf.c b/src/mesa/drivers/dri/i965/brw_sf.c
index 26cbaf7..c00e85a 100644
--- a/src/mesa/drivers/dri/i965/brw_sf.c
+++ b/src/mesa/drivers/dri/i965/brw_sf.c
@@ -139,6 +139,7 @@ brw_upload_sf_prog(struct brw_context *brw)
    struct brw_sf_prog_key key;
    /* _NEW_BUFFERS */
    bool render_to_fbo = _mesa_is_user_fbo(ctx->DrawBuffer);
+   int i;
 
    memset(&key, 0, sizeof(key));
 
@@ -190,11 +191,16 @@ brw_upload_sf_prog(struct brw_context *brw)
    if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) != render_to_fbo)
       key.sprite_origin_lower_left = true;
 
-   /* _NEW_LIGHT */
-   key.do_flat_shading = (ctx->Light.ShadeModel == GL_FLAT);
+   /* BRW_NEW_FRAGMENT_PROGRAM, _NEW_LIGHT */
+   key.has_flat_shading = 0;
+   for (i = 0; i < BRW_VERT_RESULT_MAX; i++) {
+      if (brw->interpolation_mode[i] == INTERP_QUALIFIER_FLAT) {
+         key.has_flat_shading = 1;
+         break;
+      }
+   }
    key.do_twoside_color = ctx->VertexProgram._TwoSideEnabled;
 
-   /* BRW_NEW_FRAGMENT_PROGRAM, _NEW_LIGHT */
    memcpy(key.interpolation_mode, brw->interpolation_mode, BRW_VERT_RESULT_MAX);
 
    /* _NEW_POLYGON */
diff --git a/src/mesa/drivers/dri/i965/brw_sf.h b/src/mesa/drivers/dri/i965/brw_sf.h
index 5e261fb..47fdb3e 100644
--- a/src/mesa/drivers/dri/i965/brw_sf.h
+++ b/src/mesa/drivers/dri/i965/brw_sf.h
@@ -50,7 +50,7 @@ struct brw_sf_prog_key {
    uint8_t point_sprite_coord_replace;
    GLuint primitive:2;
    GLuint do_twoside_color:1;
-   GLuint do_flat_shading:1;
+   GLuint has_flat_shading:1;
    GLuint frontface_ccw:1;
    GLuint do_point_sprite:1;
    GLuint do_point_coord:1;
diff --git a/src/mesa/drivers/dri/i965/brw_sf_emit.c b/src/mesa/drivers/dri/i965/brw_sf_emit.c
index 9d8aa38..c99578a 100644
--- a/src/mesa/drivers/dri/i965/brw_sf_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_sf_emit.c
@@ -44,6 +44,17 @@
 
 
 /**
+ * Determine the vue slot corresponding to the given half of the given
+ * register.  half=0 means the first half of a register, half=1 means the
+ * second half.
+ */
+static inline int vert_reg_to_vue_slot(struct brw_sf_compile *c, GLuint reg,
+                                       int half)
+{
+   return (reg + c->urb_entry_read_offset) * 2 + half;
+}
+
+/**
  * Determine the vert_result corresponding to the given half of the given
  * register.  half=0 means the first half of a register, half=1 means the
  * second half.
@@ -51,11 +62,24 @@
 static inline int vert_reg_to_vert_result(struct brw_sf_compile *c, GLuint reg,
                                           int half)
 {
-   int vue_slot = (reg + c->urb_entry_read_offset) * 2 + half;
+   int vue_slot = vert_reg_to_vue_slot(c, reg, half);
    return c->vue_map.slot_to_vert_result[vue_slot];
 }
 
 /**
+ * Determine the register corresponding to the given vue slot.
+ */
+static struct brw_reg get_vue_slot(struct brw_sf_compile *c,
+                                   struct brw_reg vert,
+                                   int vue_slot)
+{
+   GLuint off = vue_slot / 2 - c->urb_entry_read_offset;
+   GLuint sub = vue_slot % 2;
+
+   return brw_vec4_grf(vert.nr + off, sub * 4);
+}
+
+/**
  * Determine the register corresponding to the given vert_result.
  */
 static struct brw_reg get_vert_result(struct brw_sf_compile *c,
@@ -64,10 +88,7 @@ static struct brw_reg get_vert_result(struct brw_sf_compile *c,
 {
    int vue_slot = c->vue_map.vert_result_to_slot[vert_result];
    assert (vue_slot >= c->urb_entry_read_offset);
-   GLuint off = vue_slot / 2 - c->urb_entry_read_offset;
-   GLuint sub = vue_slot % 2;
-
-   return brw_vec4_grf(vert.nr + off, sub * 4);
+   return get_vue_slot(c, vert, vue_slot);
 }
 
 static bool
@@ -128,31 +149,37 @@ static void do_twoside_color( struct brw_sf_compile *c )
  * Flat shading
  */
 
-#define VERT_RESULT_COLOR_BITS (BITFIELD64_BIT(VERT_RESULT_COL0) | \
-                                BITFIELD64_BIT(VERT_RESULT_COL1))
-
-static void copy_colors( struct brw_sf_compile *c,
-		     struct brw_reg dst,
-                     struct brw_reg src,
-                     int allow_twoside)
+static void copy_flatshaded_attributes( struct brw_sf_compile *c,
+                                        struct brw_reg dst,
+                                        struct brw_reg src)
 {
    struct brw_compile *p = &c->func;
+   struct brw_context *brw = p->brw;
    GLuint i;
 
-   for (i = VERT_RESULT_COL0; i <= VERT_RESULT_COL1; i++) {
-      if (have_attr(c,i)) {
+   for (i = 0; i < BRW_VERT_RESULT_MAX; i++) {
+      if (brw->interpolation_mode[i] == INTERP_QUALIFIER_FLAT) {
 	 brw_MOV(p, 
-		 get_vert_result(c, dst, i),
-		 get_vert_result(c, src, i));
+		 get_vue_slot(c, dst, i),
+		 get_vue_slot(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));
       }
    }
 }
 
+static GLuint count_flatshaded_attributes(struct brw_sf_compile *c )
+{
+   struct brw_compile *p = &c->func;
+   struct brw_context *brw = p->brw;
+   GLuint count = 0;
+   GLuint i;
+   for (i = 0; i < BRW_VERT_RESULT_MAX; i++) {
+      if (brw->interpolation_mode[i] == INTERP_QUALIFIER_FLAT)
+         count++;
+   }
+   return count;
+}
+
 
 
 /* Need to use a computed jump to copy flatshaded attributes as the
@@ -168,18 +195,6 @@ static void do_flatshade_triangle( struct brw_sf_compile *c )
 
    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;
-
    /* Already done in clip program:
     */
    if (c->key.primitive == SF_UNFILLED_TRIS)
@@ -188,21 +203,23 @@ static void do_flatshade_triangle( struct brw_sf_compile *c )
    if (intel->gen == 5)
        jmpi = 2;
 
+   nr = count_flatshaded_attributes(c);
+
    brw_push_insn_state(p);
    
    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], c->key.do_twoside_color);
-   copy_colors(c, c->vert[2], c->vert[0], c->key.do_twoside_color);
+   copy_flatshaded_attributes(c, c->vert[1], c->vert[0]);
+   copy_flatshaded_attributes(c, c->vert[2], c->vert[0]);
    brw_JMPI(p, ip, ip, brw_imm_d(jmpi*(nr*4+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);
+   copy_flatshaded_attributes(c, c->vert[0], c->vert[1]);
+   copy_flatshaded_attributes(c, c->vert[2], c->vert[1]);
    brw_JMPI(p, ip, ip, brw_imm_d(jmpi*nr*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);
+   copy_flatshaded_attributes(c, c->vert[0], c->vert[2]);
+   copy_flatshaded_attributes(c, c->vert[1], c->vert[2]);
 
    brw_pop_insn_state(p);
 }
@@ -213,12 +230,9 @@ static void do_flatshade_line( 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 nr;
    GLuint jmpi = 1;
 
-   if (!nr)
-      return;
-
    /* Already done in clip program: 
     */
    if (c->key.primitive == SF_UNFILLED_TRIS)
@@ -227,14 +241,16 @@ static void do_flatshade_line( struct brw_sf_compile *c )
    if (intel->gen == 5)
        jmpi = 2;
 
+   nr = count_flatshaded_attributes(c);
+
    brw_push_insn_state(p);
    
    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], 0);
+   copy_flatshaded_attributes(c, c->vert[1], c->vert[0]);
 
    brw_JMPI(p, ip, ip, brw_imm_ud(jmpi*nr));
-   copy_colors(c, c->vert[0], c->vert[1], 0);
+   copy_flatshaded_attributes(c, c->vert[0], c->vert[1]);
 
    brw_pop_insn_state(p);
 }
@@ -332,40 +348,25 @@ static void invert_det( struct brw_sf_compile *c)
 
 static bool
 calculate_masks(struct brw_sf_compile *c,
-	        GLuint reg,
-		GLushort *pc,
-		GLushort *pc_persp,
-		GLushort *pc_linear)
+                GLuint reg,
+                GLushort *pc,
+                GLushort *pc_persp,
+                GLushort *pc_linear)
 {
+   struct brw_compile *p = &c->func;
+   struct brw_context *brw = p->brw;
+   enum glsl_interp_qualifier interp;
    bool is_last_attr = (reg == c->nr_setup_regs - 1);
-   GLbitfield64 persp_mask;
-   GLbitfield64 linear_mask;
-
-   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_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_BFC0) |
-                                     BITFIELD64_BIT(VERT_RESULT_BFC1));
-   else
-      linear_mask = c->key.attrs;
 
    *pc_persp = 0;
    *pc_linear = 0;
    *pc = 0xf;
-      
-   if (persp_mask & BITFIELD64_BIT(vert_reg_to_vert_result(c, reg, 0)))
-      *pc_persp = 0xf;
 
-   if (linear_mask & BITFIELD64_BIT(vert_reg_to_vert_result(c, reg, 0)))
+   interp = brw->interpolation_mode[vert_reg_to_vue_slot(c, reg, 0)];
+   if (interp == INTERP_QUALIFIER_SMOOTH) {
+      *pc_linear = 0xf;
+      *pc_persp = 0xf;
+   } else if(interp == INTERP_QUALIFIER_NOPERSPECTIVE)
       *pc_linear = 0xf;
 
    /* Maybe only processs one attribute on the final round:
@@ -373,11 +374,12 @@ calculate_masks(struct brw_sf_compile *c,
    if (vert_reg_to_vert_result(c, reg, 1) != BRW_VERT_RESULT_MAX) {
       *pc |= 0xf0;
 
-      if (persp_mask & BITFIELD64_BIT(vert_reg_to_vert_result(c, reg, 1)))
-	 *pc_persp |= 0xf0;
-
-      if (linear_mask & BITFIELD64_BIT(vert_reg_to_vert_result(c, reg, 1)))
-	 *pc_linear |= 0xf0;
+      interp = brw->interpolation_mode[vert_reg_to_vue_slot(c, reg, 1)];
+      if (interp == INTERP_QUALIFIER_SMOOTH) {
+         *pc_linear |= 0xf0;
+         *pc_persp |= 0xf0;
+      } else if(interp == INTERP_QUALIFIER_NOPERSPECTIVE)
+         *pc_linear |= 0xf0;
    }
 
    return is_last_attr;
@@ -430,7 +432,7 @@ void brw_emit_tri_setup(struct brw_sf_compile *c, bool allocate)
    if (c->key.do_twoside_color) 
       do_twoside_color(c);
 
-   if (c->key.do_flat_shading)
+   if (c->key.has_flat_shading)
       do_flatshade_triangle(c);
       
    
@@ -443,7 +445,6 @@ void brw_emit_tri_setup(struct brw_sf_compile *c, bool allocate)
       struct brw_reg a2 = offset(c->vert[2], i);
       GLushort pc, pc_persp, pc_linear;
       bool last = calculate_masks(c, i, &pc, &pc_persp, &pc_linear);
-
       if (pc_persp)
       {
 	 brw_set_predicate_control_flag_value(p, pc_persp);
@@ -507,7 +508,6 @@ void brw_emit_line_setup(struct brw_sf_compile *c, bool allocate)
    struct brw_compile *p = &c->func;
    GLuint i;
 
-
    c->nr_verts = 2;
 
    if (allocate)
@@ -516,7 +516,7 @@ void brw_emit_line_setup(struct brw_sf_compile *c, bool allocate)
    invert_det(c);
    copy_z_inv_w(c);
 
-   if (c->key.do_flat_shading)
+   if (c->key.has_flat_shading)
       do_flatshade_line(c);
 
    for (i = 0; i < c->nr_setup_regs; i++)
@@ -799,7 +799,3 @@ void brw_emit_anyprim_setup( struct brw_sf_compile *c )
 
    brw_emit_point_setup( c, false );
 }
-
-
-
-
-- 
1.7.10.280.gaa39

[PATCH 7/9] intel gen4-5: Correctly handle flat vs. non-flat in the clipper.

[Olivier Galibert]

At that point, all interpolation piglit tests involving fixed clipping
work as long as there's no noperspective.

Signed-off-by: Olivier Galibert <galibert@pobox.com>
Reviewed-by: Paul Berry <stereotype441@gmail.com>
---
 src/mesa/drivers/dri/i965/brw_clip.c          |   13 ++++--
 src/mesa/drivers/dri/i965/brw_clip.h          |    6 +--
 src/mesa/drivers/dri/i965/brw_clip_line.c     |    6 +--
 src/mesa/drivers/dri/i965/brw_clip_tri.c      |   20 ++++-----
 src/mesa/drivers/dri/i965/brw_clip_unfilled.c |    2 +-
 src/mesa/drivers/dri/i965/brw_clip_util.c     |   56 +++++++------------------
 src/mesa/drivers/dri/i965/brw_sf_emit.c       |    8 ++++
 7 files changed, 50 insertions(+), 61 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_clip.c b/src/mesa/drivers/dri/i965/brw_clip.c
index b4a2e0a..8512172 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.c
+++ b/src/mesa/drivers/dri/i965/brw_clip.c
@@ -218,7 +218,7 @@ brw_upload_clip_prog(struct brw_context *brw)
    struct intel_context *intel = &brw->intel;
    struct gl_context *ctx = &intel->ctx;
    struct brw_clip_prog_key key;
-
+   int i;
    memset(&key, 0, sizeof(key));
 
    /* Populate the key:
@@ -231,11 +231,16 @@ brw_upload_clip_prog(struct brw_context *brw)
    key.primitive = brw->intel.reduced_primitive;
    /* CACHE_NEW_VS_PROG (also part of VUE map) */
    key.attrs = brw->vs.prog_data->outputs_written;
-   /* _NEW_LIGHT */
-   key.do_flat_shading = (ctx->Light.ShadeModel == GL_FLAT);
+   /* BRW_NEW_FRAGMENT_PROGRAM, _NEW_LIGHT */
+   key.has_flat_shading = 0;
+   for (i = 0; i < BRW_VERT_RESULT_MAX; i++) {
+      if (brw->interpolation_mode[i] == INTERP_QUALIFIER_FLAT) {
+         key.has_flat_shading = 1;
+         break;
+      }
+   }
    key.pv_first = (ctx->Light.ProvokingVertex == GL_FIRST_VERTEX_CONVENTION);
 
-   /* BRW_NEW_FRAGMENT_PROGRAM, _NEW_LIGHT */
    memcpy(key.interpolation_mode, brw->interpolation_mode, BRW_VERT_RESULT_MAX);
 
    /* _NEW_TRANSFORM (also part of VUE map)*/
diff --git a/src/mesa/drivers/dri/i965/brw_clip.h b/src/mesa/drivers/dri/i965/brw_clip.h
index e78d074..3ad2e13 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.h
+++ b/src/mesa/drivers/dri/i965/brw_clip.h
@@ -46,7 +46,7 @@ struct brw_clip_prog_key {
    unsigned char interpolation_mode[BRW_VERT_RESULT_MAX]; /* copy of the main context */
    GLuint primitive:4;
    GLuint nr_userclip:4;
-   GLuint do_flat_shading:1;
+   GLuint has_flat_shading:1;
    GLuint pv_first:1;
    GLuint do_unfilled:1;
    GLuint fill_cw:2;		/* includes cull information */
@@ -166,8 +166,8 @@ void brw_clip_kill_thread(struct brw_clip_compile *c);
 struct brw_reg brw_clip_plane_stride( struct brw_clip_compile *c );
 struct brw_reg brw_clip_plane0_address( struct brw_clip_compile *c );
 
-void brw_clip_copy_colors( struct brw_clip_compile *c,
-			   GLuint to, GLuint from );
+void brw_clip_copy_flatshaded_attributes( struct brw_clip_compile *c,
+                                          GLuint to, GLuint from );
 
 void brw_clip_init_clipmask( struct brw_clip_compile *c );
 
diff --git a/src/mesa/drivers/dri/i965/brw_clip_line.c b/src/mesa/drivers/dri/i965/brw_clip_line.c
index 6cf2bd2..729d8c0 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_line.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_line.c
@@ -271,11 +271,11 @@ void brw_emit_line_clip( struct brw_clip_compile *c )
    brw_clip_line_alloc_regs(c);
    brw_clip_init_ff_sync(c);
 
-   if (c->key.do_flat_shading) {
+   if (c->key.has_flat_shading) {
       if (c->key.pv_first)
-         brw_clip_copy_colors(c, 1, 0);
+         brw_clip_copy_flatshaded_attributes(c, 1, 0);
       else
-         brw_clip_copy_colors(c, 0, 1);
+         brw_clip_copy_flatshaded_attributes(c, 0, 1);
    }
                 
    clip_and_emit_line(c);
diff --git a/src/mesa/drivers/dri/i965/brw_clip_tri.c b/src/mesa/drivers/dri/i965/brw_clip_tri.c
index a29f8e0..71225f5 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_tri.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_tri.c
@@ -187,8 +187,8 @@ void brw_clip_tri_flat_shade( struct brw_clip_compile *c )
 
    brw_IF(p, BRW_EXECUTE_1);
    {
-      brw_clip_copy_colors(c, 1, 0);
-      brw_clip_copy_colors(c, 2, 0);
+      brw_clip_copy_flatshaded_attributes(c, 1, 0);
+      brw_clip_copy_flatshaded_attributes(c, 2, 0);
    }
    brw_ELSE(p);
    {
@@ -200,19 +200,19 @@ void brw_clip_tri_flat_shade( struct brw_clip_compile *c )
 		 brw_imm_ud(_3DPRIM_TRIFAN));
 	 brw_IF(p, BRW_EXECUTE_1);
 	 {
-	    brw_clip_copy_colors(c, 0, 1);
-	    brw_clip_copy_colors(c, 2, 1);
+	    brw_clip_copy_flatshaded_attributes(c, 0, 1);
+	    brw_clip_copy_flatshaded_attributes(c, 2, 1);
 	 }
 	 brw_ELSE(p);
 	 {
-	    brw_clip_copy_colors(c, 1, 0);
-	    brw_clip_copy_colors(c, 2, 0);
+	    brw_clip_copy_flatshaded_attributes(c, 1, 0);
+	    brw_clip_copy_flatshaded_attributes(c, 2, 0);
 	 }
 	 brw_ENDIF(p);
       }
       else {
-         brw_clip_copy_colors(c, 0, 2);
-         brw_clip_copy_colors(c, 1, 2);
+         brw_clip_copy_flatshaded_attributes(c, 0, 2);
+         brw_clip_copy_flatshaded_attributes(c, 1, 2);
       }
    }
    brw_ENDIF(p);
@@ -606,8 +606,8 @@ void brw_emit_tri_clip( struct brw_clip_compile *c )
     * flatshading, need to apply the flatshade here because we don't
     * respect the PV when converting to trifan for emit:
     */
-   if (c->key.do_flat_shading) 
-      brw_clip_tri_flat_shade(c); 
+   if (c->key.has_flat_shading) 
+      brw_clip_tri_flat_shade(c);
       
    if ((c->key.clip_mode == BRW_CLIPMODE_NORMAL) ||
        (c->key.clip_mode == BRW_CLIPMODE_KERNEL_CLIP))
diff --git a/src/mesa/drivers/dri/i965/brw_clip_unfilled.c b/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
index 03c7d42..96f9a84 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
@@ -502,7 +502,7 @@ void brw_emit_unfilled_clip( struct brw_clip_compile *c )
 
    /* Need to do this whether we clip or not:
     */
-   if (c->key.do_flat_shading)
+   if (c->key.has_flat_shading)
       brw_clip_tri_flat_shade(c);
    
    brw_clip_init_clipmask(c);
diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c b/src/mesa/drivers/dri/i965/brw_clip_util.c
index bf8cc3a..692573e 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_util.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
@@ -165,7 +165,7 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
                  vert_result == VERT_RESULT_CLIP_DIST1) {
 	 /* PSIZ doesn't need interpolation because it isn't used by the
           * fragment shader.  CLIP_DIST0 and CLIP_DIST1 don't need
-          * intepolation because on pre-GEN6, these are just placeholder VUE
+          * interpolation because on pre-GEN6, these are just placeholder VUE
           * slots that don't perform any action.
           */
       } else if (vert_result < VERT_RESULT_MAX) {
@@ -291,49 +291,25 @@ struct brw_reg brw_clip_plane_stride( struct brw_clip_compile *c )
 }
 
 
-/* If flatshading, distribute color from provoking vertex prior to
+/* Distribute flatshaded attributes from provoking vertex prior to
  * clipping.
  */
-void brw_clip_copy_colors( struct brw_clip_compile *c,
-			   GLuint to, GLuint from )
+void brw_clip_copy_flatshaded_attributes( struct brw_clip_compile *c,
+                                          GLuint to, GLuint from )
 {
    struct brw_compile *p = &c->func;
-
-   if (brw_clip_have_vert_result(c, VERT_RESULT_COL0))
-      brw_MOV(p, 
-	      byte_offset(c->reg.vertex[to],
-                          brw_vert_result_to_offset(&c->vue_map,
-                                                    VERT_RESULT_COL0)),
-	      byte_offset(c->reg.vertex[from],
-                          brw_vert_result_to_offset(&c->vue_map,
-                                                    VERT_RESULT_COL0)));
-
-   if (brw_clip_have_vert_result(c, VERT_RESULT_COL1))
-      brw_MOV(p, 
-	      byte_offset(c->reg.vertex[to],
-                          brw_vert_result_to_offset(&c->vue_map,
-                                                    VERT_RESULT_COL1)),
-	      byte_offset(c->reg.vertex[from],
-                          brw_vert_result_to_offset(&c->vue_map,
-                                                    VERT_RESULT_COL1)));
-
-   if (brw_clip_have_vert_result(c, VERT_RESULT_BFC0))
-      brw_MOV(p, 
-	      byte_offset(c->reg.vertex[to],
-                          brw_vert_result_to_offset(&c->vue_map,
-                                                    VERT_RESULT_BFC0)),
-	      byte_offset(c->reg.vertex[from],
-                          brw_vert_result_to_offset(&c->vue_map,
-                                                    VERT_RESULT_BFC0)));
-
-   if (brw_clip_have_vert_result(c, VERT_RESULT_BFC1))
-      brw_MOV(p, 
-	      byte_offset(c->reg.vertex[to],
-                          brw_vert_result_to_offset(&c->vue_map,
-                                                    VERT_RESULT_BFC1)),
-	      byte_offset(c->reg.vertex[from],
-                          brw_vert_result_to_offset(&c->vue_map,
-                                                    VERT_RESULT_BFC1)));
+   struct brw_context *brw = p->brw;
+   GLuint i;
+
+   for (i = 0; i < BRW_VERT_RESULT_MAX; i++) {
+      if (brw->interpolation_mode[i] == INTERP_QUALIFIER_FLAT) {
+	 brw_MOV(p, 
+                 byte_offset(c->reg.vertex[to],
+                             brw_vue_slot_to_offset(i)),
+                 byte_offset(c->reg.vertex[from],
+                             brw_vue_slot_to_offset(i)));
+      }
+   }
 }
 
 
diff --git a/src/mesa/drivers/dri/i965/brw_sf_emit.c b/src/mesa/drivers/dri/i965/brw_sf_emit.c
index c99578a..2e9beed 100644
--- a/src/mesa/drivers/dri/i965/brw_sf_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_sf_emit.c
@@ -177,6 +177,14 @@ static GLuint count_flatshaded_attributes(struct brw_sf_compile *c )
       if (brw->interpolation_mode[i] == INTERP_QUALIFIER_FLAT)
          count++;
    }
+
+   /* This should only be called if there is at least one flatshaded
+    * attribute.  While nothing should break if there isn't any, the
+    * generated code would be heavily pessimized.  So check that all
+    * is well.
+    */
+   assert(count != 0);
+
    return count;
 }
 
-- 
1.7.10.280.gaa39

[PATCH 8/9] intel gen4-5: Make noperspective clipping work.

[Olivier Galibert]

At this point all interpolation tests with fixed clipping work.

Signed-off-by: Olivier Galibert <galibert@pobox.com>
Reviewed-by: Paul Berry <stereotype441@gmail.com>
---
 src/mesa/drivers/dri/i965/brw_clip.c      |    9 ++
 src/mesa/drivers/dri/i965/brw_clip.h      |    1 +
 src/mesa/drivers/dri/i965/brw_clip_util.c |  147 ++++++++++++++++++++++++++---
 3 files changed, 146 insertions(+), 11 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_clip.c b/src/mesa/drivers/dri/i965/brw_clip.c
index 8512172..eca2844 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.c
+++ b/src/mesa/drivers/dri/i965/brw_clip.c
@@ -239,6 +239,15 @@ brw_upload_clip_prog(struct brw_context *brw)
          break;
       }
    }
+   key.has_noperspective_shading = 0;
+   for (i = 0; i < BRW_VERT_RESULT_MAX; i++) {
+      if (brw->interpolation_mode[i] == INTERP_QUALIFIER_NOPERSPECTIVE &&
+          brw->vs.prog_data->vue_map.slot_to_vert_result[i] != VERT_RESULT_HPOS) {
+         key.has_noperspective_shading = 1;
+         break;
+      }
+   }
+
    key.pv_first = (ctx->Light.ProvokingVertex == GL_FIRST_VERTEX_CONVENTION);
 
    memcpy(key.interpolation_mode, brw->interpolation_mode, BRW_VERT_RESULT_MAX);
diff --git a/src/mesa/drivers/dri/i965/brw_clip.h b/src/mesa/drivers/dri/i965/brw_clip.h
index 3ad2e13..66dd928 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.h
+++ b/src/mesa/drivers/dri/i965/brw_clip.h
@@ -47,6 +47,7 @@ struct brw_clip_prog_key {
    GLuint primitive:4;
    GLuint nr_userclip:4;
    GLuint has_flat_shading:1;
+   GLuint has_noperspective_shading:1;
    GLuint pv_first:1;
    GLuint do_unfilled:1;
    GLuint fill_cw:2;		/* includes cull information */
diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c b/src/mesa/drivers/dri/i965/brw_clip_util.c
index 692573e..b06ad1d 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_util.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
@@ -129,6 +129,8 @@ static void brw_clip_project_vertex( struct brw_clip_compile *c,
 
 /* Interpolate between two vertices and put the result into a0.0.  
  * Increment a0.0 accordingly.
+ *
+ * Beware that dest_ptr can be equal to v0_ptr.
  */
 void brw_clip_interp_vertex( struct brw_clip_compile *c,
 			     struct brw_indirect dest_ptr,
@@ -138,7 +140,8 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
 			     bool force_edgeflag)
 {
    struct brw_compile *p = &c->func;
-   struct brw_reg tmp = get_tmp(c);
+   struct brw_context *brw = p->brw;
+   struct brw_reg t_nopersp, v0_ndc_copy;
    GLuint slot;
 
    /* Just copy the vertex header:
@@ -148,13 +151,130 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
     * back on Ironlake, so needn't change it
     */
    brw_copy_indirect_to_indirect(p, dest_ptr, v0_ptr, 1);
-      
-   /* Iterate over each attribute (could be done in pairs?)
+
+   /*
+    * First handle the 3D and NDC positioning, in case we need
+    * noperspective interpolation.  Doing it early has no performance
+    * impact in any case.
+    */
+
+   /* Start by picking up the v0 NDC coordinates, because that vertex
+    * may be shared with the destination.
+    */
+   if (c->key.has_noperspective_shading) {
+      GLuint offset = brw_vert_result_to_offset(&c->vue_map,
+                                                BRW_VERT_RESULT_NDC);
+      v0_ndc_copy = get_tmp(c);
+      brw_MOV(p, v0_ndc_copy, deref_4f(v0_ptr, offset));
+   }      
+
+   /*
+    * Compute the new 3D position
+    *
+    * dest_hpos = v0_hpos * (1 - t0) + v1_hpos * t0
+    */
+   {
+      GLuint delta = brw_vert_result_to_offset(&c->vue_map, VERT_RESULT_HPOS);
+      struct brw_reg tmp = get_tmp(c);
+      brw_MUL(p, 
+              vec4(brw_null_reg()),
+              deref_4f(v1_ptr, delta),
+              t0);
+
+      brw_MAC(p,
+              tmp,	      
+              negate(deref_4f(v0_ptr, delta)),
+              t0);
+	      
+      brw_ADD(p,
+              deref_4f(dest_ptr, delta), 
+              deref_4f(v0_ptr, delta),
+              tmp);
+      release_tmp(c, tmp);
+   }
+
+   /* Then recreate the projected (NDC) coordinate in the new vertex
+    * header
+    */
+   brw_clip_project_vertex(c, dest_ptr);
+
+   /*
+    * If we have noperspective attributes, we now need to compute the
+    * screen-space t.
+    */
+   if (c->key.has_noperspective_shading) {
+      GLuint delta = brw_vert_result_to_offset(&c->vue_map, BRW_VERT_RESULT_NDC);
+      struct brw_reg tmp = get_tmp(c);
+      t_nopersp = get_tmp(c);
+
+      /* Build a register with coordinates from the second and new vertices
+       *
+       * t_nopersp = vec4(v1.xy, dest.xy)
+       */
+      brw_MOV(p, t_nopersp, deref_4f(v1_ptr, delta));
+      brw_MOV(p, tmp, deref_4f(dest_ptr, delta));
+      brw_set_access_mode(p, BRW_ALIGN_16);
+      brw_MOV(p,
+              brw_writemask(t_nopersp, WRITEMASK_ZW),
+              brw_swizzle(tmp, 0,1,0,1));
+
+      /* Subtract the coordinates of the first vertex
+       *
+       * t_nopersp = vec4(v1.xy, dest.xy) - v0.xyxy
+       */
+      brw_ADD(p, t_nopersp, t_nopersp, negate(brw_swizzle(v0_ndc_copy, 0,1,0,1)));
+
+      /* Add the absolute value of the X and Y deltas so that if the
+       * points aren't in the same place on the screen we get non-zero
+       * values to divide.
+       *
+       * After that we have vert1-vert0 in t_nopersp.x and vertnew-vert0 in t_nopersp.y.
+       *
+       * t_nopersp = vec2(|v1.x  -v0.x| + |v1.y  -v0.y|,
+       *                  |dest.x-v0.x| + |dest.y-v0.y|)
+       */
+      brw_ADD(p,
+              brw_writemask(t_nopersp, WRITEMASK_XY),
+              brw_abs(brw_swizzle(t_nopersp, 0,2,0,0)),
+              brw_abs(brw_swizzle(t_nopersp, 1,3,0,0)));
+      brw_set_access_mode(p, BRW_ALIGN_1);
+
+      /* If the points are in the same place (vert1-vert0 == 0), just
+       * substitute a value that will ensure that we don't divide by
+       * 0.
+       */
+      brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_EQ,
+              vec1(t_nopersp),
+              brw_imm_f(0));
+      brw_IF(p, BRW_EXECUTE_1);
+      brw_MOV(p, t_nopersp, brw_imm_vf4(VF_ONE, VF_ZERO, VF_ZERO, VF_ZERO));
+      brw_ENDIF(p);
+
+      /* Now compute t_nopersp = t_nopersp.y/t_nopersp.x and broadcast it */
+      brw_math_invert(p, get_element(t_nopersp, 0), get_element(t_nopersp, 0));
+      brw_MUL(p,
+              vec1(t_nopersp),
+              vec1(t_nopersp),
+              vec1(suboffset(t_nopersp, 1)));
+      brw_set_access_mode(p, BRW_ALIGN_16);
+      brw_MOV(p, t_nopersp, brw_swizzle(t_nopersp, 0,0,0,0));
+      brw_set_access_mode(p, BRW_ALIGN_1);
+
+      release_tmp(c, tmp);
+      release_tmp(c, v0_ndc_copy);
+   }
+
+   /* Now we can iterate over each attribute
+    * (could be done in pairs?)
     */
    for (slot = 0; slot < c->vue_map.num_slots; slot++) {
       int vert_result = c->vue_map.slot_to_vert_result[slot];
       GLuint delta = brw_vue_slot_to_offset(slot);
 
+      /* HPOS is already handled */
+      if(vert_result == VERT_RESULT_HPOS)
+         continue;
+
       if (vert_result == VERT_RESULT_EDGE) {
 	 if (force_edgeflag) 
 	    brw_MOV(p, deref_4f(dest_ptr, delta), brw_imm_f(1));
@@ -174,20 +294,29 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
 	  *
 	  *        New = attr0 + t*attr1 - t*attr0
 	  */
+
+         struct brw_reg tmp = get_tmp(c);
+
+         struct brw_reg t =
+            brw->interpolation_mode[slot] == INTERP_QUALIFIER_NOPERSPECTIVE ?
+            t_nopersp : t0;
+
 	 brw_MUL(p, 
 		 vec4(brw_null_reg()),
 		 deref_4f(v1_ptr, delta),
-		 t0);
+		 t);
 
 	 brw_MAC(p, 
 		 tmp,	      
 		 negate(deref_4f(v0_ptr, delta)),
-		 t0); 
+		 t); 
 	      
 	 brw_ADD(p,
 		 deref_4f(dest_ptr, delta), 
 		 deref_4f(v0_ptr, delta),
 		 tmp);
+
+         release_tmp(c, tmp);
       }
    }
 
@@ -197,12 +326,8 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
       brw_MOV(p, deref_4f(dest_ptr, delta), brw_imm_f(0));
    }
 
-   release_tmp(c, tmp);
-
-   /* Recreate the projected (NDC) coordinate in the new vertex
-    * header:
-    */
-   brw_clip_project_vertex(c, dest_ptr );
+   if (c->key.has_noperspective_shading)
+      release_tmp(c, t_nopersp);
 }
 
 void brw_clip_emit_vue(struct brw_clip_compile *c, 
-- 
1.7.10.280.gaa39

[PATCH 9/9] intel gen4-5: Don't touch flatshaded values when clipping, only copy them.

[Olivier Galibert]

This patch ensures that integers will pass through unscathed.  Doing
(useless) computations on them is risky, especially when their bit
patterns correspond to values like inf or nan.

Signed-off-by: Olivier Galibert <galibert@pobox.com>
---
 src/mesa/drivers/dri/i965/brw_clip_util.c |   48 ++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c b/src/mesa/drivers/dri/i965/brw_clip_util.c
index b06ad1d..998c304 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_util.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
@@ -293,30 +293,42 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
 	  * header), so interpolate:
 	  *
 	  *        New = attr0 + t*attr1 - t*attr0
+          *
+          * unless it's flat shaded, then just copy the value from a
+          * source vertex.
 	  */
 
-         struct brw_reg tmp = get_tmp(c);
+         GLuint interp = brw->interpolation_mode[slot];
 
-         struct brw_reg t =
-            brw->interpolation_mode[slot] == INTERP_QUALIFIER_NOPERSPECTIVE ?
-            t_nopersp : t0;
+         if(interp == INTERP_QUALIFIER_SMOOTH ||
+            interp == INTERP_QUALIFIER_NOPERSPECTIVE) {
+            struct brw_reg tmp = get_tmp(c);
+            struct brw_reg t =
+               interp == INTERP_QUALIFIER_NOPERSPECTIVE ?
+               t_nopersp : t0;
 
-	 brw_MUL(p, 
-		 vec4(brw_null_reg()),
-		 deref_4f(v1_ptr, delta),
-		 t);
+            brw_MUL(p,
+                    vec4(brw_null_reg()),
+                    deref_4f(v1_ptr, delta),
+                    t);
 
-	 brw_MAC(p, 
-		 tmp,	      
-		 negate(deref_4f(v0_ptr, delta)),
-		 t); 
+            brw_MAC(p,
+                    tmp,
+                    negate(deref_4f(v0_ptr, delta)),
+                    t);
 	      
-	 brw_ADD(p,
-		 deref_4f(dest_ptr, delta), 
-		 deref_4f(v0_ptr, delta),
-		 tmp);
-
-         release_tmp(c, tmp);
+            brw_ADD(p,
+                    deref_4f(dest_ptr, delta),
+                    deref_4f(v0_ptr, delta),
+                    tmp);
+
+            release_tmp(c, tmp);
+
+         } else if(interp == INTERP_QUALIFIER_FLAT) {
+            brw_MOV(p,
+                    deref_4f(dest_ptr, delta),
+                    deref_4f(v0_ptr, delta));
+         }
       }
    }
 
-- 
1.7.10.280.gaa39