[Mesa-dev] [PATCH V3 3/5] i965 Gen4/5: clip: correctly handle flat varyings
Paul Berry
stereotype441 at gmail.com
Tue Jul 30 14:33:29 PDT 2013
On 14 July 2013 02:39, 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
>
> [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);
> +
> + /* 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;
> + }
> + }
>
Similar to my comment on patch 2/5, I'd prefer to see this for() loop moved
into compile_clip_prog() (and key.has_flat_shading moved into
brw_clip_compile) so that we only do this computation when compiling the
clip program.
In fact, since this is the same computation that patch 2/5 added to SF, we
should probably move it to its own function to avoid code duplication.
With that fixed, this patch is:
Reviewed-by: Paul Berry <stereotype441 at gmail.com>
> +
> 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)));
> + }
> + }
> }
>
>
> --
> 1.8.3.2
>
> _______________________________________________
> 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/20130730/652e5ef7/attachment.html>
More information about the mesa-dev
mailing list