[Mesa-dev] [PATCH] mesa: Rework crazy error code rules in glDrawBuffers().

[Jordan Justen]

On Fri, Nov 16, 2012 at 2:58 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> Perhaps most importantly, this patch adds comments quoting the relevant
> spec paragraphs above each error condition.
>
> It also makes three changes:
> - For FBOs, GL_COLOR_ATTACHMENTm where m >= MaxDrawBuffers is supposed
>   to generate INVALID_OPERATION (not INVALID_ENUM).
> - Constants that refer to multiple buffers (such as FRONT, BACK, LEFT,
>   RIGHT, and FRONT_AND_BACK) are supposed to generate INVALID_OPERATION,
>   not INVALID_ENUM.
> - In ES 3.0, for FBOs, buffers[i] must be NONE or GL_COLOR_ATTACHMENTi
>   or else INVALID_OPERATION occurs.  (This is a new restriction.)
>
> Fixes es3conform's draw-buffers-api test.
>
> Cc: Matt Turner <mattst88 at gmail.com>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/main/buffers.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 62 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
> index cc268b6..1dd420b 100644
> --- a/src/mesa/main/buffers.c
> +++ b/src/mesa/main/buffers.c
> @@ -290,6 +290,10 @@ _mesa_DrawBuffersARB(GLsizei n, const GLenum *buffers)
>
>     /* 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 || n > (GLsizei) ctx->Const.MaxDrawBuffers) {
>        _mesa_error(ctx, GL_INVALID_VALUE, "glDrawBuffersARB(n)");
> @@ -305,20 +309,75 @@ _mesa_DrawBuffersARB(GLsizei n, const GLenum *buffers)
>           destMask[output] = 0x0;
>        }
>        else {
> +         /* From the OpenGL 3.0 specification, page 258:
> +          * "If the GL is bound to a framebuffer object and DrawBuffers is
> +          *  supplied with [...] COLOR_ATTACHMENTm where m is greater than or
> +          *  equal to the value of MAX_COLOR_ATTACHMENTS, then the error
> +          *  INVALID_OPERATION results."
> +          */
> +         if (_mesa_is_user_fbo(ctx->DrawBuffer) && buffers[output] >=
> +             GL_COLOR_ATTACHMENT0 + ctx->Const.MaxDrawBuffers) {

Seems like it would be better to move 'buffers[output] >=' to the next line.

Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

> +            _mesa_error(ctx, GL_INVALID_OPERATION, "glDrawBuffersARB(buffer)");
> +            return;
> +         }
> +
>           destMask[output] = draw_buffer_enum_to_bitmask(buffers[output]);
> -         if (destMask[output] == BAD_MASK
> -             || _mesa_bitcount(destMask[output]) > 1) {
> +
> +         /* 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, "glDrawBuffersARB(buffer)");
>              return;
>           }
> +
> +         /* From the OpenGL 3.0 specification, page 259:
> +          * "For both the default framebuffer and framebuffer objects, the
> +          *  constants FRONT, BACK, LEFT, RIGHT, and FRONT_AND_BACK are not
> +          *  valid in the bufs array passed to DrawBuffers, and will result in
> +          *  the error INVALID_OPERATION.  This restriction is because these
> +          *  constants may themselves refer to multiple buffers, as shown in
> +          *  table 4.4."
> +          */
> +         if (_mesa_bitcount(destMask[output]) > 1) {
> +            _mesa_error(ctx, GL_INVALID_OPERATION, "glDrawBuffersARB(buffer)");
> +            return;
> +         }
> +
> +         /* From the OpenGL 3.0 specification, page 259:
> +          * "If the GL is bound to the default framebuffer and DrawBuffers is
> +          *  supplied with a constant (other than NONE) that does not indicate
> +          *  any of the color buffers allocated to the GL context by the window
> +          *  system, the error INVALID_OPERATION will be generated.
> +          *
> +          *  If the GL is bound to a framebuffer object and DrawBuffers is
> +          *  supplied with a constant from table 4.6 [...] then the error
> +          *  INVALID_OPERATION results."
> +          */
>           destMask[output] &= supportedMask;
>           if (destMask[output] == 0) {
>              _mesa_error(ctx, GL_INVALID_OPERATION,
>                          "glDrawBuffersARB(unsupported buffer)");
>              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."
> +          */
> +         if (_mesa_is_gles3(ctx) && _mesa_is_user_fbo(ctx->DrawBuffer) &&
> +             buffers[output] != GL_NONE &&
> +             buffers[output] != GL_COLOR_ATTACHMENT0 + output) {
> +            _mesa_error(ctx, GL_INVALID_OPERATION, "glDrawBuffersARB(buffer)");
> +         }
> +
> +         /* 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) {
> -            /* can't specify a dest buffer more than once! */
>              _mesa_error(ctx, GL_INVALID_OPERATION,
>                          "glDrawBuffersARB(duplicated buffer)");
>              return;
> --
> 1.8.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev