[Mesa-dev] [PATCH 05/12] mesa: Install a minimal dispatch table during glBegin()/glEnd().

Brian Paul brianp at vmware.com
Fri Jan 18 15:55:45 PST 2013


On 01/18/2013 03:30 PM, Eric Anholt wrote:
> This is a step toward getting rid of ASSERT_OUTSIDE_BEGIN_END() in Mesa.
> ---
>   src/mesa/main/context.c     |   84 ++++++++++++++++++++++++++++++++++---------
>   src/mesa/main/mtypes.h      |   25 +++++++++++--
>   src/mesa/main/vtxfmt.c      |    2 ++
>   src/mesa/vbo/vbo_exec_api.c |   17 +++++++++
>   4 files changed, 109 insertions(+), 19 deletions(-)
>
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index 561eb46..7ea7ff9 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -81,6 +81,7 @@
>   #include "imports.h"
>   #include "accum.h"
>   #include "api_exec.h"
> +#include "api_loopback.h"
>   #include "arrayobj.h"
>   #include "attrib.h"
>   #include "blend.h"
> @@ -859,6 +860,55 @@ _mesa_alloc_dispatch_table(int size)
>      return table;
>   }
>
> +/**
> + * Creates
> + */

Looks like an unfinished comment there.


> +static struct _glapi_table *
> +create_beginend_table(struct gl_context *ctx)

Can ctx be const-qualified there?


> +{
> +   struct _glapi_table *table;
> +
> +   table = _mesa_alloc_dispatch_table(_gloffset_COUNT);
> +   if (!table)
> +      return NULL;
> +
> +   /* Fill in functions which return a value, since they should return some
> +    * specific value even if they emit a GL_INVALID_OPERATION error from them
> +    * being called within glBegin()/glEnd().
> +    */
> +#define COPY_DISPATCH(func) SET_##func(table, GET_##func(ctx->Exec))
> +
> +   COPY_DISPATCH(GenLists);
> +   COPY_DISPATCH(IsProgram);
> +   COPY_DISPATCH(IsVertexArray);
> +   COPY_DISPATCH(IsBuffer);
> +   COPY_DISPATCH(IsEnabled);
> +   COPY_DISPATCH(IsEnabledi);
> +   COPY_DISPATCH(IsRenderbuffer);
> +   COPY_DISPATCH(IsFramebuffer);
> +   COPY_DISPATCH(CheckFramebufferStatus);
> +   COPY_DISPATCH(RenderMode);
> +   COPY_DISPATCH(GetString);
> +   COPY_DISPATCH(GetStringi);
> +   COPY_DISPATCH(GetPointerv);
> +   COPY_DISPATCH(IsQuery);
> +   COPY_DISPATCH(IsSampler);
> +   COPY_DISPATCH(IsSync);
> +   COPY_DISPATCH(IsTexture);
> +   COPY_DISPATCH(IsTransformFeedback);
> +   COPY_DISPATCH(DeleteQueries);
> +   COPY_DISPATCH(AreTexturesResident);
> +   COPY_DISPATCH(FenceSync);
> +   COPY_DISPATCH(ClientWaitSync);
> +   COPY_DISPATCH(MapBuffer);
> +   COPY_DISPATCH(UnmapBuffer);
> +   COPY_DISPATCH(MapBufferRange);
> +   COPY_DISPATCH(MapBufferRange);
> +   COPY_DISPATCH(ObjectPurgeableAPPLE);
> +   COPY_DISPATCH(ObjectUnpurgeableAPPLE);
> +
> +   return table;
> +}
>
>   /**
>    * Initialize a struct gl_context struct (rendering context).
> @@ -933,19 +983,15 @@ _mesa_initialize_context(struct gl_context *ctx,
>
>      _mesa_reference_shared_state(ctx,&ctx->Shared, shared);
>
> -   if (!init_attrib_groups( ctx )) {
> -      _mesa_reference_shared_state(ctx,&ctx->Shared, NULL);
> -      return GL_FALSE;
> -   }
> +   if (!init_attrib_groups( ctx ))
> +      goto fail;

I'd remove the extra space in (( ctx )) while you're there.


>
>      /* setup the API dispatch tables with all nop functions */
> -   ctx->Exec = _mesa_alloc_dispatch_table(_gloffset_COUNT);
> -
> -   if (!ctx->Exec) {
> -      _mesa_reference_shared_state(ctx,&ctx->Shared, NULL);
> -      return GL_FALSE;
> -   }
> -   ctx->CurrentDispatch = ctx->Exec;
> +   ctx->OutsideBeginEnd = _mesa_alloc_dispatch_table(_gloffset_COUNT);
> +   if (!ctx->OutsideBeginEnd)
> +      goto fail;
> +   ctx->Exec = ctx->OutsideBeginEnd;
> +   ctx->CurrentDispatch = ctx->OutsideBeginEnd;
>
>      ctx->FragmentProgram._MaintainTexEnvProgram
>         = (_mesa_getenv("MESA_TEX_PROG") != NULL);
> @@ -967,12 +1013,12 @@ _mesa_initialize_context(struct gl_context *ctx,
>
>      switch (ctx->API) {
>      case API_OPENGL_COMPAT:
> +      ctx->BeginEnd = create_beginend_table(ctx);
>         ctx->Save = _mesa_create_save_table(ctx);
> -      if (!ctx->Save) {
> -         _mesa_reference_shared_state(ctx,&ctx->Shared, NULL);
> -	 free(ctx->Exec);
> -	 return GL_FALSE;
> -      }
> +      if (!ctx->BeginEnd || !ctx->Save)
> +         goto fail;
> +
> +      _mesa_loopback_init_api_table(ctx, ctx->BeginEnd);
>
>         /* fall-through */
>      case API_OPENGL_CORE:
> @@ -1002,6 +1048,12 @@ _mesa_initialize_context(struct gl_context *ctx,
>      ctx->FirstTimeCurrent = GL_TRUE;
>
>      return GL_TRUE;
> +
> +fail:
> +   free(ctx->BeginEnd);
> +   free(ctx->Exec);
> +   free(ctx->Save);
> +   return GL_FALSE;
>   }
>
>
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index d0c0e24..e7a6ee0 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3440,9 +3440,28 @@ struct gl_context
>      /** \name API function pointer tables */
>      /*@{*/
>      gl_api API;
> -   struct _glapi_table *Save;	/**<  Display list save functions */
> -   struct _glapi_table *Exec;	/**<  Execute functions */
> -   struct _glapi_table *CurrentDispatch;  /**<  == Save or Exec !! */
> +   /**
> +    * The current dispatch table for non-displaylist-saving execution, either
> +    * BeginEnd or OutsideBeginEnd
> +    */
> +   struct _glapi_table *Exec;
> +   /**
> +    * The normal dispatch table for non-displaylist-saving, non-begin/end
> +    */
> +   struct _glapi_table *OutsideBeginEnd;
> +   /** The dispatch table used between glNewList() and glEndList() */
> +   struct _glapi_table *Save;
> +   /**
> +    * The dispatch table used between glBegin() and glEnd() (outside of a
> +    * display list).  Only valid functions between those two are set, which is
> +    * mostly just the set in a GLvertexformat struct.
> +    */
> +   struct _glapi_table *BeginEnd;
> +   /**
> +    * Tracks the current dispatch table out of the 3 above, so that it can be
> +    * re-set on glXMakeCurrent().
> +    */
> +   struct _glapi_table *CurrentDispatch;
>      /*@}*/
>
>      struct gl_config Visual;
> diff --git a/src/mesa/main/vtxfmt.c b/src/mesa/main/vtxfmt.c
> index 6d687de..347d07d 100644
> --- a/src/mesa/main/vtxfmt.c
> +++ b/src/mesa/main/vtxfmt.c
> @@ -252,6 +252,8 @@ void
>   _mesa_install_exec_vtxfmt(struct gl_context *ctx, const GLvertexformat *vfmt)
>   {
>      install_vtxfmt( ctx, ctx->Exec, vfmt );
> +   if (ctx->BeginEnd)
> +      install_vtxfmt( ctx, ctx->BeginEnd, vfmt );
>   }
>
>
> diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
> index 84bcdd6..f987439 100644
> --- a/src/mesa/vbo/vbo_exec_api.c
> +++ b/src/mesa/vbo/vbo_exec_api.c
> @@ -833,6 +833,17 @@ static void GLAPIENTRY vbo_exec_Begin( GLenum mode )
>      exec->vtx.prim[i].base_instance = 0;
>
>      ctx->Driver.CurrentExecPrimitive = mode;
> +
> +   ctx->Exec = ctx->BeginEnd;
> +   /* We may have been called from a display list, in which case we should
> +    * leave dlist.c's dispatch table in place.
> +    */
> +   if (ctx->CurrentDispatch == ctx->OutsideBeginEnd) {
> +      ctx->CurrentDispatch = ctx->BeginEnd;
> +      _glapi_set_dispatch(ctx->CurrentDispatch);
> +   } else {
> +      assert(ctx->CurrentDispatch == ctx->Save);
> +   }

If someone did this (in immediate mode):

glBegin(mode);
glBegin(mode);

wouldn't the assert in the else clause fail?  I think we'd want check 
for that before the assertion and raise GL_INVALID_OPERATION there. 
I'd have to test to be sure though.


>   }
>
>
> @@ -849,6 +860,12 @@ static void GLAPIENTRY vbo_exec_End( void )
>         return;
>      }
>
> +   ctx->Exec = ctx->OutsideBeginEnd;
> +   if (ctx->CurrentDispatch == ctx->BeginEnd) {
> +      ctx->CurrentDispatch = ctx->OutsideBeginEnd;
> +      _glapi_set_dispatch(ctx->CurrentDispatch);
> +   }
> +
>      if (exec->vtx.prim_count>  0) {
>         /* close off current primitive */
>         int idx = exec->vtx.vert_count;

Patches 1-4 are: Reviewed-by: Brian Paul <brianp at vmware.com>



More information about the mesa-dev mailing list