[Mesa-dev] [PATCH 13/18] mesa: use accessors for struct gl_debug_state

Chia-I Wu olvaffe at gmail.com
Fri Apr 25 15:27:52 PDT 2014


On Fri, Apr 25, 2014 at 10:30 PM, Brian Paul <brianp at vmware.com> wrote:
> On 04/25/2014 04:42 AM, Chia-I Wu wrote:
>>
>> When GL_DEBUG_OUTPUT_SYNCHRONOUS is GL_TRUE, drivers are allowed to log
>> debug
>> messages from other threads.  That requires gl_debug_state to be protected
>> by
>> a mutex, even when it is a context state.  While we do not spawn threads
>> in
>> Mesa yet, this commit makes it easier to do when we want to.
>>
>> Since the definition of struct gl_debug_state is no longer needed by the
>> rest
>> of the driver, move it to main/errors.c.  This should make it even harder
>> to
>> use the struct incorrectly.
>>
>> Signed-off-by: Chia-I Wu <olv at lunarg.com>
>> ---
>>   src/mesa/drivers/dri/common/dri_util.c |   5 +-
>>   src/mesa/main/enable.c                 |  35 ++-------
>>   src/mesa/main/errors.c                 | 125
>> ++++++++++++++++++++++++++++++++-
>>   src/mesa/main/errors.h                 |  10 ++-
>>   src/mesa/main/get.c                    |  16 +----
>>   src/mesa/main/getstring.c              |  17 +----
>>   src/mesa/main/mtypes.h                 |  40 +----------
>>   src/mesa/state_tracker/st_manager.c    |   4 +-
>>   8 files changed, 144 insertions(+), 108 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/common/dri_util.c
>> b/src/mesa/drivers/dri/common/dri_util.c
>> index aed73c7..c410474 100644
>> --- a/src/mesa/drivers/dri/common/dri_util.c
>> +++ b/src/mesa/drivers/dri/common/dri_util.c
>> @@ -449,10 +449,7 @@ driContextSetFlags(struct gl_context *ctx, uint32_t
>> flags)
>>       if ((flags & __DRI_CTX_FLAG_FORWARD_COMPATIBLE) != 0)
>>           ctx->Const.ContextFlags |=
>> GL_CONTEXT_FLAG_FORWARD_COMPATIBLE_BIT;
>>       if ((flags & __DRI_CTX_FLAG_DEBUG) != 0) {
>> -        struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
>> -        if (debug) {
>> -            debug->DebugOutput = GL_TRUE;
>> -        }
>> +       _mesa_set_debug_state_int(ctx, GL_DEBUG_OUTPUT, GL_TRUE);
>>           ctx->Const.ContextFlags |= GL_CONTEXT_FLAG_DEBUG_BIT;
>>       }
>>   }
>> diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
>> index edd4751..0f3bcf0 100644
>> --- a/src/mesa/main/enable.c
>> +++ b/src/mesa/main/enable.c
>> @@ -368,26 +368,11 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap,
>> GLboolean state)
>>            ctx->Depth.Test = state;
>>            break;
>>         case GL_DEBUG_OUTPUT:
>> -         if (!_mesa_is_desktop_gl(ctx)) {
>> -            goto invalid_enum_error;
>> -         }
>> -         else {
>> -            struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
>> -            if (debug) {
>> -               debug->DebugOutput = state;
>> -            }
>> -         }
>> -         break;
>>         case GL_DEBUG_OUTPUT_SYNCHRONOUS_ARB:
>> -         if (!_mesa_is_desktop_gl(ctx)) {
>> +         if (!_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>> -         }
>> -         else {
>> -            struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
>> -            if (debug) {
>> -               debug->SyncOutput = state;
>> -            }
>> -         }
>> +         else
>> +            _mesa_set_debug_state_int(ctx, cap, state);
>>            break;
>>         case GL_DITHER:
>>            if (ctx->Color.DitherFlag == state)
>> @@ -1239,21 +1224,11 @@ _mesa_IsEnabled( GLenum cap )
>>         case GL_CULL_FACE:
>>            return ctx->Polygon.CullFlag;
>>         case GL_DEBUG_OUTPUT:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> -            goto invalid_enum_error;
>> -         if (ctx->Debug) {
>> -            return ctx->Debug->DebugOutput;
>> -         } else {
>> -            return GL_FALSE;
>> -         }
>>         case GL_DEBUG_OUTPUT_SYNCHRONOUS_ARB:
>>            if (!_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>> -         if (ctx->Debug) {
>> -            return ctx->Debug->SyncOutput;
>> -         } else {
>> -            return GL_FALSE;
>> -         }
>> +         else
>> +            return (GLboolean) _mesa_get_debug_state_int(ctx, cap);
>>         case GL_DEPTH_TEST:
>>            return ctx->Depth.Test;
>>         case GL_DITHER:
>> diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
>> index 277f38c..9258eb6 100644
>> --- a/src/mesa/main/errors.c
>> +++ b/src/mesa/main/errors.c
>> @@ -47,6 +47,45 @@ struct gl_debug_severity
>>      GLuint ID;
>>   };
>>
>> +/**
>> + * An error, warning, or other piece of debug information for an
>> application
>> + * to consume via GL_ARB_debug_output/GL_KHR_debug.
>> + */
>> +struct gl_debug_msg
>> +{
>> +   enum mesa_debug_source source;
>> +   enum mesa_debug_type type;
>> +   GLuint id;
>> +   enum mesa_debug_severity severity;
>> +   GLsizei length;
>> +   GLcharARB *message;
>> +};
>> +
>> +struct gl_debug_namespace
>> +{
>> +   struct _mesa_HashTable *IDs;
>> +   unsigned ZeroID; /* a HashTable won't take zero, so store its state
>> here */
>> +   /** lists of IDs in the hash table at each severity */
>> +   struct simple_node Severity[MESA_DEBUG_SEVERITY_COUNT];
>> +};
>> +
>> +struct gl_debug_state
>> +{
>> +   GLDEBUGPROC Callback;
>> +   const void *CallbackData;
>> +   GLboolean SyncOutput;
>> +   GLboolean DebugOutput;
>> +   GLboolean
>> Defaults[MAX_DEBUG_GROUP_STACK_DEPTH][MESA_DEBUG_SEVERITY_COUNT][MESA_DEBUG_SOURCE_COUNT][MESA_DEBUG_TYPE_COUNT];
>> +   struct gl_debug_namespace
>> Namespaces[MAX_DEBUG_GROUP_STACK_DEPTH][MESA_DEBUG_SOURCE_COUNT][MESA_DEBUG_TYPE_COUNT];
>> +   struct gl_debug_msg Log[MAX_DEBUG_LOGGED_MESSAGES];
>> +   struct gl_debug_msg DebugGroupMsgs[MAX_DEBUG_GROUP_STACK_DEPTH];
>> +   GLint GroupStackDepth;
>> +   GLint NumMessages;
>> +   GLint NextMsg;
>> +   GLint NextMsgLength; /* redundant, but copied here from
>> Log[NextMsg].length
>> +                           for the sake of the offsetof() code in get.c
>> */
>> +};
>> +
>>   static char out_of_memory[] = "Debugging error: out of memory";
>>
>>   static const GLenum debug_source_enums[] = {
>> @@ -597,7 +636,7 @@ debug_pop_group(struct gl_debug_state *debug)
>>    * Return debug state for the context.  The debug state will be
>> allocated
>>    * and initialized upon the first call.
>>    */
>> -struct gl_debug_state *
>> +static struct gl_debug_state *
>>   _mesa_get_debug_state(struct gl_context *ctx)
>>   {
>>      if (!ctx->Debug) {
>> @@ -610,6 +649,90 @@ _mesa_get_debug_state(struct gl_context *ctx)
>>      return ctx->Debug;
>>   }
>>
>
> I think it would be good to have comments on the following three new
> functions to explain their purpose and parameters.
I will add

/**
 * Query the integer debug state specified by \p pname.  This can be called
 * _mesa_GetIntegerv for example.
 */

for _mesa_get_debug_state_int, and similar comments for the other two functions.


>
>
>
>> +bool
>> +_mesa_set_debug_state_int(struct gl_context *ctx, GLenum pname, GLint
>> val)
>> +{
>> +   struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
>> +
>> +   if (!debug)
>> +      return false;
>> +
>> +   switch (pname) {
>> +   case GL_DEBUG_OUTPUT:
>> +      debug->DebugOutput = (val != 0);
>> +      break;
>> +   case GL_DEBUG_OUTPUT_SYNCHRONOUS_ARB:
>> +      debug->SyncOutput = (val != 0);
>> +      break;
>> +   default:
>> +      assert(!"unknown debug output param");
>> +      break;
>> +   }
>> +
>> +   return true;
>> +}
>> +
>> +GLint
>> +_mesa_get_debug_state_int(struct gl_context *ctx, GLenum pname)
>> +{
>> +   struct gl_debug_state *debug;
>> +   GLint val;
>> +
>> +   debug = ctx->Debug;
>> +   if (!debug)
>> +      return 0;
>> +
>> +   switch (pname) {
>> +   case GL_DEBUG_OUTPUT:
>> +      val = debug->DebugOutput;
>> +      break;
>> +   case GL_DEBUG_OUTPUT_SYNCHRONOUS_ARB:
>> +      val = debug->SyncOutput;
>> +      break;
>> +   case GL_DEBUG_LOGGED_MESSAGES:
>> +      val = debug->NumMessages;
>> +      break;
>> +   case GL_DEBUG_NEXT_LOGGED_MESSAGE_LENGTH:
>> +      val = debug->NextMsgLength;
>> +      break;
>> +   case GL_DEBUG_GROUP_STACK_DEPTH:
>> +      val = debug->GroupStackDepth;
>> +      break;
>> +   default:
>> +      assert(!"unknown debug output param");
>> +      val = 0;
>> +      break;
>> +   }
>> +
>> +   return val;
>> +}
>> +
>> +void *
>> +_mesa_get_debug_state_ptr(struct gl_context *ctx, GLenum pname)
>> +{
>> +   struct gl_debug_state *debug;
>> +   void *val;
>> +
>> +   debug = ctx->Debug;
>> +   if (!debug)
>> +      return NULL;
>> +
>> +   switch (pname) {
>> +   case GL_DEBUG_CALLBACK_FUNCTION_ARB:
>> +      val = (void *) debug->Callback;
>> +      break;
>> +   case GL_DEBUG_CALLBACK_USER_PARAM_ARB:
>> +      val = (void *) debug->CallbackData;
>> +      break;
>> +   default:
>> +      assert(!"unknown debug output param");
>> +      val = NULL;
>> +      break;
>> +   }
>> +
>> +   return val;
>> +}
>> +
>>
>>   /**
>>    * Log a client or driver debug message.
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev



-- 
olv at LunarG.com


More information about the mesa-dev mailing list