[Mesa-dev] [PATCH V2 4/8] mesa: stop passing state bitfield to UpdateState()

Brian Paul brianp at vmware.com
Thu Jun 8 04:06:16 UTC 2017


Reviewed-by: Brian Paul <brianp at vmware.com>

On 06/07/2017 05:39 PM, 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