Looks good Kristian. This has been worthy of cleanup for some time... Keith
2010/10/13 Kristian Høgsberg <k...@bitplanet.net>: > 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 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
