[Mesa-dev] [PATCH] mesa: Fix assertion error with glDebugMessageControl

[Brian Paul]

On 08/22/2013 05:33 PM, Timothy Arceri wrote:
> Hi guys,
>
> As I'm yet to receive feedback I thought I'd explain more thoroughly what my patch does to make reviewing it easier.
>
> Currently glDebugMessageControlARB() ALWAYS throws an assertion whenever the count parameter it set to anything greater than 0.
> This is because the gl_enum_to_debug_* functions are called twice which means that type, severity and source are ALWAYS equal
> to MESA_DEBUG_*_COUNT
> I also removed the assert because _mesa_DebugMessageControlARB already does this same test and returns with an GL_INVALID_OPERATION
> error so the assert would never be true once the extra enum conversion is removed.
>
> As this is a bug that stops glDebugMessageControlARB() from working properly I would also say this is probably a good candidate for the stable branches.
>
> Thanks for your time,
> Tim
>
>
>
> ----- Original Message -----
> From: Timothy Arceri <t_arceri at yahoo.com.au>
> To: mesa-dev at lists.freedesktop.org
> Cc: Timothy Arceri <t_arceri at yahoo.com.au>
> Sent: Wednesday, 21 August 2013 4:18 PM
> Subject: [PATCH] mesa: Fix assertion error with glDebugMessageControl
>
> enums were being converted twice resulting in incorrect values.
> The extra conversion has been remove and the redundant assert is
> removed also.
>
> Signed-off-by: Timothy Arceri <t_arceri at yahoo.com.au>
> ---
> src/mesa/main/errors.c | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
> index 5a0758d..cc93d3b 100644
> --- a/src/mesa/main/errors.c
> +++ b/src/mesa/main/errors.c
> @@ -609,11 +609,6 @@ control_app_messages(struct gl_context *ctx, GLenum esource, GLenum etype,
>      enum mesa_debug_type type = gl_enum_to_debug_type(etype);
>      enum mesa_debug_severity severity = gl_enum_to_debug_severity(eseverity);
>
> -   if (count)
> -      assert(severity == MESA_DEBUG_SEVERITY_COUNT
> -             && type != MESA_DEBUG_TYPE_COUNT
> -             && source != MESA_DEBUG_SOURCE_COUNT);
> -
>      for (i = 0; i < count; i++)
>         set_message_state(ctx, source, type, ids[i], enabled);
>
> @@ -629,9 +624,6 @@ _mesa_DebugMessageControlARB(GLenum gl_source, GLenum gl_type,
>                                GLsizei count, const GLuint *ids,
>                                GLboolean enabled)
> {
> -   enum mesa_debug_source source;
> -   enum mesa_debug_type type;
> -   enum mesa_debug_severity severity;
>      GET_CURRENT_CONTEXT(ctx);
>
>      if (count < 0) {
> @@ -651,11 +643,8 @@ _mesa_DebugMessageControlARB(GLenum gl_source, GLenum gl_type,
>         return;
>      }
>
> -   source = gl_enum_to_debug_source(gl_source);
> -   type = gl_enum_to_debug_type(gl_type);
> -   severity = gl_enum_to_debug_severity(gl_severity);
> -
> -   control_app_messages(ctx, source, type, severity, count, ids, enabled);
> +   control_app_messages(ctx, gl_source, gl_type, gl_severity,
> +                        count, ids, enabled);
> }
>
> void GLAPIENTRY
>

Looks good.  Thanks.

Reviewed-by: Brian Paul <brianp at vmware.com>

I presume you need someone to commit this for you- I'll take care of 
that and mark this for the 9.2 branch.

-Brian