[Mesa-dev] [PATCH V3 3/5] i965 Gen4/5: clip: correctly handle flat varyings

Kenneth Graunke kenneth at whitecape.org
Mon Jul 22 23:32:02 PDT 2013


On 07/14/2013 02:39 AM, Chris Forbes wrote:
> Previously we only gave special treatment to the builtin color varyings.
> This patch adds support for arbitrary flat-shaded varyings, which is
> required for GLSL 1.30.
>
> Based on Olivier Galibert's patch from last year:
> http://lists.freedesktop.org/archives/mesa-dev/2012-July/024340.html
>
> [V1-2]: Signed-off-by: Olivier Galibert <galibert at pobox.com>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
>   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      | 18 +++++-----
>   src/mesa/drivers/dri/i965/brw_clip_unfilled.c |  2 +-
>   src/mesa/drivers/dri/i965/brw_clip_util.c     | 51 +++++++--------------------
>   6 files changed, 40 insertions(+), 56 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.c b/src/mesa/drivers/dri/i965/brw_clip.c
> index 488e0a0..7df4b18 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip.c
> @@ -186,6 +186,7 @@ brw_upload_clip_prog(struct brw_context *brw)
>   {
>      struct gl_context *ctx = &brw->ctx;
>      struct brw_clip_prog_key key;
> +   int i;
>
>      memset(&key, 0, sizeof(key));
>
> @@ -200,8 +201,16 @@ brw_upload_clip_prog(struct brw_context *brw)
>      key.primitive = brw->reduced_primitive;
>      /* BRW_NEW_VUE_MAP_GEOM_OUT */
>      key.attrs = brw->vue_map_geom_out.slots_valid;
> -   /* _NEW_LIGHT */
> -   key.do_flat_shading = (ctx->Light.ShadeModel == GL_FLAT);

If you end up splitting the s/do/has/ out of the previous patch, you 
might put this rename in there too.

Other than that tiny comment, this patch looks good to me!
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> +
> +   /* BRW_NEW_FRAGMENT_PROGRAM, _NEW_LIGHT */
> +   key.has_flat_shading = 0;
> +   for (i = 0; i < brw->vue_map_geom_out.num_slots; i++) {
> +      if (key.interpolation_mode[i] == INTERP_QUALIFIER_FLAT) {
> +         key.has_flat_shading = 1;
> +         break;
> +      }
> +   }
> +
>      key.pv_first = (ctx->Light.ProvokingVertex == GL_FIRST_VERTEX_CONVENTION);
>      /* _NEW_TRANSFORM (also part of VUE map)*/
>      key.nr_userclip = _mesa_bitcount_64(ctx->Transform.ClipPlanesEnabled);
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.h b/src/mesa/drivers/dri/i965/brw_clip.h
> index fcbe2a0..656254b 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_VARYING_SLOT_COUNT];
>      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 */
> @@ -173,8 +173,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 9ce80b8..8df7d0c 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_line.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_line.c
> @@ -272,11 +272,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 bea0853..ac24929 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_tri.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_tri.c
> @@ -190,8 +190,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);
>      {
> @@ -203,19 +203,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);
> @@ -645,7 +645,7 @@ 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)
> +   if (c->key.has_flat_shading)
>         brw_clip_tri_flat_shade(c);
>
>      if ((c->key.clip_mode == BRW_CLIPMODE_NORMAL) ||
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_unfilled.c b/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
> index e211b95..0e645c0 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
> @@ -501,7 +501,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 37b7734..8a21c1f 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_util.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
> @@ -290,49 +290,24 @@ 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,
> +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_varying(c, VARYING_SLOT_COL0))
> -      brw_MOV(p,
> -	      byte_offset(c->reg.vertex[to],
> -                          brw_varying_to_offset(&c->vue_map,
> -                                                VARYING_SLOT_COL0)),
> -	      byte_offset(c->reg.vertex[from],
> -                          brw_varying_to_offset(&c->vue_map,
> -                                                VARYING_SLOT_COL0)));
> -
> -   if (brw_clip_have_varying(c, VARYING_SLOT_COL1))
> -      brw_MOV(p,
> -	      byte_offset(c->reg.vertex[to],
> -                          brw_varying_to_offset(&c->vue_map,
> -                                                VARYING_SLOT_COL1)),
> -	      byte_offset(c->reg.vertex[from],
> -                          brw_varying_to_offset(&c->vue_map,
> -                                                VARYING_SLOT_COL1)));
> -
> -   if (brw_clip_have_varying(c, VARYING_SLOT_BFC0))
> -      brw_MOV(p,
> -	      byte_offset(c->reg.vertex[to],
> -                          brw_varying_to_offset(&c->vue_map,
> -                                                VARYING_SLOT_BFC0)),
> -	      byte_offset(c->reg.vertex[from],
> -                          brw_varying_to_offset(&c->vue_map,
> -                                                VARYING_SLOT_BFC0)));
> -
> -   if (brw_clip_have_varying(c, VARYING_SLOT_BFC1))
> -      brw_MOV(p,
> -	      byte_offset(c->reg.vertex[to],
> -                          brw_varying_to_offset(&c->vue_map,
> -                                                VARYING_SLOT_BFC1)),
> -	      byte_offset(c->reg.vertex[from],
> -                          brw_varying_to_offset(&c->vue_map,
> -                                                VARYING_SLOT_BFC1)));
> +   int i;
> +
> +   for (i=0; i < c->vue_map.num_slots; i++) {
> +      if (c->key.interpolation_mode[i] == INTERP_QUALIFIER_FLAT) {
> +         printf("flatshaded: %d @ %d\n", i,
> +               brw_vue_slot_to_offset(i));
> +         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)));
> +      }
> +   }
>   }
>
>
>



More information about the mesa-dev mailing list