[Mesa-dev] [PATCH] Drop the "neutral" tnl module
Keith Whitwell
keith.whitwell at gmail.com
Wed Oct 13 15:04:02 PDT 2010
Looks good Kristian. This has been worthy of cleanup for some time...
Keith
2010/10/13 Kristian Høgsberg <krh at bitplanet.net>:
> 2010/10/13 Kristian Høgsberg <krh at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list