2010/10/13 Kristian Høgsberg <k...@bitplanet.net>: > Just always check for FLUSH_UPDATE_CURRENT and call Driver.BeginVertices > when necessary. By using the unlikely() macros, this ends up as > a 10% performance improvement (for isosurf, anyway) over the old, > complicated function pointer swapping. > ---
I should say that this also fixes the bug we discussed a few weeks back: http://lists.freedesktop.org/archives/mesa-dev/2010-September/002950.html The root cause, it turns out, was that I forgot to set up the neutral tnl module in case of a DRI driver that supports both GL and GLES (ie has FEATURE_beginend set) but is used for a GLES2 context. That would have been a more minimal patch, but this gets rid of the pointer swapping and is faster. Kristian > src/mesa/main/context.c | 5 --- > src/mesa/main/mtypes.h | 29 ------------------ > src/mesa/main/vtxfmt.c | 67 +----------------------------------------- > src/mesa/vbo/vbo_exec_api.c | 14 ++++---- > 4 files changed, 9 insertions(+), 106 deletions(-) > > diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c > index 41f30ca..bb2dbf4 100644 > --- a/src/mesa/main/context.c > +++ b/src/mesa/main/context.c > @@ -949,11 +949,6 @@ _mesa_initialize_context_for_api(struct gl_context *ctx, > > switch (ctx->API) { > case API_OPENGL: > - /* Neutral tnl module stuff */ > - _mesa_init_exec_vtxfmt( ctx ); > - ctx->TnlModule.Current = NULL; > - ctx->TnlModule.SwapCount = 0; > - > #if FEATURE_dlist > ctx->Save = _mesa_create_save_table(); > if (!ctx->Save) { > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h > index aace09d..6702032 100644 > --- a/src/mesa/main/mtypes.h > +++ b/src/mesa/main/mtypes.h > @@ -2942,32 +2942,6 @@ struct gl_matrix_stack > #include "dd.h" > > > -#define NUM_VERTEX_FORMAT_ENTRIES (sizeof(GLvertexformat) / sizeof(void *)) > - > -/** > - * Core Mesa's support for tnl modules: > - */ > -struct gl_tnl_module > -{ > - /** > - * Vertex format to be lazily swapped into current dispatch. > - */ > - const GLvertexformat *Current; > - > - /** > - * \name Record of functions swapped out. > - * On restore, only need to swap these functions back in. > - */ > - /*...@{*/ > - struct { > - _glapi_proc * location; > - _glapi_proc function; > - } Swapped[NUM_VERTEX_FORMAT_ENTRIES]; > - GLuint SwapCount; > - /*...@}*/ > -}; > - > - > /** > * Display list flags. > * Strictly this is a tnl-private concept, but it doesn't seem > @@ -3231,9 +3205,6 @@ struct gl_context > */ > GLboolean mvp_with_dp4; > > - /** Core tnl module support */ > - struct gl_tnl_module TnlModule; > - > /** > * \name Hooks for module contexts. > * > diff --git a/src/mesa/main/vtxfmt.c b/src/mesa/main/vtxfmt.c > index 284e777..887c7d9 100644 > --- a/src/mesa/main/vtxfmt.c > +++ b/src/mesa/main/vtxfmt.c > @@ -34,51 +34,11 @@ > #include "vtxfmt.h" > #include "eval.h" > #include "dlist.h" > +#include "main/dispatch.h" > > > #if FEATURE_beginend > > - > -/* The neutral vertex format. This wraps all tnl module functions, > - * verifying that the currently-installed module is valid and then > - * installing the function pointers in a lazy fashion. It records the > - * function pointers that have been swapped out, which allows a fast > - * restoration of the neutral module in almost all cases -- a typical > - * app might only require 4-6 functions to be modified from the neutral > - * baseline, and only restoring these is certainly preferable to doing > - * the entire module's 60 or so function pointers. > - */ > - > -#define PRE_LOOPBACK( FUNC ) \ > -{ \ > - GET_CURRENT_CONTEXT(ctx); \ > - struct gl_tnl_module * const tnl = &(ctx->TnlModule); \ > - const int tmp_offset = _gloffset_ ## FUNC ; \ > - \ > - ASSERT( tnl->Current ); \ > - ASSERT( tnl->SwapCount < NUM_VERTEX_FORMAT_ENTRIES ); \ > - ASSERT( tmp_offset >= 0 ); \ > - \ > - if (tnl->SwapCount == 0) \ > - ctx->Driver.BeginVertices( ctx ); \ > - \ > - /* Save the swapped function's dispatch entry so it can be */ \ > - /* restored later. */ \ > - tnl->Swapped[tnl->SwapCount].location = & (((_glapi_proc > *)ctx->Exec)[tmp_offset]); \ > - tnl->Swapped[tnl->SwapCount].function = (_glapi_proc)TAG(FUNC); \ > - tnl->SwapCount++; \ > - \ > - if ( 0 ) \ > - _mesa_debug(ctx, " swapping gl" #FUNC"...\n" ); > \ > - \ > - /* Install the tnl function pointer. */ > \ > - SET_ ## FUNC(ctx->Exec, tnl->Current->FUNC); > \ > -} > - > -#define TAG(x) neutral_##x > -#include "vtxfmt_tmp.h" > - > - > /** > * Use the per-vertex functions found in <vfmt> to initialze the given > * API dispatch table. > @@ -165,17 +125,9 @@ install_vtxfmt( struct _glapi_table *tab, const > GLvertexformat *vfmt ) > } > > > -void _mesa_init_exec_vtxfmt( struct gl_context *ctx ) > -{ > - install_vtxfmt( ctx->Exec, &neutral_vtxfmt ); > - ctx->TnlModule.SwapCount = 0; > -} > - > - > void _mesa_install_exec_vtxfmt( struct gl_context *ctx, const GLvertexformat > *vfmt ) > { > - ctx->TnlModule.Current = vfmt; > - _mesa_restore_exec_vtxfmt( ctx ); > + install_vtxfmt( ctx->Exec, vfmt ); > } > > > @@ -185,19 +137,4 @@ void _mesa_install_save_vtxfmt( struct gl_context *ctx, > const GLvertexformat *vf > } > > > -void _mesa_restore_exec_vtxfmt( struct gl_context *ctx ) > -{ > - struct gl_tnl_module *tnl = &(ctx->TnlModule); > - GLuint i; > - > - /* Restore the neutral tnl module wrapper. > - */ > - for ( i = 0 ; i < tnl->SwapCount ; i++ ) { > - *(tnl->Swapped[i].location) = tnl->Swapped[i].function; > - } > - > - tnl->SwapCount = 0; > -} > - > - > #endif /* FEATURE_beginend */ > diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c > index 5070b35..1b31eed 100644 > --- a/src/mesa/vbo/vbo_exec_api.c > +++ b/src/mesa/vbo/vbo_exec_api.c > @@ -358,10 +358,12 @@ static void vbo_exec_fixup_vertex( struct gl_context > *ctx, > #define ATTR( A, N, V0, V1, V2, V3 ) \ > do { \ > struct vbo_exec_context *exec = &vbo_context(ctx)->exec; \ > - \ > - if (exec->vtx.active_sz[A] != N) \ > - vbo_exec_fixup_vertex(ctx, A, N); \ > - \ > + \ > + if (unlikely(exec->vtx.active_sz[A] != N)) \ > + vbo_exec_fixup_vertex(ctx, A, N); > \ > + if (unlikely(!(exec->ctx->Driver.NeedFlush & FLUSH_UPDATE_CURRENT))) \ > + ctx->Driver.BeginVertices( ctx ); \ > + \ > { \ > GLfloat *dest = exec->vtx.attrptr[A]; \ > if (N>0) dest[0] = V0; \ > @@ -911,10 +913,8 @@ void vbo_exec_FlushVertices( struct gl_context *ctx, > GLuint flags ) > > /* Need to do this to ensure BeginVertices gets called again: > */ > - if (exec->ctx->Driver.NeedFlush & FLUSH_UPDATE_CURRENT) { > - _mesa_restore_exec_vtxfmt( ctx ); > + if (exec->ctx->Driver.NeedFlush & FLUSH_UPDATE_CURRENT) > exec->ctx->Driver.NeedFlush &= ~FLUSH_UPDATE_CURRENT; > - } > > exec->ctx->Driver.NeedFlush &= ~flags; > > -- > 1.7.3.1 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
