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

[Kenneth Graunke]

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) {
+            _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