From cf61ea3029b7c9a3e334ea7f1b45844fad2d0a77 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Tue, 22 Apr 2014 14:22:49 +0800 Subject: [PATCH] mesa: use accessors for struct gl_debug_state 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. v2: add comments for the accessors Signed-off-by: Chia-I Wu Reviewed-by: Brian Paul --- src/mesa/drivers/dri/common/dri_util.c | 5 +- src/mesa/main/enable.c | 35 ++------- src/mesa/main/errors.c | 137 ++++++++++++++++++++++++++++++++- 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, 156 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..1993744 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,102 @@ _mesa_get_debug_state(struct gl_context *ctx) return ctx->Debug; } +/** + * Set the integer debug state specified by \p pname. This can be called from + * _mesa_set_enable for example. + */ +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; +} + +/** + * Query the integer debug state specified by \p pname. This can be called + * _mesa_GetIntegerv for example. + */ +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; +} + +/** + * Query the pointer debug state specified by \p pname. This can be called + * _mesa_GetPointerv for example. + */ +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. diff --git a/src/mesa/main/errors.h b/src/mesa/main/errors.h index e0706e5..06d0b21 100644 --- a/src/mesa/main/errors.h +++ b/src/mesa/main/errors.h @@ -83,8 +83,14 @@ _mesa_gl_debug(struct gl_context *ctx, } \ } while (0) -struct gl_debug_state * -_mesa_get_debug_state(struct gl_context *ctx); +bool +_mesa_set_debug_state_int(struct gl_context *ctx, GLenum pname, GLint val); + +GLint +_mesa_get_debug_state_int(struct gl_context *ctx, GLenum pname); + +void * +_mesa_get_debug_state_ptr(struct gl_context *ctx, GLenum pname); extern void _mesa_shader_debug(struct gl_context *ctx, GLenum type, GLuint *id, diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index 6d95790..fe35ff3 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -989,24 +989,10 @@ find_custom_value(struct gl_context *ctx, const struct value_desc *d, union valu break; /* GL_KHR_DEBUG */ case GL_DEBUG_LOGGED_MESSAGES: - { - struct gl_debug_state *debug = _mesa_get_debug_state(ctx); - v->value_int = debug ? debug->NumMessages : 0; - } - break; case GL_DEBUG_NEXT_LOGGED_MESSAGE_LENGTH: - { - struct gl_debug_state *debug = _mesa_get_debug_state(ctx); - v->value_int = debug ? debug->NextMsgLength : 0; - } - break; case GL_DEBUG_GROUP_STACK_DEPTH: - { - struct gl_debug_state *debug = _mesa_get_debug_state(ctx); - v->value_int = debug ? debug->GroupStackDepth : 0; - } + v->value_int = _mesa_get_debug_state_int(ctx, d->pname); break; - /* GL_ARB_shader_atomic_counters */ case GL_ATOMIC_COUNTER_BUFFER_BINDING: v->value_int = ctx->AtomicBuffer->Name; diff --git a/src/mesa/main/getstring.c b/src/mesa/main/getstring.c index b0bd319..431d60b 100644 --- a/src/mesa/main/getstring.c +++ b/src/mesa/main/getstring.c @@ -253,22 +253,11 @@ _mesa_GetPointerv( GLenum pname, GLvoid **params ) *params = (GLvoid *) ctx->Array.VAO->VertexAttrib[VERT_ATTRIB_POINT_SIZE].Ptr; break; case GL_DEBUG_CALLBACK_FUNCTION_ARB: - if (!_mesa_is_desktop_gl(ctx)) { - goto invalid_pname; - } - else { - struct gl_debug_state *debug = _mesa_get_debug_state(ctx); - *params = debug ? (void *) debug->Callback : NULL; - } - break; case GL_DEBUG_CALLBACK_USER_PARAM_ARB: - if (!_mesa_is_desktop_gl(ctx)) { + if (!_mesa_is_desktop_gl(ctx)) goto invalid_pname; - } - else { - struct gl_debug_state *debug = _mesa_get_debug_state(ctx); - *params = debug ? (void *) debug->CallbackData : NULL; - } + else + *params = _mesa_get_debug_state_ptr(ctx, pname); break; default: goto invalid_pname; diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 6694383..8930f7c 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -76,6 +76,7 @@ struct gl_list_extensions; struct gl_meta_state; struct gl_program_cache; struct gl_texture_object; +struct gl_debug_state; struct gl_context; struct st_context; struct gl_uniform_storage; @@ -3822,45 +3823,6 @@ enum mesa_debug_severity { /** @} */ /** - * 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 */ -}; - -/** * Enum for the OpenGL APIs we know about and may support. * * NOTE: This must match the api_enum table in diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c index 314d342..f573877 100644 --- a/src/mesa/state_tracker/st_manager.c +++ b/src/mesa/state_tracker/st_manager.c @@ -663,13 +663,11 @@ st_api_create_context(struct st_api *stapi, struct st_manager *smapi, } if (attribs->flags & ST_CONTEXT_FLAG_DEBUG){ - struct gl_debug_state *debug = _mesa_get_debug_state(st->ctx); - if (!debug) { + if (!_mesa_set_debug_state_int(st->ctx, GL_DEBUG_OUTPUT, GL_TRUE)) { *error = ST_CONTEXT_ERROR_NO_MEMORY; return NULL; } st->ctx->Const.ContextFlags |= GL_CONTEXT_FLAG_DEBUG_BIT; - debug->DebugOutput = GL_TRUE; } if (attribs->flags & ST_CONTEXT_FLAG_FORWARD_COMPATIBLE) -- 2.7.4