[Mesa-dev] [PATCH V2 4/8] mesa: stop passing state bitfield to UpdateState()
Samuel Pitoiset
samuel.pitoiset at gmail.com
Thu Jun 8 07:39:56 UTC 2017
Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
On 06/08/2017 01:39 AM, Timothy Arceri wrote:
> The code comment which seems to have been added in cab974cf6c2db
> (from year 2000) says:
>
> "Set ctx->NewState to zero to avoid recursion if
> Driver.UpdateState() has to call FLUSH_VERTICES(). (fixed?)"
>
> As far as I can tell nothing in any of the UpdateState() calls
> should cause it to be called recursively.
>
> V2: add a wrapper around the osmesa update function so it can still
> be used internally.
> ---
> src/mesa/drivers/dri/i915/i915_context.c | 4 +++-
> src/mesa/drivers/dri/i915/intel_context.c | 3 ++-
> src/mesa/drivers/dri/i965/brw_context.c | 3 ++-
> src/mesa/drivers/dri/nouveau/nouveau_state.c | 3 ++-
> src/mesa/drivers/dri/r200/r200_state.c | 4 +++-
> src/mesa/drivers/dri/radeon/radeon_state.c | 4 +++-
> src/mesa/drivers/dri/swrast/swrast.c | 4 +++-
> src/mesa/drivers/osmesa/osmesa.c | 9 +++++++--
> src/mesa/drivers/x11/xm_dd.c | 5 +++--
> src/mesa/drivers/x11/xmesaP.h | 4 ----
> src/mesa/main/dd.h | 2 +-
> src/mesa/main/state.c | 7 ++-----
> src/mesa/state_tracker/st_context.c | 3 ++-
> 13 files changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i915/i915_context.c b/src/mesa/drivers/dri/i915/i915_context.c
> index 6c48823..1406b65 100644
> --- a/src/mesa/drivers/dri/i915/i915_context.c
> +++ b/src/mesa/drivers/dri/i915/i915_context.c
> @@ -45,22 +45,24 @@
> #include "i915_reg.h"
> #include "i915_program.h"
>
> /***************************************
> * Mesa's Driver Functions
> ***************************************/
>
> /* Override intel default.
> */
> static void
> -i915InvalidateState(struct gl_context * ctx, GLuint new_state)
> +i915InvalidateState(struct gl_context * ctx)
> {
> + GLuint new_state = ctx->NewState;
> +
> _swrast_InvalidateState(ctx, new_state);
> _swsetup_InvalidateState(ctx, new_state);
> _vbo_InvalidateState(ctx, new_state);
> _tnl_InvalidateState(ctx, new_state);
> _tnl_invalidate_vertex_state(ctx, new_state);
> intel_context(ctx)->NewGLState |= new_state;
>
> /* Todo: gather state values under which tracked parameters become
> * invalidated, add callbacks for things like
> * ProgramLocalParameters, etc.
> diff --git a/src/mesa/drivers/dri/i915/intel_context.c b/src/mesa/drivers/dri/i915/intel_context.c
> index 5607d5b..6c59b42 100644
> --- a/src/mesa/drivers/dri/i915/intel_context.c
> +++ b/src/mesa/drivers/dri/i915/intel_context.c
> @@ -307,22 +307,23 @@ static const struct debug_control debug_control[] = {
> { "sync", DEBUG_SYNC},
> { "dri", DEBUG_DRI },
> { "stats", DEBUG_STATS },
> { "wm", DEBUG_WM },
> { "aub", DEBUG_AUB },
> { NULL, 0 }
> };
>
>
> static void
> -intelInvalidateState(struct gl_context * ctx, GLuint new_state)
> +intelInvalidateState(struct gl_context * ctx)
> {
> + GLuint new_state = ctx->NewState;
> struct intel_context *intel = intel_context(ctx);
>
> if (ctx->swrast_context)
> _swrast_InvalidateState(ctx, new_state);
> _vbo_InvalidateState(ctx, new_state);
>
> intel->NewGLState |= new_state;
>
> if (intel->vtbl.invalidate_state)
> intel->vtbl.invalidate_state( intel, new_state );
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 3e3fe2d..8525d53 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -215,22 +215,23 @@ intel_texture_view_requires_resolve(struct brw_context *brw,
> _mesa_get_format_name(intel_tex->mt->format));
>
> if (intel_disable_rb_aux_buffer(brw, intel_tex->mt->bo))
> perf_debug("Sampling renderbuffer with non-compressible format - "
> "turning off compression");
>
> return true;
> }
>
> static void
> -intel_update_state(struct gl_context * ctx, GLuint new_state)
> +intel_update_state(struct gl_context * ctx)
> {
> + GLuint new_state = ctx->NewState;
> struct brw_context *brw = brw_context(ctx);
> struct intel_texture_object *tex_obj;
> struct intel_renderbuffer *depth_irb;
>
> if (ctx->swrast_context)
> _swrast_InvalidateState(ctx, new_state);
> _vbo_InvalidateState(ctx, new_state);
>
> brw->NewGLState |= new_state;
>
> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_state.c b/src/mesa/drivers/dri/nouveau/nouveau_state.c
> index de36fa4..567f32f 100644
> --- a/src/mesa/drivers/dri/nouveau/nouveau_state.c
> +++ b/src/mesa/drivers/dri/nouveau/nouveau_state.c
> @@ -444,22 +444,23 @@ nouveau_state_emit(struct gl_context *ctx)
>
> while ((i = nouveau_next_dirty_state(ctx)) >= 0) {
> BITSET_CLEAR(nctx->dirty, i);
> drv->emit[i](ctx, i);
> }
>
> BITSET_ZERO(nctx->dirty);
> }
>
> static void
> -nouveau_update_state(struct gl_context *ctx, GLbitfield new_state)
> +nouveau_update_state(struct gl_context *ctx)
> {
> + GLbitfield new_state = ctx->NewState;
> int i;
>
> if (new_state & (_NEW_PROJECTION | _NEW_MODELVIEW))
> context_dirty(ctx, PROJECTION);
>
> if (new_state & _NEW_MODELVIEW)
> context_dirty(ctx, MODELVIEW);
>
> if (new_state & _NEW_TEXTURE_MATRIX) {
> for (i = 0; i < ctx->Const.MaxTextureCoordUnits; i++)
> diff --git a/src/mesa/drivers/dri/r200/r200_state.c b/src/mesa/drivers/dri/r200/r200_state.c
> index 9fb15f2..d5a6f09 100644
> --- a/src/mesa/drivers/dri/r200/r200_state.c
> +++ b/src/mesa/drivers/dri/r200/r200_state.c
> @@ -2269,22 +2269,24 @@ GLboolean r200ValidateState( struct gl_context *ctx )
> r200SetupVertexProg( ctx );
> }
> else TCL_FALLBACK(ctx, R200_TCL_FALLBACK_VERTEX_PROGRAM, 0);
> }
>
> rmesa->radeon.NewGLState = 0;
> return GL_TRUE;
> }
>
>
> -static void r200InvalidateState( struct gl_context *ctx, GLuint new_state )
> +static void r200InvalidateState(struct gl_context *ctx)
> {
> + GLuint new_state = ctx->NewState;
> +
> r200ContextPtr rmesa = R200_CONTEXT(ctx);
>
> _swrast_InvalidateState( ctx, new_state );
> _swsetup_InvalidateState( ctx, new_state );
> _vbo_InvalidateState( ctx, new_state );
> _tnl_InvalidateState( ctx, new_state );
> R200_CONTEXT(ctx)->radeon.NewGLState |= new_state;
>
> if (new_state & _NEW_PROGRAM)
> rmesa->curr_vp_hw = NULL;
> diff --git a/src/mesa/drivers/dri/radeon/radeon_state.c b/src/mesa/drivers/dri/radeon/radeon_state.c
> index 1baf229..ff2a708 100644
> --- a/src/mesa/drivers/dri/radeon/radeon_state.c
> +++ b/src/mesa/drivers/dri/radeon/radeon_state.c
> @@ -2037,22 +2037,24 @@ GLboolean radeonValidateState( struct gl_context *ctx )
> radeonUpdateClipPlanes( ctx );
> }
>
>
> rmesa->radeon.NewGLState = 0;
>
> return GL_TRUE;
> }
>
>
> -static void radeonInvalidateState( struct gl_context *ctx, GLuint new_state )
> +static void radeonInvalidateState(struct gl_context *ctx)
> {
> + GLuint new_state = ctx->NewState;
> +
> _swrast_InvalidateState( ctx, new_state );
> _swsetup_InvalidateState( ctx, new_state );
> _vbo_InvalidateState( ctx, new_state );
> _tnl_InvalidateState( ctx, new_state );
> R100_CONTEXT(ctx)->radeon.NewGLState |= new_state;
> }
>
>
> /* A hack. Need a faster way to find this out.
> */
> diff --git a/src/mesa/drivers/dri/swrast/swrast.c b/src/mesa/drivers/dri/swrast/swrast.c
> index de1fe4c..a68f7a0 100644
> --- a/src/mesa/drivers/dri/swrast/swrast.c
> +++ b/src/mesa/drivers/dri/swrast/swrast.c
> @@ -690,22 +690,24 @@ get_string(struct gl_context *ctx, GLenum pname)
> case GL_VENDOR:
> return (const GLubyte *) swrast_vendor_string;
> case GL_RENDERER:
> return (const GLubyte *) swrast_renderer_string;
> default:
> return NULL;
> }
> }
>
> static void
> -update_state( struct gl_context *ctx, GLuint new_state )
> +update_state(struct gl_context *ctx)
> {
> + GLuint new_state = ctx->NewState;
> +
> /* not much to do here - pass it on */
> _swrast_InvalidateState( ctx, new_state );
> _swsetup_InvalidateState( ctx, new_state );
> _vbo_InvalidateState( ctx, new_state );
> _tnl_InvalidateState( ctx, new_state );
> }
>
> static void
> viewport(struct gl_context *ctx)
> {
> diff --git a/src/mesa/drivers/osmesa/osmesa.c b/src/mesa/drivers/osmesa/osmesa.c
> index a3d4fac..ed69353 100644
> --- a/src/mesa/drivers/osmesa/osmesa.c
> +++ b/src/mesa/drivers/osmesa/osmesa.c
> @@ -110,29 +110,34 @@ get_string( struct gl_context *ctx, GLenum name )
> #else
> return (const GLubyte *) "Mesa OffScreen";
> #endif
> default:
> return NULL;
> }
> }
>
>
> static void
> -osmesa_update_state( struct gl_context *ctx, GLuint new_state )
> +osmesa_update_state(struct gl_context *ctx, GLuint new_state)
> {
> /* easy - just propogate */
> _swrast_InvalidateState( ctx, new_state );
> _swsetup_InvalidateState( ctx, new_state );
> _tnl_InvalidateState( ctx, new_state );
> _vbo_InvalidateState( ctx, new_state );
> }
>
> +static void
> +osmesa_update_state_wrapper(struct gl_context *ctx)
> +{
> + osmesa_update_state(ctx, ctx->NewState);
> +}
>
>
> /**
> * Macros for optimized line/triangle rendering.
> * Only for 8-bit channel, RGBA, BGRA, ARGB formats.
> */
>
> #define PACK_RGBA(DST, R, G, B, A) \
> do { \
> (DST)[osmesa->rInd] = R; \
> @@ -821,21 +826,21 @@ OSMesaCreateContextAttribs(const int *attribList, OSMesaContext sharelist)
> );
> if (!osmesa->gl_visual) {
> free(osmesa);
> return NULL;
> }
>
> /* Initialize device driver function table */
> _mesa_init_driver_functions(&functions);
> /* override with our functions */
> functions.GetString = get_string;
> - functions.UpdateState = osmesa_update_state;
> + functions.UpdateState = osmesa_update_state_wrapper;
>
> if (!_mesa_initialize_context(&osmesa->mesa,
> api_profile,
> osmesa->gl_visual,
> sharelist ? &sharelist->mesa
> : (struct gl_context *) NULL,
> &functions)) {
> _mesa_destroy_visual( osmesa->gl_visual );
> free(osmesa);
> return NULL;
> diff --git a/src/mesa/drivers/x11/xm_dd.c b/src/mesa/drivers/x11/xm_dd.c
> index cd5809e..e06831c 100644
> --- a/src/mesa/drivers/x11/xm_dd.c
> +++ b/src/mesa/drivers/x11/xm_dd.c
> @@ -671,23 +671,24 @@ enable( struct gl_context *ctx, GLenum pname, GLboolean state )
> default:
> ; /* silence compiler warning */
> }
> }
>
>
> /**
> * Called when the driver should update its state, based on the new_state
> * flags.
> */
> -void
> -xmesa_update_state( struct gl_context *ctx, GLbitfield new_state )
> +static void
> +xmesa_update_state(struct gl_context *ctx)
> {
> + GLbitfield new_state = ctx->NewState;
> const XMesaContext xmesa = XMESA_CONTEXT(ctx);
>
> /* Propagate statechange information to swrast and swrast_setup
> * modules. The X11 driver has no internal GL-dependent state.
> */
> _swrast_InvalidateState( ctx, new_state );
> _tnl_InvalidateState( ctx, new_state );
> _vbo_InvalidateState( ctx, new_state );
> _swsetup_InvalidateState( ctx, new_state );
>
> diff --git a/src/mesa/drivers/x11/xmesaP.h b/src/mesa/drivers/x11/xmesaP.h
> index 6cd020f..40d6734 100644
> --- a/src/mesa/drivers/x11/xmesaP.h
> +++ b/src/mesa/drivers/x11/xmesaP.h
> @@ -347,24 +347,20 @@ xmesa_get_window_size(XMesaDisplay *dpy, XMesaBuffer b,
> GLuint *width, GLuint *height);
>
> extern void
> xmesa_check_and_update_buffer_size(XMesaContext xmctx, XMesaBuffer drawBuffer);
>
> extern void
> xmesa_init_driver_functions( XMesaVisual xmvisual,
> struct dd_function_table *driver );
>
> extern void
> -xmesa_update_state( struct gl_context *ctx, GLbitfield new_state );
> -
> -
> -extern void
> xmesa_MapRenderbuffer(struct gl_context *ctx,
> struct gl_renderbuffer *rb,
> GLuint x, GLuint y, GLuint w, GLuint h,
> GLbitfield mode,
> GLubyte **mapOut, GLint *rowStrideOut);
>
> extern void
> xmesa_UnmapRenderbuffer(struct gl_context *ctx, struct gl_renderbuffer *rb);
>
> extern void
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index 3f31025..0b262d0 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -86,21 +86,21 @@ struct dd_function_table {
> * returned.
> */
> const GLubyte * (*GetString)( struct gl_context *ctx, GLenum name );
>
> /**
> * Notify the driver after Mesa has made some internal state changes.
> *
> * This is in addition to any state change callbacks Mesa may already have
> * made.
> */
> - void (*UpdateState)( struct gl_context *ctx, GLbitfield new_state );
> + void (*UpdateState)(struct gl_context *ctx);
>
> /**
> * This is called whenever glFinish() is called.
> */
> void (*Finish)( struct gl_context *ctx );
>
> /**
> * This is called whenever glFlush() is called.
> */
> void (*Flush)( struct gl_context *ctx );
> diff --git a/src/mesa/main/state.c b/src/mesa/main/state.c
> index 73872b8..1de1d69 100644
> --- a/src/mesa/main/state.c
> +++ b/src/mesa/main/state.c
> @@ -408,27 +408,24 @@ _mesa_update_state_locked( struct gl_context *ctx )
> _mesa_update_vao_client_arrays(ctx, ctx->Array.VAO);
>
> out:
> new_prog_state |= update_program_constants(ctx);
>
> /*
> * Give the driver a chance to act upon the new_state flags.
> * The driver might plug in different span functions, for example.
> * Also, this is where the driver can invalidate the state of any
> * active modules (such as swrast_setup, swrast, tnl, etc).
> - *
> - * Set ctx->NewState to zero to avoid recursion if
> - * Driver.UpdateState() has to call FLUSH_VERTICES(). (fixed?)
> */
> - new_state = ctx->NewState | new_prog_state;
> + ctx->NewState |= new_prog_state;
> + ctx->Driver.UpdateState(ctx);
> ctx->NewState = 0;
> - ctx->Driver.UpdateState(ctx, new_state);
> ctx->Array.VAO->NewArrays = 0x0;
> }
>
>
> /* This is the usual entrypoint for state updates:
> */
> void
> _mesa_update_state( struct gl_context *ctx )
> {
> _mesa_lock_context_textures(ctx);
> diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
> index 3207e95..058d9c3 100644
> --- a/src/mesa/state_tracker/st_context.c
> +++ b/src/mesa/state_tracker/st_context.c
> @@ -158,22 +158,23 @@ st_get_active_states(struct gl_context *ctx)
>
> /* Mark non-shader-resource shader states as "always active". */
> return active_shader_states | ~ST_ALL_SHADER_RESOURCES;
> }
>
>
> /**
> * Called via ctx->Driver.UpdateState()
> */
> static void
> -st_invalidate_state(struct gl_context * ctx, GLbitfield new_state)
> +st_invalidate_state(struct gl_context * ctx)
> {
> + GLbitfield new_state = ctx->NewState;
> struct st_context *st = st_context(ctx);
>
> if (new_state & _NEW_BUFFERS) {
> st_invalidate_buffers(st);
> } else {
> /* These set a subset of flags set by _NEW_BUFFERS, so we only have to
> * check them when _NEW_BUFFERS isn't set.
> */
> if (new_state & (_NEW_DEPTH |
> _NEW_STENCIL))
>
More information about the mesa-dev
mailing list