[Mesa-dev] [PATCH 21/36] i965: SF: New implementation of get_attr_override using the VUE map.

[Eric Anholt]

On Fri,  2 Sep 2011 09:07:00 -0700, Paul Berry <stereotype441 at gmail.com> wrote:
> This patch changes get_attr_override() (which computes the
> relationship between vertex shader outputs and fragment shader inputs)
> to use the VUE map.
> ---
>  src/mesa/drivers/dri/i965/brw_state.h     |    3 +-
>  src/mesa/drivers/dri/i965/gen6_sf_state.c |  108 +++++++++++++++++------------
>  src/mesa/drivers/dri/i965/gen7_sf_state.c |   13 +++-
>  3 files changed, 75 insertions(+), 49 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
> index cede4e5..167134b 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -206,7 +206,8 @@ void upload_default_color(struct brw_context *brw,
>  
>  /* gen6_sf_state.c */
>  uint32_t
> -get_attr_override(struct brw_context *brw, int fs_attr, int two_side_color);
> +get_attr_override(struct brw_vue_map *vue_map, int urb_entry_read_offset,
> +                  int fs_attr);
>  
>  /* gen7_misc_state.c */
>  unsigned int
> diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> index 714914a..5e121f7 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c

> +   if (vs_attr < 0 || vs_attr == VERT_RESULT_HPOS) {
> +      /* These attributes will be overwritten by the fragment shader's
> +       * interpolation code (see emit_interp() in brw_wm_fp.c), so just let
> +       * them reference attribute the first available attribute.
> +       */
> +      return 0;

One too many attributes in that comment.

> +   /* Compute the location of the attribute relative to urb_entry_read_offset.
> +    * Each increment of urb_entry_read_offset represents a 256-bit value, so
> +    * it counts for two 128-bit VUE slots.
> +    */
> +   attr_override = slot - 2*urb_entry_read_offset;

I like spaces between binary operators unless you're really hurting for
space.

> +   assert (attr_override >= 0 && attr_override < 32);
>  
> -   return attr_index;
> +   /* If the VUE slot following this one represents a back-facing color, then
> +    * we need to instruct the SF unit to do back-facing swizzling.
> +    */
> +   if (vue_map->slot_to_vert_result[slot] == VERT_RESULT_COL0 &&
> +       vue_map->slot_to_vert_result[slot+1] == VERT_RESULT_BFC0)
> +      attr_override |= (ATTRIBUTE_SWIZZLE_INPUTATTR_FACING << ATTRIBUTE_SWIZZLE_SHIFT);
> +   else if (vue_map->slot_to_vert_result[slot] == VERT_RESULT_COL1 &&
> +            vue_map->slot_to_vert_result[slot+1] == VERT_RESULT_BFC1)
> +      attr_override |= (ATTRIBUTE_SWIZZLE_INPUTATTR_FACING << ATTRIBUTE_SWIZZLE_SHIFT);
> +
> +   return attr_override;

I think that this is the wrong criteria for backface color swizzle, not
that it's a new bug you're introducing.  We need some tests for this.
From the GL 3.2 compatibility spec, page 75:

    "Additionally, vertex and geometry shaders can operate in two-sided
     color mode. When a vertex or geometry shader is active, front and
     back colors can be computed by the shader and written to the
     gl_FrontColor, gl_BackColor, gl_FrontSecondaryColor and
     gl_BackSecondaryColor outputs. If VERTEX_PROGRAM_TWO_SIDE is
     enabled, the GL chooses between front and back colors, as described
     below. Otherwise, the front color output is always
     selected. Two-sided color mode is enabled and disabled by calling
     Enable or Disable with the symbolic value VERTEX_PROGRAM_TWO_SIDE.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110902/cdf72b6e/attachment.pgp>