[Mesa-dev] [PATCH 35/38] main: Refactor _mesa_ReadBuffer.

[Fredrik Höglund]

On Wednesday 04 March 2015, Laura Ekstrand wrote:
> This could have added a new DD table entry for ReadBuffer that takes an
> arbitrary read 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 ReadBuffer
> have limited functionality, especially with respect to
> ARB_direct_state_access.
> ---
>  src/mesa/main/buffers.c | 45 +++++++++++++++++++++++++--------------------
>  src/mesa/main/buffers.h |  7 ++++++-
>  src/mesa/main/context.c |  2 +-
>  3 files changed, 32 insertions(+), 22 deletions(-)
> 
> diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
> index 8d95c68..e244d1e 100644
> --- a/src/mesa/main/buffers.c
> +++ b/src/mesa/main/buffers.c
> @@ -624,11 +624,10 @@ _mesa_update_draw_buffers(struct gl_context *ctx)
>   * \param bufferIndex  the numerical index corresponding to 'buffer'
>   */
>  void
> -_mesa_readbuffer(struct gl_context *ctx, GLenum buffer, GLint bufferIndex)
> +_mesa_readbuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +                 GLenum buffer, GLint bufferIndex)
>  {
> -   struct gl_framebuffer *fb = ctx->ReadBuffer;
> -
> -   if (_mesa_is_winsys_fbo(fb)) {
> +   if ((fb == ctx->ReadBuffer) && _mesa_is_winsys_fbo(fb)) {
>        /* Only update the per-context READ_BUFFER state if we're bound to
>         * a window-system framebuffer.
>         */
> @@ -647,23 +646,17 @@ _mesa_readbuffer(struct gl_context *ctx, GLenum buffer, GLint bufferIndex)
>   * Called by glReadBuffer to set the source renderbuffer for reading pixels.
>   * \param mode color buffer such as GL_FRONT, GL_BACK, etc.
>   */
> -void GLAPIENTRY
> -_mesa_ReadBuffer(GLenum buffer)
> +void
> +_mesa_read_buffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +                  GLenum buffer, const char *caller)
>  {
> -   struct gl_framebuffer *fb;
>     GLbitfield supportedMask;
>     GLint srcBuffer;
> -   GET_CURRENT_CONTEXT(ctx);
>  
>     FLUSH_VERTICES(ctx, 0);
>  
>     if (MESA_VERBOSE & VERBOSE_API)
> -      _mesa_debug(ctx, "glReadBuffer %s\n", _mesa_lookup_enum_by_nr(buffer));
> -
> -   fb = ctx->ReadBuffer;
> -
> -   if (MESA_VERBOSE & VERBOSE_API)
> -      _mesa_debug(ctx, "glReadBuffer %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) {
>        /* This is legal--it means that no buffer should be bound for reading. */
> @@ -674,24 +667,36 @@ _mesa_ReadBuffer(GLenum buffer)
>        srcBuffer = read_buffer_enum_to_index(buffer);
>        if (srcBuffer == -1) {
>           _mesa_error(ctx, GL_INVALID_ENUM,
> -                     "glReadBuffer(buffer=0x%x)", buffer);
> +                     "%s(invalid buffer %s)", caller,
> +                     _mesa_lookup_enum_by_nr(buffer));
>           return;
>        }
>        supportedMask = supported_buffer_bitmask(ctx, fb);
>        if (((1 << srcBuffer) & supportedMask) == 0) {
>           _mesa_error(ctx, GL_INVALID_OPERATION,
> -                     "glReadBuffer(buffer=0x%x)", buffer);
> +                     "%s(invalid buffer %s)", caller,
> +                     _mesa_lookup_enum_by_nr(buffer));
>           return;
>        }
>     }
>  
>     /* OK, all error checking has been completed now */
>  
> -   _mesa_readbuffer(ctx, buffer, srcBuffer);
> +   _mesa_readbuffer(ctx, fb, buffer, srcBuffer);
>  
>     /*
> -    * Call device driver function.
> +    * Call the device driver function only if caller is the traditional entry
> +    * point.
>      */
> -   if (ctx->Driver.ReadBuffer)
> -      (*ctx->Driver.ReadBuffer)(ctx, buffer);
> +   if (strcmp(caller, "glReadBuffer") == 0) {
> +      if (ctx->Driver.ReadBuffer)
> +         (*ctx->Driver.ReadBuffer)(ctx, buffer);
> +   }
> +}

As in patch 33, I think the test should be whether fb is the currently
bound framebuffer.

> +
> +void GLAPIENTRY
> +_mesa_ReadBuffer(GLenum buffer)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   _mesa_read_buffer(ctx, ctx->ReadBuffer, buffer, "glReadBuffer");
>  }
> diff --git a/src/mesa/main/buffers.h b/src/mesa/main/buffers.h
> index 12d5743..ca7ad19 100644
> --- a/src/mesa/main/buffers.h
> +++ b/src/mesa/main/buffers.h
> @@ -57,12 +57,17 @@ _mesa_drawbuffers(struct gl_context *ctx, struct gl_framebuffer *fb,
>                    const GLbitfield *destMask);
>  
>  extern void
> -_mesa_readbuffer(struct gl_context *ctx, GLenum buffer, GLint bufferIndex);
> +_mesa_readbuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +                 GLenum buffer, GLint bufferIndex);
>  
>  extern void
>  _mesa_update_draw_buffers(struct gl_context *ctx);
>  
>  
> +extern void
> +_mesa_read_buffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +                  GLenum buffer, const char *caller);
> +
>  extern void GLAPIENTRY
>  _mesa_ReadBuffer( GLenum mode );
>  
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index b285d51..8ea9e33 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -1581,7 +1581,7 @@ handle_first_current(struct gl_context *ctx)
>              bufferIndex = BUFFER_FRONT_LEFT;
>           }
>  
> -         _mesa_readbuffer(ctx, buffer, bufferIndex);
> +         _mesa_readbuffer(ctx, ctx->ReadBuffer, buffer, bufferIndex);
>        }
>     }
>  
>