[Mesa-dev] [PATCH 37/38] main: Refactor DrawBuffers.
Fredrik Höglund
fredrik at kde.org
Thu Apr 16 10:21:06 PDT 2015
On Wednesday 04 March 2015, Laura Ekstrand wrote:
> This could have added a new DD table entry for DrawBuffers 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 | 70 +++++++++++++++++++++++++++++++++----------------
> src/mesa/main/buffers.h | 4 +++
> 2 files changed, 51 insertions(+), 23 deletions(-)
>
> diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
> index d0627b5..32986f5 100644
> --- a/src/mesa/main/buffers.c
> +++ b/src/mesa/main/buffers.c
> @@ -325,13 +325,13 @@ _mesa_NamedFramebufferDrawBuffer(GLuint framebuffer, GLenum buf)
> * names cannot specify more than one buffer. For example,
> * GL_FRONT_AND_BACK is illegal.
> */
> -void GLAPIENTRY
> -_mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> +void
> +_mesa_draw_buffers(struct gl_context *ctx, struct gl_framebuffer *fb,
> + GLsizei n, const GLenum *buffers, const char *caller)
> {
> GLuint output;
> GLbitfield usedBufferMask, supportedMask;
> GLbitfield destMask[MAX_DRAW_BUFFERS];
> - GET_CURRENT_CONTEXT(ctx);
>
> FLUSH_VERTICES(ctx, 0);
>
> @@ -342,12 +342,18 @@ _mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> * "An INVALID_VALUE error is generated if n is greater than
> * MAX_DRAW_BUFFERS."
> */
> - if (n < 0 || n > (GLsizei) ctx->Const.MaxDrawBuffers) {
> - _mesa_error(ctx, GL_INVALID_VALUE, "glDrawBuffersARB(n)");
> + if (n < 0) {
> + _mesa_error(ctx, GL_INVALID_VALUE, "%s(n < 0)", caller);
> + return;
> + }
> +
> + if (n > (GLsizei) ctx->Const.MaxDrawBuffers) {
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "%s(n > maximum number of draw buffers)", caller);
> return;
> }
>
> - supportedMask = supported_buffer_bitmask(ctx, ctx->DrawBuffer);
> + supportedMask = supported_buffer_bitmask(ctx, fb);
> usedBufferMask = 0x0;
>
> /* From the ES 3.0 specification, page 180:
> @@ -355,9 +361,9 @@ _mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> * and the constant must be BACK or NONE."
> * (same restriction applies with GL_EXT_draw_buffers specification)
> */
> - if (ctx->API == API_OPENGLES2 && _mesa_is_winsys_fbo(ctx->DrawBuffer) &&
> + if (ctx->API == API_OPENGLES2 && _mesa_is_winsys_fbo(fb) &&
> (n != 1 || (buffers[0] != GL_NONE && buffers[0] != GL_BACK))) {
> - _mesa_error(ctx, GL_INVALID_OPERATION, "glDrawBuffers(buffer)");
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid buffers)", caller);
> return;
> }
>
> @@ -389,9 +395,11 @@ _mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> * or equal to the value of MAX_COLOR_ATTACHMENTS, then the error
> * INVALID_OPERATION results."
> */
> - if (_mesa_is_user_fbo(ctx->DrawBuffer) && buffers[output] >=
> + if (_mesa_is_user_fbo(fb) && buffers[output] >=
> GL_COLOR_ATTACHMENT0 + ctx->Const.MaxDrawBuffers) {
> - _mesa_error(ctx, GL_INVALID_OPERATION, "glDrawBuffersARB(buffer)");
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(buffers[%d] >= maximum number of draw buffers)",
> + caller, output);
> return;
> }
>
> @@ -402,9 +410,10 @@ _mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> * 4.5 or 4.6. Otherwise, an INVALID_ENUM error is generated.
> */
> if (destMask[output] == BAD_MASK) {
> - _mesa_error(ctx, GL_INVALID_ENUM, "glDrawBuffersARB(buffer)");
> + _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid buffer %s)",
> + caller, _mesa_lookup_enum_by_nr(buffers[output]));
> return;
> - }
> + }
>
> /* From the OpenGL 4.0 specification, page 256:
> * "For both the default framebuffer and framebuffer objects, the
> @@ -417,7 +426,8 @@ _mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> * but the Khronos conformance tests expect INVALID_ENUM.
> */
> if (_mesa_bitcount(destMask[output]) > 1) {
> - _mesa_error(ctx, GL_INVALID_ENUM, "glDrawBuffersARB(buffer)");
> + _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid buffer %s)",
> + caller, _mesa_lookup_enum_by_nr(buffers[output]));
> return;
> }
>
> @@ -434,7 +444,8 @@ _mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> destMask[output] &= supportedMask;
> if (destMask[output] == 0) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> - "glDrawBuffersARB(unsupported buffer)");
> + "%s(unsupported buffer %s)",
> + caller, _mesa_lookup_enum_by_nr(buffers[output]));
> return;
> }
>
> @@ -443,10 +454,12 @@ _mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> * in bufs must be COLOR_ATTACHMENTi or NONE. [...] INVALID_OPERATION."
> * (same restriction applies with GL_EXT_draw_buffers specification)
> */
> - if (ctx->API == API_OPENGLES2 && _mesa_is_user_fbo(ctx->DrawBuffer) &&
> + if (ctx->API == API_OPENGLES2 && _mesa_is_user_fbo(fb) &&
> buffers[output] != GL_NONE &&
> buffers[output] != GL_COLOR_ATTACHMENT0 + output) {
> - _mesa_error(ctx, GL_INVALID_OPERATION, "glDrawBuffers(buffer)");
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(unsupported buffer %s)",
> + caller, _mesa_lookup_enum_by_nr(buffers[output]));
> return;
> }
>
> @@ -457,7 +470,8 @@ _mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> */
> if (destMask[output] & usedBufferMask) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> - "glDrawBuffersARB(duplicated buffer)");
> + "%s(duplicated buffer %s)",
> + caller, _mesa_lookup_enum_by_nr(buffers[output]));
> return;
> }
>
> @@ -467,17 +481,27 @@ _mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> }
>
> /* OK, if we get here, there were no errors so set the new state */
> - _mesa_drawbuffers(ctx, ctx->DrawBuffer, n, buffers, destMask);
> + _mesa_drawbuffers(ctx, fb, n, buffers, destMask);
>
> /*
> - * Call device driver function. Note that n can be equal to 0,
> + * Call device driver function if traditional entry point.
> + * Note that n can be equal to 0,
> * in which case we don't want to reference buffers[0], which
> * may not be valid.
> */
> - if (ctx->Driver.DrawBuffers)
> - ctx->Driver.DrawBuffers(ctx, n, buffers);
> - else if (ctx->Driver.DrawBuffer)
> - ctx->Driver.DrawBuffer(ctx, n > 0 ? buffers[0] : GL_NONE);
> + if (strcmp(caller, "glDrawBuffers") == 0) {
> + if (ctx->Driver.DrawBuffers)
> + ctx->Driver.DrawBuffers(ctx, n, buffers);
> + else if (ctx->Driver.DrawBuffer)
> + ctx->Driver.DrawBuffer(ctx, n > 0 ? buffers[0] : GL_NONE);
> + }
As in patches 33 and 35, I think the test should be whether fb
is the currently bound framebuffer.
> +}
> +
> +void GLAPIENTRY
> +_mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
> +{
> + GET_CURRENT_CONTEXT(ctx);
> + _mesa_draw_buffers(ctx, ctx->DrawBuffer, n, buffers, "glDrawBuffers");
> }
>
>
> diff --git a/src/mesa/main/buffers.h b/src/mesa/main/buffers.h
> index 52a2318..66871d7 100644
> --- a/src/mesa/main/buffers.h
> +++ b/src/mesa/main/buffers.h
> @@ -48,6 +48,10 @@ _mesa_DrawBuffer( GLenum mode );
> extern void GLAPIENTRY
> _mesa_NamedFramebufferDrawBuffer(GLuint framebuffer, GLenum buf);
>
> +extern void
> +_mesa_draw_buffers(struct gl_context *ctx, struct gl_framebuffer *fb,
> + GLsizei n, const GLenum *buffers, const char *caller);
> +
> extern void GLAPIENTRY
> _mesa_DrawBuffers(GLsizei n, const GLenum *buffers);
>
>
More information about the mesa-dev
mailing list