[Mesa-dev] [PATCH 05/22] i965: Move data from brw->vs into a base class if gs will also need it.

[Kenneth Graunke]

On 08/26/2013 03:12 PM, Paul Berry wrote:
> This paves the way for sharing the code that will set up the vertex
> and geometry shader pipeline state.
> ---
>   src/mesa/drivers/dri/i965/brw_context.h          | 47 ++++++++++++++----------
>   src/mesa/drivers/dri/i965/brw_draw.c             |  3 +-
>   src/mesa/drivers/dri/i965/brw_misc_state.c       |  6 +--
>   src/mesa/drivers/dri/i965/brw_vs.c               | 12 +++---
>   src/mesa/drivers/dri/i965/brw_vs_state.c         | 24 ++++++------
>   src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 43 ++++++++++++----------
>   src/mesa/drivers/dri/i965/brw_vtbl.c             |  2 +-
>   src/mesa/drivers/dri/i965/brw_wm_sampler_state.c |  8 ++--
>   src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  4 +-
>   src/mesa/drivers/dri/i965/gen6_sampler_state.c   |  2 +-
>   src/mesa/drivers/dri/i965/gen6_vs_state.c        | 23 +++++++-----
>   src/mesa/drivers/dri/i965/gen7_vs_state.c        | 18 +++++----
>   12 files changed, 107 insertions(+), 85 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index dcd4c9a..9784956 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -818,6 +818,32 @@ struct brw_query_object {
>
>
>   /**
> + * Data shared between brw_context::vs and brw_context::gs
> + */
> +struct brw_vec4_context_base
> +{
> +   drm_intel_bo *scratch_bo;
> +   drm_intel_bo *const_bo;
> +   /** Offset in the program cache to the program */
> +   uint32_t prog_offset;
> +   uint32_t state_offset;
> +
> +   uint32_t push_const_offset; /* Offset in the batchbuffer */
> +   int push_const_size; /* in 256-bit register increments */
> +
> +   uint32_t bind_bo_offset;
> +   uint32_t surf_offset[BRW_MAX_VEC4_SURFACES];
> +
> +   /** SAMPLER_STATE count and table offset */
> +   uint32_t sampler_count;
> +   uint32_t sampler_offset;
> +
> +   /** Offsets in the batch to sampler default colors (texture border color) */
> +   uint32_t sdc_offset[BRW_MAX_TEX_UNIT];
> +};

I like what this patch is doing, but I really don't like the names.

With the exception of ralloc, "context"/"ctx" generally always mean the 
global GL context: gl_context or a subclass like brw_context.  (For 
ralloc, we inherited the "context" terminology from talloc, so it kind 
of stuck.)  vec4_ctx/brw_vec4_context_base are something totally different.

This is a structure that represents the shader program state for a 
particular pipeline stage.  Also, other than BRW_MAX_VEC4_SURFACES, 
there's nothing vec4 specific about this at all.  The pixel shader could 
use every one of these fields (and should eventually).  So I dislike 
"vec4" in the name - we're just going to have to change it.

I had suggested names like brw_shader_state or brw_pipeline_state...I'm 
open to other ideas.

--Ken