[1/3] mesa: rework error handling in glDrawBuffers

Submitted by Marek Olšák on April 27, 2019, 3:31 a.m.

Details

Message ID 20190427033147.12859-2-maraeo@gmail.com
State New
Headers show
Series "Pbuffer fixes" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Marek Olšák April 27, 2019, 3:31 a.m.
From: Marek Olšák <marek.olsak@amd.com>

It's needed by the next pbuffer fix, which changes the behavior of
draw_buffer_enum_to_bitmask, so it can't be used to help with error
checking.
---
 src/mesa/main/buffers.c | 53 ++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 24 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
index d98c015bb24..2148fa1316c 100644
--- a/src/mesa/main/buffers.c
+++ b/src/mesa/main/buffers.c
@@ -430,65 +430,70 @@  draw_buffers(struct gl_context *ctx, struct gl_framebuffer *fb, GLsizei n,
          _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid buffers)", caller);
          return;
       }
    }
 
    supportedMask = supported_buffer_bitmask(ctx, fb);
    usedBufferMask = 0x0;
 
    /* complicated error checking... */
    for (output = 0; output < n; output++) {
-      destMask[output] = draw_buffer_enum_to_bitmask(ctx, buffers[output]);
-
       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;
-         }
-
          /* 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):
+          * From the OpenGL 4.5 specification, 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 (util_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]));
+         if (buffers[output] == GL_BACK &&
+             _mesa_is_winsys_fbo(fb) &&
+             _mesa_is_desktop_gl(ctx) &&
+             ctx->Version >= 40) {
+            if (n != 1) {
+               _mesa_error(ctx, GL_INVALID_OPERATION, "%s(with GL_BACK n must be 1)",
+                           caller);
                return;
             }
+         } else if (buffers[output] == GL_FRONT ||
+                    buffers[output] == GL_LEFT ||
+                    buffers[output] == GL_RIGHT ||
+                    buffers[output] == GL_FRONT_AND_BACK) {
+            _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid buffer %s)",
+                        caller, _mesa_enum_to_string(buffers[output]));
+            return;
+         }
+      }
+
+      destMask[output] = draw_buffer_enum_to_bitmask(ctx, buffers[output]);
+
+      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 .

Comments

Hi Marek,

one comment/failure inline:

On Saturday, 27 April 2019 05:31:45 CEST Marek Olšák wrote:
> From: Marek Olšák <marek.olsak@amd.com>
> 
> It's needed by the next pbuffer fix, which changes the behavior of
> draw_buffer_enum_to_bitmask, so it can't be used to help with error
> checking.
> ---
>  src/mesa/main/buffers.c | 53 ++++++++++++++++++++++-------------------
>  1 file changed, 29 insertions(+), 24 deletions(-)
> 
> diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
> index d98c015bb24..2148fa1316c 100644
> --- a/src/mesa/main/buffers.c
> +++ b/src/mesa/main/buffers.c
> @@ -430,65 +430,70 @@ draw_buffers(struct gl_context *ctx, struct gl_framebuffer *fb, GLsizei n,
>           _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid buffers)", caller);
>           return;
>        }
>     }
>  
>     supportedMask = supported_buffer_bitmask(ctx, fb);
>     usedBufferMask = 0x0;
>  
>     /* complicated error checking... */
>     for (output = 0; output < n; output++) {
> -      destMask[output] = draw_buffer_enum_to_bitmask(ctx, buffers[output]);
> -
>        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;
> -         }
> -
>           /* 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):
> +          * From the OpenGL 4.5 specification, 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 (util_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]));
> +         if (buffers[output] == GL_BACK &&
> +             _mesa_is_winsys_fbo(fb) &&
> +             _mesa_is_desktop_gl(ctx) &&
> +             ctx->Version >= 40) {
> +            if (n != 1) {
> +               _mesa_error(ctx, GL_INVALID_OPERATION, "%s(with GL_BACK n must be 1)",
> +                           caller);
>                 return;
>              }
> +         } else if (buffers[output] == GL_FRONT ||
> +                    buffers[output] == GL_LEFT ||
> +                    buffers[output] == GL_RIGHT ||
> +                    buffers[output] == GL_FRONT_AND_BACK) {
> +            _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid buffer %s)",
> +                        caller, _mesa_enum_to_string(buffers[output]));
> +            return;
> +         }
> +      }

According to the comment above "Note "special value BACK". GL_BACK...",
there must also be a GL_INVALID_ENUM on GL_BACK when not in opengles. Right?

That specific case can be tested using
MESA_GL_VERSION_OVERRIDE=3.3 piglit/bin/gl-3.1-draw-buffers-errors 

best

Mathias

> +
> +      destMask[output] = draw_buffer_enum_to_bitmask(ctx, buffers[output]);
> +
> +      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 .
>