On 29 August 2013 15:46, Eric Anholt <e...@anholt.net> wrote:
> Kenneth Graunke <kenn...@whitecape.org> writes:
>
> > 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.
>
> brw_stage_state[_base]? "stage" is the terminology used in ARB_sso (and
> in our conversations), and this looks like it's the common state among
> all of vs/gs/fs. pipeline is for a group of the things, for sure.
>
> On the other hand, we've been using "shader" as the variable name to
> store the per-stage thing, and shader_program for the whole-pipeline
> thing, so brw_shader_state seems fine to me too.
>
Ken and I like brw_stage_state[_base]. Ken prefers without _base, I have a
minor preference for _base, but we both feel like that's a detail that can
be decided on the fly when I get around to doing the renaming.
I'll plan on changing the name to brw_state_state[_base] tomorrow morning
unless I hear objections.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
