[Mesa-dev] [PATCH 08/15] i965/gen6+: Remove VUE map dependency on userclip_active.
Ian Romanick
idr at freedesktop.org
Mon Sep 9 13:24:36 PDT 2013
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,
>
More information about the mesa-dev
mailing list