[Mesa-dev] [PATCH V5 3/6] i965 Gen4/5: Generalize SF interpolation setup for GLSL1.3

Paul Berry stereotype441 at gmail.com
Wed Jul 31 06:45:23 PDT 2013


On 31 July 2013 04:07, Chris Forbes <chrisf at ijw.co.nz> wrote:

> Previously the SF only handled the builtin color varying specially.
> This patch generalizes that support to cover user-defined varyings,
> driven by the interpolation mode array set up alongside the VUE map.
>
> Based on the following patches from Olivier Galibert:
> - http://lists.freedesktop.org/archives/mesa-dev/2012-July/024335.html
> - http://lists.freedesktop.org/archives/mesa-dev/2012-July/024339.html
>
> With this patch, all the GLSL 1.3 interpolation tests that do not clip
> (spec/glsl-1.30/execution/interpolation/*-none.shader_test) pass.
>
> V5: Move key.do_flat_shading to brw_sf_compile.has_flat_shading; drop
> vestigial hunks.
>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp    |   2 +-
>  src/mesa/drivers/dri/i965/brw_sf.c      |   2 +-
>  src/mesa/drivers/dri/i965/brw_sf.h      |   2 +-
>  src/mesa/drivers/dri/i965/brw_sf_emit.c | 149
> ++++++++++++++++++--------------
>  4 files changed, 85 insertions(+), 70 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index afd29de..a81e97f 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1048,7 +1048,7 @@ fs_visitor::emit_general_interpolation(ir_variable
> *ir)
>                    inst->predicate = BRW_PREDICATE_NORMAL;
>                    inst->predicate_inverse = true;
>                 }
> -               if (brw->gen < 6) {
> +               if (brw->gen < 6 && interpolation_mode ==
> INTERP_QUALIFIER_SMOOTH) {
>                    emit(BRW_OPCODE_MUL, attr, attr, this->pixel_w);
>                 }
>                attr.reg_offset++;
> diff --git a/src/mesa/drivers/dri/i965/brw_sf.c
> b/src/mesa/drivers/dri/i965/brw_sf.c
> index b062c0b..1634078 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf.c
> @@ -81,6 +81,7 @@ static void compile_sf_prog( struct brw_context *brw,
>
>     c.prog_data.urb_read_length = c.nr_attr_regs;
>     c.prog_data.urb_entry_size = c.nr_setup_regs * 2;
> +   c.has_flat_shading = brw_any_flat_varyings(&key->interpolation_mode);
>
>     /* Which primitive?  Or all three?
>      */
> @@ -193,7 +194,6 @@ brw_upload_sf_prog(struct brw_context *brw)
>     key.interpolation_mode = brw->interpolation_mode;
>
>     /* _NEW_LIGHT | _NEW_PROGRAM */
> -   key.do_flat_shading = (ctx->Light.ShadeModel == GL_FLAT);
>     key.do_twoside_color = ((ctx->Light.Enabled &&
> ctx->Light.Model.TwoSide) ||
>                             ctx->VertexProgram._TwoSideEnabled);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_sf.h
> b/src/mesa/drivers/dri/i965/brw_sf.h
> index d65f495..f738461 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf.h
> +++ b/src/mesa/drivers/dri/i965/brw_sf.h
> @@ -50,7 +50,6 @@ 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 frontface_ccw:1;
>     GLuint do_point_sprite:1;
>     GLuint do_point_coord:1;
> @@ -96,6 +95,7 @@ struct brw_sf_compile {
>     int urb_entry_read_offset;
>
>     struct brw_vue_map vue_map;
> +   GLuint has_flat_shading:1;
>

I would just make this a bool rather than a 1-bit wide bitfield.  The only
reason brw_sf_prog_key uses bitfields is that it's a hash key, so compact
representation is important.  That's not an issue for brw_sf_compile.

Wth that fixed, this patch is:

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


>  };
>
>
> diff --git a/src/mesa/drivers/dri/i965/brw_sf_emit.c
> b/src/mesa/drivers/dri/i965/brw_sf_emit.c
> index bd68f68..0131de5 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf_emit.c
> @@ -44,6 +44,15 @@
>
>
>  /**
> + * Determine the vue slot corresponding to the given half of the given
> register.
> + */
> +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 varying 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 +60,24 @@
>  static inline int vert_reg_to_varying(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_varying[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 varying.
>   */
>  static struct brw_reg get_varying(struct brw_sf_compile *c,
> @@ -64,10 +86,7 @@ static struct brw_reg get_varying(struct brw_sf_compile
> *c,
>  {
>     int vue_slot = c->vue_map.varying_to_slot[varying];
>     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
> @@ -105,14 +124,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 backface color, do the selection. The
> VS
> +    * promises to set up the front color if the backface color is
> provided, but
> +    * it may contain junk if never written to.
>      */
>     if (!(have_attr(c, VARYING_SLOT_COL0) && have_attr(c,
> VARYING_SLOT_BFC0)) &&
>         !(have_attr(c, VARYING_SLOT_COL1) && have_attr(c,
> VARYING_SLOT_BFC1)))
>        return;
> -
> +
>     /* Need to use BRW_EXECUTE_4 and also do an 4-wide compare in order
>      * to get all channels active inside the IF.  In the clipping code
>      * we run with NoMask, so it's not an option and we can use
> @@ -138,24 +157,34 @@ static void do_twoside_color( struct brw_sf_compile
> *c )
>   * Flat shading
>   */
>
> -#define VARYING_SLOT_COLOR_BITS (BITFIELD64_BIT(VARYING_SLOT_COL0) | \
> -                                 BITFIELD64_BIT(VARYING_SLOT_COL1))
> -
> -static void copy_colors( struct brw_sf_compile *c,
> -                    struct brw_reg dst,
> -                    struct brw_reg src)
> +static void copy_flatshaded_attributes(struct brw_sf_compile *c,
> +                                       struct brw_reg dst,
> +                                       struct brw_reg src)
>  {
>     struct brw_compile *p = &c->func;
> -   GLuint i;
> +   int i;
>
> -   for (i = VARYING_SLOT_COL0; i <= VARYING_SLOT_COL1; i++) {
> -      if (have_attr(c,i))
> -        brw_MOV(p,
> -                get_varying(c, dst, i),
> -                get_varying(c, src, i));
> +   for (i = 0; i < c->vue_map.num_slots; i++) {
> +      if (c->key.interpolation_mode.mode[i] == INTERP_QUALIFIER_FLAT) {
> +         brw_MOV(p,
> +                 get_vue_slot(c, dst, i),
> +                 get_vue_slot(c, src, i));
> +      }
>     }
>  }
>
> +static int count_flatshaded_attributes(struct brw_sf_compile *c)
> +{
> +   int i;
> +   int count = 0;
> +
> +   for (i = 0; i < c->vue_map.num_slots; i++)
> +      if (c->key.interpolation_mode.mode[i] == INTERP_QUALIFIER_FLAT)
> +         count++;
> +
> +   return count;
> +}
> +
>
>
>  /* Need to use a computed jump to copy flatshaded attributes as the
> @@ -167,12 +196,9 @@ static void do_flatshade_triangle( struct
> brw_sf_compile *c )
>     struct brw_compile *p = &c->func;
>     struct brw_context *brw = p->brw;
>     struct brw_reg ip = brw_ip_reg();
> -   GLuint nr = _mesa_bitcount_64(c->key.attrs & VARYING_SLOT_COLOR_BITS);
> +   GLuint nr;
>     GLuint jmpi = 1;
>
> -   if (!nr)
> -      return;
> -
>     /* Already done in clip program:
>      */
>     if (c->key.primitive == SF_UNFILLED_TRIS)
> @@ -181,21 +207,23 @@ static void do_flatshade_triangle( struct
> brw_sf_compile *c )
>     if (brw->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]);
> -   copy_colors(c, c->vert[2], c->vert[0]);
> +   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]);
> -   copy_colors(c, c->vert[2], c->vert[1]);
> +   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]);
> -   copy_colors(c, c->vert[1], c->vert[2]);
> +   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);
>  }
> @@ -206,12 +234,9 @@ static void do_flatshade_line( struct brw_sf_compile
> *c )
>     struct brw_compile *p = &c->func;
>     struct brw_context *brw = p->brw;
>     struct brw_reg ip = brw_ip_reg();
> -   GLuint nr = _mesa_bitcount_64(c->key.attrs & VARYING_SLOT_COLOR_BITS);
> +   GLuint nr;
>     GLuint jmpi = 1;
>
> -   if (!nr)
> -      return;
> -
>     /* Already done in clip program:
>      */
>     if (c->key.primitive == SF_UNFILLED_TRIS)
> @@ -220,14 +245,16 @@ static void do_flatshade_line( struct brw_sf_compile
> *c )
>     if (brw->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]);
> +   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]);
> +   copy_flatshaded_attributes(c, c->vert[0], c->vert[1]);
>
>     brw_pop_insn_state(p);
>  }
> @@ -324,36 +351,23 @@ 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)
>  {
>     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(VARYING_SLOT_POS) |
> -                                    BITFIELD64_BIT(VARYING_SLOT_COL0) |
> -                                    BITFIELD64_BIT(VARYING_SLOT_COL1));
> -   else
> -      persp_mask = c->key.attrs & ~(BITFIELD64_BIT(VARYING_SLOT_POS));
> -
> -   if (c->key.do_flat_shading)
> -      linear_mask = c->key.attrs & ~(BITFIELD64_BIT(VARYING_SLOT_COL0) |
> -                                     BITFIELD64_BIT(VARYING_SLOT_COL1));
> -   else
> -      linear_mask = c->key.attrs;
> +   enum glsl_interp_qualifier interp;
>
>     *pc_persp = 0;
>     *pc_linear = 0;
>     *pc = 0xf;
>
> -   if (persp_mask & BITFIELD64_BIT(vert_reg_to_varying(c, reg, 0)))
> +   interp = c->key.interpolation_mode.mode[vert_reg_to_vue_slot(c, reg,
> 0)];
> +   if (interp == INTERP_QUALIFIER_SMOOTH) {
> +      *pc_linear = 0xf;
>        *pc_persp = 0xf;
> -
> -   if (linear_mask & BITFIELD64_BIT(vert_reg_to_varying(c, reg, 0)))
> +   } else if (interp == INTERP_QUALIFIER_NOPERSPECTIVE)
>        *pc_linear = 0xf;
>
>     /* Maybe only processs one attribute on the final round:
> @@ -361,11 +375,12 @@ calculate_masks(struct brw_sf_compile *c,
>     if (vert_reg_to_varying(c, reg, 1) != BRW_VARYING_SLOT_COUNT) {
>        *pc |= 0xf0;
>
> -      if (persp_mask & BITFIELD64_BIT(vert_reg_to_varying(c, reg, 1)))
> -        *pc_persp |= 0xf0;
> -
> -      if (linear_mask & BITFIELD64_BIT(vert_reg_to_varying(c, reg, 1)))
> -        *pc_linear |= 0xf0;
> +      interp = c->key.interpolation_mode.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;
> @@ -418,7 +433,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->has_flat_shading)
>        do_flatshade_triangle(c);
>
>
> @@ -504,7 +519,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->has_flat_shading)
>        do_flatshade_line(c);
>
>     for (i = 0; i < c->nr_setup_regs; 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/61c9c510/attachment-0001.html>


More information about the mesa-dev mailing list