On 2 September 2011 13:57, Eric Anholt <<a href="mailto:eric@anholt.net">eric@anholt.net</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"> <div class="im">On Fri, 2 Sep 2011 09:07:00 -0700, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> 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 </div><div class="im">> + 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; </div>One too many attributes in that comment. </blockquote><div> Oops, I'll extra delete the extra word. </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> <div class="im"> > + /* 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; </div>I like spaces between binary operators unless you're really hurting for space. </blockquote><div> Yeah, I flip flop on whether to put spaces around "*". The programmer in me wants the spaces, the mathematician in me deplores them. I guess I was feeling like a mathematician when I wrote this line. </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> <div class="im"> > + 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; </div>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.