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.pitoi...@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 <tarc...@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");
  }

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to