[Mesa-dev] [PATCH V5 4/6] i965 Gen4/5: clip: correctly handle flat varyings

Paul Berry stereotype441 at gmail.com
Wed Jul 31 06:46:42 PDT 2013


On 31 July 2013 04:07, Chris Forbes <chrisf at ijw.co.nz> 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
>
> V5: Move key.do_flat_shading to brw_clip_compile.has_flat_shading
>
> [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          |  5 ++-
>  src/mesa/drivers/dri/i965/brw_clip.h          |  7 ++--
>  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     | 46
> ++++++---------------------
>  6 files changed, 30 insertions(+), 54 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.c
> b/src/mesa/drivers/dri/i965/brw_clip.c
> index 7621675..1c2a4bc 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip.c
> @@ -70,6 +70,9 @@ static void compile_clip_prog( struct brw_context *brw,
>     c.key = *key;
>     c.vue_map = brw->vue_map_geom_out;
>
> +   c.has_flat_shading =
> +      brw_any_flat_varyings(&key->interpolation_mode);
> +
>     /* nr_regs is the number of registers filled by reading data from the
> VUE.
>      * This program accesses the entire VUE, so nr_regs needs to be the
> size of
>      * the VUE (measured in pairs, since two slots are stored in each
> @@ -149,8 +152,8 @@ 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);
>     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 e0d75b0..c408fac 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.h
> +++ b/src/mesa/drivers/dri/i965/brw_clip.h
> @@ -46,7 +46,6 @@ struct brw_clip_prog_key {
>     struct interpolation_mode_map interpolation_mode;
>     GLuint primitive:4;
>     GLuint nr_userclip:4;
> -   GLuint do_flat_shading:1;
>     GLuint pv_first:1;
>     GLuint do_unfilled:1;
>     GLuint fill_cw:2;           /* includes cull information */
> @@ -121,6 +120,8 @@ struct brw_clip_compile {
>     bool need_direction;
>
>     struct brw_vue_map vue_map;
> +
> +   GLuint has_flat_shading:1;
>

As with the last patch, I'd recommend making this a bool.  With that fixed,
this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>


>  };
>
>  /**
> @@ -173,8 +174,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..9001d36 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->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..19179c1 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->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..af327d6 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->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..9d77d1e 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_util.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
> @@ -290,49 +290,21 @@ 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)));
> +   for (int i = 0; i < c->vue_map.num_slots; i++) {
> +      if (c->key.interpolation_mode.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)));
> +      }
> +   }
>  }
>
>
> --
> 1.8.3.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130731/dfde6b10/attachment.html>


More information about the mesa-dev mailing list