[Mesa-dev] [PATCH] mesa: fix error handling in DrawBuffers

[Tapani Pälli]

Patch rearranges error checking so that enum checking provided via
destmask happens before other checks. It needs to be done in this
order because other error checks do not work properly if there were
invalid enums passed.

Patch also refines one existing check and it's documentation to match
GLES 3.0 spec (also in later specs). This was somewhat mysteriously
referring to desktop GL but had a check for gles3.

Fixes following dEQP tests:

   dEQP-GLES31.functional.debug.negative_coverage.get_error.buffer.draw_buffers

no CI regressions observed.

Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98134
---
 src/mesa/main/buffers.c | 71 ++++++++++++++++++++++++++-----------------------
 1 file changed, 37 insertions(+), 34 deletions(-)

diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
index 86696b8..2b24e5a 100644
--- a/src/mesa/main/buffers.c
+++ b/src/mesa/main/buffers.c
@@ -389,17 +389,48 @@ draw_buffers(struct gl_context *ctx, struct gl_framebuffer *fb,
 
    /* complicated error checking... */
    for (output = 0; output < n; output++) {
-      /* Section 4.2 (Whole Framebuffer Operations) of the OpenGL 3.0
+      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.0 specification, page 256:
+       * "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_ENUM. This restriction is because these
+       *  constants may themselves refer to multiple buffers, as shown in
+       *  table 4.4."
+       *  Previous versions of the OpenGL specification say INVALID_OPERATION,
+       *  but the Khronos conformance tests expect INVALID_ENUM.
+       */
+      if (_mesa_bitcount(destMask[output]) > 1) {
+         _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:
        *
-       *     "Each buffer listed in bufs must be BACK, NONE, or one of the values
-       *      from table 4.3 (NONE, COLOR_ATTACHMENTi)"
+       *     "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) && buffers[output] != GL_NONE &&
-          buffers[output] != GL_BACK &&
+      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_ENUM, "glDrawBuffers(buffer)");
+         _mesa_error(ctx, GL_INVALID_OPERATION, "glDrawBuffers(buffer)");
          return;
       }
 
@@ -423,34 +454,6 @@ draw_buffers(struct gl_context *ctx, struct gl_framebuffer *fb,
             return;
          }
 
-         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.0 specification, page 256:
-          * "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_ENUM. This restriction is because these
-          *  constants may themselves refer to multiple buffers, as shown in
-          *  table 4.4."
-          *  Previous versions of the OpenGL specification say INVALID_OPERATION,
-          *  but the Khronos conformance tests expect INVALID_ENUM.
-          */
-         if (_mesa_bitcount(destMask[output]) > 1) {
-            _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid buffer %s)",
-                        caller, _mesa_enum_to_string(buffers[output]));
-            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
-- 
2.7.4