diff mbox

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

Message ID 1341082215-22933-5-git-send-email-galibert@pobox.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Olivier Galibert June 30, 2012, 6:50 p.m. UTC
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>
---
 src/mesa/drivers/dri/i965/brw_clip.c          |   10 ++++-
 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 +++++++------------------
 6 files changed, 41 insertions(+), 59 deletions(-)

Comments

Paul Berry July 17, 2012, 12:55 p.m. UTC | #1
On 30 June 2012 11:50, Olivier Galibert <galibert@pobox.com> wrote:

> 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>
> ---
>  src/mesa/drivers/dri/i965/brw_clip.c          |   10 ++++-
>  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
> +++++++------------------
>  6 files changed, 41 insertions(+), 59 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.c
> b/src/mesa/drivers/dri/i965/brw_clip.c
> index 52e8c47..952eb4a 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip.c
> @@ -215,7 +215,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));
>
>     brw_lookup_interpolation(brw);
> @@ -227,7 +227,13 @@ brw_upload_clip_prog(struct brw_context *brw)
>     /* 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);
> +   key.has_flat_shading = 0;
> +   for (i = 0; i < BRW_VERT_RESULT_MAX; i++) {
> +      if (brw_get_interpolation_mode(brw, i) == INTERP_QUALIFIER_FLAT) {
> +         key.has_flat_shading = 1;
> +         break;
> +      }
> +   }
>     key.pv_first = (ctx->Light.ProvokingVertex ==
> GL_FIRST_VERTEX_CONVENTION);
>     brw_copy_interpolation_modes(brw, key.interpolation_mode);
>     /* _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 6f811ae..0ea0394 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 {
>     GLbitfield64 interpolation_mode[2]; /* 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..7b0205a 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
>

Heh.  Thanks for fixing my typo :)


>            * 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_get_interpolation_mode(brw, 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)));
> +      }
> +   }
>  }
>

Ah, a loop!  This is so much better.  Thanks.


>
>
> --
> 1.7.10.rc3.1.gb306
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

This patch is:

Reviewed-by: Paul Berry <stereotype441@gmail.com>
diff mbox

Patch

diff --git a/src/mesa/drivers/dri/i965/brw_clip.c b/src/mesa/drivers/dri/i965/brw_clip.c
index 52e8c47..952eb4a 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.c
+++ b/src/mesa/drivers/dri/i965/brw_clip.c
@@ -215,7 +215,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));
 
    brw_lookup_interpolation(brw);
@@ -227,7 +227,13 @@  brw_upload_clip_prog(struct brw_context *brw)
    /* 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);
+   key.has_flat_shading = 0;
+   for (i = 0; i < BRW_VERT_RESULT_MAX; i++) {
+      if (brw_get_interpolation_mode(brw, i) == INTERP_QUALIFIER_FLAT) {
+         key.has_flat_shading = 1;
+         break;
+      }
+   }
    key.pv_first = (ctx->Light.ProvokingVertex == GL_FIRST_VERTEX_CONVENTION);
    brw_copy_interpolation_modes(brw, key.interpolation_mode);
    /* _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 6f811ae..0ea0394 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 {
    GLbitfield64 interpolation_mode[2]; /* 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..7b0205a 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_get_interpolation_mode(brw, 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)));
+      }
+   }
 }