On 09/03/2013 06:18 PM, Paul Berry wrote: > Previously, on Gen6+, we laid out the vertex (or geometry) shader VUE > map differently depending whether user clipping was active. If it was > active, we put the clip distances in slots 2 and 3 (where the clipper > expects them); if it was inactive, we assigned them in the order of > the gl_varying_slot enum. > > This made for unnecessary recompiles, since turning clipping on/off > for a shader that used gl_ClipDistance might rearrange the varyings. > It also required extra bookkeeping, since it required the user > clipping flag to be provided to brw_compute_vue_map() as a parameter. > > With this patch, we always put clip distances at in slots 2 and 3 if > they are written to. do_vs_prog() and do_gs_prog() are responsible > for ensuring that clip distances are written to when user clipping is > enabled (as do_vs_prog() previously did for gen4-5). > > This makes the only input to brw_compute_vue_map() a bitfield of which > varyings the shader writes to, a fact that we'll take advantage of in > forthcoming patches. > --- > src/mesa/drivers/dri/i965/brw_context.h | 2 +- > src/mesa/drivers/dri/i965/brw_vec4_gs.c | 15 ++++++++++++--- > src/mesa/drivers/dri/i965/brw_vs.c | 26 +++++++++++++------------- > 3 files changed, 26 insertions(+), 17 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > b/src/mesa/drivers/dri/i965/brw_context.h > index 167ed4a..0c1fd9e 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.h > +++ b/src/mesa/drivers/dri/i965/brw_context.h > @@ -436,7 +436,7 @@ static inline GLuint brw_varying_to_offset(struct > brw_vue_map *vue_map, > } > > void brw_compute_vue_map(struct brw_context *brw, struct brw_vue_map > *vue_map, > - GLbitfield64 slots_valid, bool userclip_active); > + GLbitfield64 slots_valid); > > > /** > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs.c > b/src/mesa/drivers/dri/i965/brw_vec4_gs.c > index 7ab03ac..94c4017 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_gs.c > +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs.c > @@ -62,9 +62,18 @@ do_gs_prog(struct brw_context *brw, > c.prog_data.base.param = rzalloc_array(NULL, const float *, param_count); > c.prog_data.base.pull_param = rzalloc_array(NULL, const float *, > param_count); > > - brw_compute_vue_map(brw, &c.prog_data.base.vue_map, > - gp->program.Base.OutputsWritten, > - c.key.base.userclip_active); > + GLbitfield64 outputs_written = gp->program.Base.OutputsWritten; > + > + /* In order for legacy clipping to work, we need to populate the clip > + * distance varying slots whenever clipping is enabled, even if the vertex > + * shader doesn't write to gl_ClipDistance. > + */ > + if (c.key.base.userclip_active) { > + outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST0); > + outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST1); > + } > + > + brw_compute_vue_map(brw, &c.prog_data.base.vue_map, outputs_written);
Having both of the callers of brw_compute_vue_map do this same dance before calling seems weird. I want to understand this better... The VS and GS code adds clip distance as a written output because fixed function hardware is going to read it when user clipping is enabled. This then gets captured in the vue_map... so when the next patch adds another caller to brw_compute_vue_map, it doesn't need to know that user clipping was enabled (vs. gl_ClipDistance being written). Yeah? > /* Compute the output vertex size. > * > diff --git a/src/mesa/drivers/dri/i965/brw_vs.c > b/src/mesa/drivers/dri/i965/brw_vs.c > index b81a538..6b97f01 100644 > --- a/src/mesa/drivers/dri/i965/brw_vs.c > +++ b/src/mesa/drivers/dri/i965/brw_vs.c > @@ -52,14 +52,10 @@ static inline void assign_vue_slot(struct brw_vue_map > *vue_map, > > /** > * Compute the VUE map for vertex shader program. > - * > - * Note that consumers of this map using cache keys must include > - * prog_data->userclip and prog_data->outputs_written in their key > - * (generated by CACHE_NEW_VS_PROG). > */ > void > brw_compute_vue_map(struct brw_context *brw, struct brw_vue_map *vue_map, > - GLbitfield64 slots_valid, bool userclip_active) > + GLbitfield64 slots_valid) > { > vue_map->slots_valid = slots_valid; > int i; > @@ -107,10 +103,11 @@ brw_compute_vue_map(struct brw_context *brw, struct > brw_vue_map *vue_map, > */ > assign_vue_slot(vue_map, VARYING_SLOT_PSIZ); > assign_vue_slot(vue_map, VARYING_SLOT_POS); > - if (userclip_active) { > + if (slots_valid & BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST0)) > assign_vue_slot(vue_map, VARYING_SLOT_CLIP_DIST0); > + if (slots_valid & BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST1)) > assign_vue_slot(vue_map, VARYING_SLOT_CLIP_DIST1); > - } > + > /* front and back colors need to be consecutive so that we can use > * ATTRIBUTE_SWIZZLE_INPUTATTR_FACING to swizzle them when doing > * two-sided color. > @@ -267,15 +264,18 @@ do_vs_prog(struct brw_context *brw, > outputs_written |= BITFIELD64_BIT(VARYING_SLOT_COL0); > if (outputs_written & BITFIELD64_BIT(VARYING_SLOT_BFC1)) > outputs_written |= BITFIELD64_BIT(VARYING_SLOT_COL1); > + } > > - if (c.key.base.userclip_active) { > - outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST0); > - outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST1); > - } > + /* In order for legacy clipping to work, we need to populate the clip > + * distance varying slots whenever clipping is enabled, even if the vertex > + * shader doesn't write to gl_ClipDistance. > + */ > + if (c.key.base.userclip_active) { > + outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST0); > + outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST1); > } > > - brw_compute_vue_map(brw, &prog_data.base.vue_map, outputs_written, > - c.key.base.userclip_active); > + brw_compute_vue_map(brw, &prog_data.base.vue_map, outputs_written); > > if (0) { > _mesa_fprint_program_opt(stdout, &c.vp->program.Base, PROG_PRINT_DEBUG, > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev