[Mesa-dev] [PATCH 066/101] mesa: add draw_buffers_error() helper
Samuel Pitoiset
samuel.pitoiset at gmail.com
Wed Jul 26 08:35:29 UTC 2017
On 07/26/2017 04:10 AM, Timothy Arceri wrote:
> On 22/07/17 03:40, Samuel Pitoiset wrote:
>> And make draw_buffers() always inline.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>> src/mesa/main/buffers.c | 223
>> ++++++++++++++++++++++++++----------------------
>> 1 file changed, 119 insertions(+), 104 deletions(-)
>>
>> diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
>> index 9a049e94c7..a2a1a3a741 100644
>> --- a/src/mesa/main/buffers.c
>> +++ b/src/mesa/main/buffers.c
>> @@ -389,9 +389,9 @@ _mesa_NamedFramebufferDrawBuffer(GLuint
>> framebuffer, GLenum buf)
>> * that is considered special and allowed as far as
>> n is one
>> * since 4.5.
>> */
>> -static void
>> -draw_buffers(struct gl_context *ctx, struct gl_framebuffer *fb,
>> - GLsizei n, const GLenum *buffers, const char *caller)
>> +static ALWAYS_INLINE void
>> +draw_buffers(struct gl_context *ctx, struct gl_framebuffer *fb,
>> GLsizei n,
>> + const GLenum *buffers, const char *caller, bool no_error)
>> {
>> GLuint output;
>> GLbitfield usedBufferMask, supportedMask;
>> @@ -399,22 +399,24 @@ draw_buffers(struct gl_context *ctx, struct
>> gl_framebuffer *fb,
>> FLUSH_VERTICES(ctx, 0);
>> - /* Turns out n==0 is a valid input that should not produce an error.
>> - * The remaining code below correctly handles the n==0 case.
>> - *
>> - * From the OpenGL 3.0 specification, page 258:
>> - * "An INVALID_VALUE error is generated if n is greater than
>> - * MAX_DRAW_BUFFERS."
>> - */
>> - if (n < 0) {
>> - _mesa_error(ctx, GL_INVALID_VALUE, "%s(n < 0)", caller);
>> - return;
>> - }
>> + if (!no_error) {
>> + /* Turns out n==0 is a valid input that should not produce an
>> error.
>> + * The remaining code below correctly handles the n==0 case.
>> + *
>> + * From the OpenGL 3.0 specification, page 258:
>> + * "An INVALID_VALUE error is generated if n is greater than
>> + * MAX_DRAW_BUFFERS."
>> + */
>> + 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;
>> + 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, fb);
>
> You seem to have missed an error check here.
>
> /* From the ES 3.0 specification, page 180:
> * "If the GL is bound to the default framebuffer, then n must be 1
> * 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(fb) &&
> (n != 1 || (buffers[0] != GL_NONE && buffers[0] != GL_BACK))) {
> _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid buffers)",
> caller);
> return;
> }
>
> This should be moved into the new if block above.
Good catch, fixed locally. Thanks.
>
> With that fixed this patch is:
>
> Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
>
>> @@ -435,65 +437,67 @@ draw_buffers(struct gl_context *ctx, struct
>> gl_framebuffer *fb,
>> for (output = 0; output < n; output++) {
>> destMask[output] = draw_buffer_enum_to_bitmask(ctx,
>> buffers[output]);
>> - /* From the OpenGL 3.0 specification, page 258:
>> - * "Each buffer listed in bufs must be one of the values from
>> tables
>> - * 4.5 or 4.6. Otherwise, an INVALID_ENUM error is generated.
>> - */
>> - if (destMask[output] == BAD_MASK) {
>> - _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid buffer %s)",
>> - caller, _mesa_enum_to_string(buffers[output]));
>> - return;
>> - }
>> -
>> - /* From the OpenGL 4.5 specification, page 493 (page 515 of the
>> PDF)
>> - * "An INVALID_ENUM error is generated if any value in bufs is
>> FRONT,
>> - * LEFT, RIGHT, or FRONT_AND_BACK . This restriction applies
>> to both
>> - * the default framebuffer and framebuffer objects, and exists
>> because
>> - * these constants may themselves refer to multiple buffers,
>> as shown
>> - * in table 17.4."
>> - *
>> - * And on page 492 (page 514 of the PDF):
>> - * "If the default framebuffer is affected, then each of the
>> constants
>> - * must be one of the values listed in table 17.6 or the
>> special value
>> - * BACK. When BACK is used, n must be 1 and color values are
>> written
>> - * into the left buffer for single-buffered contexts, or into
>> the back
>> - * left buffer for double-buffered contexts."
>> - *
>> - * Note "special value BACK". GL_BACK also refers to multiple
>> buffers,
>> - * but it is consider a special case here. This is a change on
>> 4.5. For
>> - * OpenGL 4.x we check that behaviour. For any previous version
>> we keep
>> - * considering it wrong (as INVALID_ENUM).
>> - */
>> - if (_mesa_bitcount(destMask[output]) > 1) {
>> - if (_mesa_is_winsys_fbo(fb) && ctx->Version >= 40 &&
>> - buffers[output] == GL_BACK) {
>> - if (n != 1) {
>> - _mesa_error(ctx, GL_INVALID_OPERATION, "%s(with
>> GL_BACK n must be 1)",
>> - caller);
>> - return;
>> - }
>> - } else {
>> + if (!no_error) {
>> + /* From the OpenGL 3.0 specification, page 258:
>> + * "Each buffer listed in bufs must be one of the values
>> from tables
>> + * 4.5 or 4.6. Otherwise, an INVALID_ENUM error is generated.
>> + */
>> + if (destMask[output] == BAD_MASK) {
>> _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid buffer %s)",
>> caller, _mesa_enum_to_string(buffers[output]));
>> return;
>> }
>> - }
>> - /* Section 4.2 (Whole Framebuffer Operations) of the OpenGL ES 3.0
>> - * specification says:
>> - *
>> - * "If the GL is bound to a draw framebuffer object, the
>> ith buffer
>> - * listed in bufs must be COLOR_ATTACHMENTi or NONE .
>> Specifying a
>> - * buffer out of order, BACK , or COLOR_ATTACHMENTm where m
>> is greater
>> - * than or equal to the value of MAX_- COLOR_ATTACHMENTS ,
>> will
>> - * generate the error INVALID_OPERATION .
>> - */
>> - if (_mesa_is_gles3(ctx) && _mesa_is_user_fbo(fb) &&
>> - buffers[output] != GL_NONE &&
>> - (buffers[output] < GL_COLOR_ATTACHMENT0 ||
>> - buffers[output] >= GL_COLOR_ATTACHMENT0 +
>> ctx->Const.MaxColorAttachments)) {
>> - _mesa_error(ctx, GL_INVALID_OPERATION,
>> "glDrawBuffers(buffer)");
>> - return;
>> + /* From the OpenGL 4.5 specification, page 493 (page 515 of
>> the PDF)
>> + * "An INVALID_ENUM error is generated if any value in bufs
>> is FRONT,
>> + * LEFT, RIGHT, or FRONT_AND_BACK . This restriction applies
>> to both
>> + * the default framebuffer and framebuffer objects, and
>> exists because
>> + * these constants may themselves refer to multiple buffers,
>> as shown
>> + * in table 17.4."
>> + *
>> + * And on page 492 (page 514 of the PDF):
>> + * "If the default framebuffer is affected, then each of the
>> constants
>> + * must be one of the values listed in table 17.6 or the
>> special value
>> + * BACK. When BACK is used, n must be 1 and color values are
>> written
>> + * into the left buffer for single-buffered contexts, or
>> into the back
>> + * left buffer for double-buffered contexts."
>> + *
>> + * Note "special value BACK". GL_BACK also refers to
>> multiple buffers,
>> + * but it is consider a special case here. This is a change
>> on 4.5.
>> + * For OpenGL 4.x we check that behaviour. For any previous
>> version we
>> + * keep considering it wrong (as INVALID_ENUM).
>> + */
>> + if (_mesa_bitcount(destMask[output]) > 1) {
>> + if (_mesa_is_winsys_fbo(fb) && ctx->Version >= 40 &&
>> + buffers[output] == GL_BACK) {
>> + if (n != 1) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(with
>> GL_BACK n must be 1)",
>> + caller);
>> + return;
>> + }
>> + } else {
>> + _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid buffer
>> %s)",
>> + caller,
>> _mesa_enum_to_string(buffers[output]));
>> + return;
>> + }
>> + }
>> +
>> + /* Section 4.2 (Whole Framebuffer Operations) of the OpenGL
>> ES 3.0
>> + * specification says:
>> + *
>> + * "If the GL is bound to a draw framebuffer object, the
>> ith
>> + * buffer listed in bufs must be COLOR_ATTACHMENTi or
>> NONE .
>> + * Specifying a buffer out of order, BACK , or
>> COLOR_ATTACHMENTm
>> + * where m is greater than or equal to the value of MAX_-
>> + * COLOR_ATTACHMENTS , will generate the error
>> INVALID_OPERATION .
>> + */
>> + if (_mesa_is_gles3(ctx) && _mesa_is_user_fbo(fb) &&
>> + buffers[output] != GL_NONE &&
>> + (buffers[output] < GL_COLOR_ATTACHMENT0 ||
>> + buffers[output] >= GL_COLOR_ATTACHMENT0 +
>> ctx->Const.MaxColorAttachments)) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> "glDrawBuffers(buffer)");
>> + return;
>> + }
>> }
>> if (buffers[output] == GL_NONE) {
>> @@ -508,7 +512,7 @@ draw_buffers(struct gl_context *ctx, struct
>> gl_framebuffer *fb,
>> * or equal to the value of MAX_COLOR_ATTACHMENTS, then
>> the error
>> * INVALID_OPERATION results."
>> */
>> - if (_mesa_is_user_fbo(fb) && buffers[output] >=
>> + if (!no_error && _mesa_is_user_fbo(fb) && buffers[output] >=
>> GL_COLOR_ATTACHMENT0 + ctx->Const.MaxDrawBuffers) {
>> _mesa_error(ctx, GL_INVALID_OPERATION,
>> "%s(buffers[%d] >= maximum number of draw
>> buffers)",
>> @@ -527,37 +531,40 @@ draw_buffers(struct gl_context *ctx, struct
>> gl_framebuffer *fb,
>> * INVALID_OPERATION results."
>> */
>> destMask[output] &= supportedMask;
>> - if (destMask[output] == 0) {
>> - _mesa_error(ctx, GL_INVALID_OPERATION,
>> - "%s(unsupported buffer %s)",
>> - caller, _mesa_enum_to_string(buffers[output]));
>> - return;
>> - }
>> + if (!no_error) {
>> + if (destMask[output] == 0) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "%s(unsupported buffer %s)",
>> + caller,
>> _mesa_enum_to_string(buffers[output]));
>> + return;
>> + }
>> - /* ES 3.0 is even more restrictive. From the ES 3.0 spec,
>> page 180:
>> - * "If the GL is bound to a framebuffer object, the ith
>> buffer listed
>> - * 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(fb) &&
>> - buffers[output] != GL_NONE &&
>> - buffers[output] != GL_COLOR_ATTACHMENT0 + output) {
>> - _mesa_error(ctx, GL_INVALID_OPERATION,
>> - "%s(unsupported buffer %s)",
>> - caller, _mesa_enum_to_string(buffers[output]));
>> - return;
>> - }
>> + /* ES 3.0 is even more restrictive. From the ES 3.0
>> spec, page 180:
>> + * "If the GL is bound to a framebuffer object, the ith
>> buffer
>> + * listed 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(fb) &&
>> + buffers[output] != GL_NONE &&
>> + buffers[output] != GL_COLOR_ATTACHMENT0 + output) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "%s(unsupported buffer %s)",
>> + caller,
>> _mesa_enum_to_string(buffers[output]));
>> + return;
>> + }
>> - /* From the OpenGL 3.0 specification, page 258:
>> - * "Except for NONE, a buffer may not appear more than once
>> in the
>> - * array pointed to by bufs. Specifying a buffer more then
>> once will
>> - * result in the error INVALID_OPERATION."
>> - */
>> - if (destMask[output] & usedBufferMask) {
>> - _mesa_error(ctx, GL_INVALID_OPERATION,
>> - "%s(duplicated buffer %s)",
>> - caller, _mesa_enum_to_string(buffers[output]));
>> - return;
>> + /* From the OpenGL 3.0 specification, page 258:
>> + * "Except for NONE, a buffer may not appear more than
>> once in the
>> + * array pointed to by bufs. Specifying a buffer more
>> then once
>> + * will result in the error INVALID_OPERATION."
>> + */
>> + if (destMask[output] & usedBufferMask) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "%s(duplicated buffer %s)",
>> + caller,
>> _mesa_enum_to_string(buffers[output]));
>> + return;
>> + }
>> }
>> /* update bitmask */
>> @@ -583,11 +590,19 @@ draw_buffers(struct gl_context *ctx, struct
>> gl_framebuffer *fb,
>> }
>> +static void
>> +draw_buffers_error(struct gl_context *ctx, struct gl_framebuffer *fb,
>> GLsizei n,
>> + const GLenum *buffers, const char *caller)
>> +{
>> + draw_buffers(ctx, fb, n, buffers, caller, false);
>> +}
>> +
>> +
>> void GLAPIENTRY
>> _mesa_DrawBuffers(GLsizei n, const GLenum *buffers)
>> {
>> GET_CURRENT_CONTEXT(ctx);
>> - draw_buffers(ctx, ctx->DrawBuffer, n, buffers, "glDrawBuffers");
>> + draw_buffers_error(ctx, ctx->DrawBuffer, n, buffers,
>> "glDrawBuffers");
>> }
>> @@ -607,7 +622,7 @@ _mesa_NamedFramebufferDrawBuffers(GLuint
>> framebuffer, GLsizei n,
>> else
>> fb = ctx->WinSysDrawBuffer;
>> - draw_buffers(ctx, fb, n, bufs, "glNamedFramebufferDrawBuffers");
>> + draw_buffers_error(ctx, fb, n, bufs,
>> "glNamedFramebufferDrawBuffers");
>> }
>>
More information about the mesa-dev
mailing list