[Mesa-dev] [PATCH 3/3] mesa: add gl_context::NewDriverState and use it for vertex arrays

[Marek Olšák]

On Mon, May 7, 2012 at 6:19 PM, Brian Paul <brian.e.paul at gmail.com> wrote:
> Looks good, but it took me a while to understand exactly what's going
> on.  Suggestions for more/improved comments below.
>
> On Tue, Apr 24, 2012 at 4:00 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> The vbo module recomputes its states if _NEW_ARRAY is set, so it shouldn't use
>> the same flag to notify the driver. Since we've run out of bits in NewState
>> and NewState is for core Mesa anyway, we need to find another way.
>>
>> This patch is the first to start decoupling the state flags meant only
>> for core Mesa and those only for drivers.
>>
>> The idea is to have two flag sets:
>> - gl_context::NewState - used by core Mesa only
>> - gl_context::NewDriverState - used by drivers only
>
> "used by drivers only.  The flags defined by the driver and
> opaque/meaningless to core Mesa."
>
>
>>
>> It makes perfect sense to use NewState|=_NEW_ARRAY to notify the vbo module
>> that the user changed vertex arrays, and the vbo module in turn sets
>> a driver-specific flag to notify the driver that it should update its vertex
>> array bindings.
>>
>> The driver decides which bits of NewDriverState should be set and stores them
>> in gl_context::DriverFlags. Then, Core Mesa can do this:
>> ctx->NewDriverState |= ctx->DriverFlags.NewArray;
>>
>> This patch implements this behavior and adapts st/mesa.
>> DriverFlags.NewArray is set to ST_NEW_VERTEX_ARRAYS.
>>
>> Core Mesa only sets NewDriverState. It's the driver's responsibility to read
>> it whenever it wants and reset it to 0.
>> ---
>>  src/mesa/main/context.c                  |    2 ++
>>  src/mesa/main/mtypes.h                   |   14 ++++++++++++++
>>  src/mesa/state_tracker/st_cb_rasterpos.c |    5 ++++-
>>  src/mesa/state_tracker/st_context.c      |    6 ++++++
>>  src/mesa/state_tracker/st_context.h      |    1 +
>>  src/mesa/state_tracker/st_draw.c         |   12 +++++++++---
>>  src/mesa/vbo/vbo_context.h               |    2 +-
>>  src/mesa/vbo/vbo_exec_array.c            |    2 +-
>>  src/mesa/vbo/vbo_exec_draw.c             |    2 +-
>>  src/mesa/vbo/vbo_rebase.c                |    2 ++
>>  src/mesa/vbo/vbo_save_draw.c             |    2 +-
>>  src/mesa/vbo/vbo_split_copy.c            |    2 ++
>>  src/mesa/vbo/vbo_split_inplace.c         |    2 ++
>>  13 files changed, 46 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
>> index d75351c..7e2ac98 100644
>> --- a/src/mesa/main/context.c
>> +++ b/src/mesa/main/context.c
>> @@ -792,6 +792,7 @@ init_attrib_groups(struct gl_context *ctx)
>>
>>    /* Miscellaneous */
>>    ctx->NewState = _NEW_ALL;
>> +   ctx->NewDriverState = ~0;
>>    ctx->ErrorValue = (GLenum) GL_NO_ERROR;
>>    ctx->ResetStatus = (GLenum) GL_NO_ERROR;
>>    ctx->varying_vp_inputs = VERT_BIT_ALL;
>> @@ -1290,6 +1291,7 @@ _mesa_copy_context( const struct gl_context *src, struct gl_context *dst,
>>    /* XXX FIXME:  Call callbacks?
>>     */
>>    dst->NewState = _NEW_ALL;
>> +   dst->NewDriverState = ~0;
>>  }
>>  #endif
>>
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index eb103ad..7f01514 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -3255,6 +3255,17 @@ typedef enum
>>    API_OPENGLES2
>>  } gl_api;
>>
>> +/**
>> + * Driver-specific state flags.
>> + *
>> + * These are or'd with gl_context::NewDriverState to notify a driver about
>> + * a state change. The driver gets to decide what bits should be set through
>> + * this structure.
>
> Just to be clear (and add to the comment), the bits here are set once
> by the driver during context creation and never changed, right?  Also,

Yes, that's right.

> could you make it a bit more clear that the values of the flags are
> defined by the driver and opaque to core Mesa?

Okay I will do that.

Marek