[Mesa-dev] [PATCH 33/38] main: Refactor _mesa_DrawBuffer.

[Fredrik Höglund]

On Wednesday 04 March 2015, Laura Ekstrand wrote:
> This could have added a new DD table entry for DrawBuffer that takes an
> arbitrary draw buffer, but, after looking at the existing DD functions,
> Kenneth Graunke recommended that we just skip calling the DD functions in the
> case of ARB_direct_state_access.  The DD implementations for DrawBuffer(s)
> have limited functionality, especially with respect to
> ARB_direct_state_access.
> ---
>  src/mesa/main/buffers.c | 39 ++++++++++++++++++++++++---------------
>  src/mesa/main/buffers.h |  4 ++++
>  2 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
> index 4fdd97e..023d7dd 100644
> --- a/src/mesa/main/buffers.c
> +++ b/src/mesa/main/buffers.c
> @@ -242,16 +242,16 @@ read_buffer_enum_to_index(GLenum buffer)
>   *
>   * See the GL_EXT_framebuffer_object spec for more info.
>   */
> -void GLAPIENTRY
> -_mesa_DrawBuffer(GLenum buffer)
> +void
> +_mesa_draw_buffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +                  GLenum buffer, const char *caller)
>  {
>     GLbitfield destMask;
> -   GET_CURRENT_CONTEXT(ctx);
>  
>     FLUSH_VERTICES(ctx, 0);
>  
>     if (MESA_VERBOSE & VERBOSE_API) {
> -      _mesa_debug(ctx, "glDrawBuffer %s\n", _mesa_lookup_enum_by_nr(buffer));
> +      _mesa_debug(ctx, "%s %s\n", caller, _mesa_lookup_enum_by_nr(buffer));
>     }
>  
>     if (buffer == GL_NONE) {
> @@ -259,33 +259,42 @@ _mesa_DrawBuffer(GLenum buffer)
>     }
>     else {
>        const GLbitfield supportedMask
> -         = supported_buffer_bitmask(ctx, ctx->DrawBuffer);
> +         = supported_buffer_bitmask(ctx, fb);
>        destMask = draw_buffer_enum_to_bitmask(ctx, buffer);
>        if (destMask == BAD_MASK) {
>           /* totally bogus buffer */
> -         _mesa_error(ctx, GL_INVALID_ENUM,
> -                     "glDrawBuffer(buffer=0x%x)", buffer);
> +         _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid buffer %s)", caller,
> +                     _mesa_lookup_enum_by_nr(buffer));
>           return;
>        }
>        destMask &= supportedMask;
>        if (destMask == 0x0) {
>           /* none of the named color buffers exist! */
> -         _mesa_error(ctx, GL_INVALID_OPERATION,
> -                     "glDrawBuffer(buffer=0x%x)", buffer);
> +         _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid buffer %s)",
> +                     caller, _mesa_lookup_enum_by_nr(buffer));
>           return;
>        }
>     }
>  
>     /* if we get here, there's no error so set new state */
> -   _mesa_drawbuffers(ctx, ctx->DrawBuffer, 1, &buffer, &destMask);
> +   _mesa_drawbuffers(ctx, fb, 1, &buffer, &destMask);
>  
>     /*
> -    * Call device driver function.
> +    * Call device driver function only if calling the traditional entry point.
>      */
> -   if (ctx->Driver.DrawBuffers)
> -      ctx->Driver.DrawBuffers(ctx, 1, &buffer);
> -   else if (ctx->Driver.DrawBuffer)
> -      ctx->Driver.DrawBuffer(ctx, buffer);
> +   if (strcmp(caller, "glDrawBuffer") == 0) {
> +      if (ctx->Driver.DrawBuffers)
> +         ctx->Driver.DrawBuffers(ctx, 1, &buffer);
> +      else if (ctx->Driver.DrawBuffer)
> +         ctx->Driver.DrawBuffer(ctx, buffer);
> +   }

I think it's better to call the driver hook when fb is the bound
framebuffer.  This will always be the case in the traditional entry point, and
it makes NamedFramebufferDrawBuffer behave exactly like the traditional
entry point when it's passed the bound framebuffer.  It looks like some
drivers depend on that behavior.

> +}
> +
> +void GLAPIENTRY
> +_mesa_DrawBuffer(GLenum buffer)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   _mesa_draw_buffer(ctx, ctx->DrawBuffer, buffer, "glDrawBuffer");
>  }
>  
>  
> diff --git a/src/mesa/main/buffers.h b/src/mesa/main/buffers.h
> index bc6d74a..4192afd 100644
> --- a/src/mesa/main/buffers.h
> +++ b/src/mesa/main/buffers.h
> @@ -38,6 +38,10 @@
>  struct gl_context;
>  struct gl_framebuffer;
>  
> +extern void
> +_mesa_draw_buffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +                  GLenum buffer, const char *caller);
> +
>  extern void GLAPIENTRY
>  _mesa_DrawBuffer( GLenum mode );
>  
>