On 12.01.2017 23:22, Alejandro Piñeiro wrote:
In most cases, if a call to get_attachment fails is because attachment
is a INVALID_ENUM. But for some specific cases, if COLOR_ATTACHMENTm
(where m >= MAX_COLOR_ATTACHMENTS) is used, it should raise an
INVALID_OPERATION exception instead.

Fixes:
GL45-CTS.direct_state_access.framebuffers_get_attachment_parameter_errors
GL45-CTS.direct_state_access.framebuffers_renderbuffer_attachment_errors
---

The code could be more simple if we do something like this:

   err = is_color_attachment ? GL_INVALID_OPERATION : GL_INVALID_ENUM;
   _mesa_error(...)

Although that would mean having the same error message for both
cases. Not a big deal. Im just slightly biased to differentiate a little.


 src/mesa/main/fbobject.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index ce5eeae..871d08c 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -3495,6 +3495,7 @@ framebuffer_renderbuffer(struct gl_context *ctx,
                          const char *func)
 {
    struct gl_renderbuffer_attachment *att;
+   bool is_color_attachment;

    if (_mesa_is_winsys_fbo(fb)) {
       /* Can't attach new renderbuffers to a window system framebuffer */
@@ -3503,11 +3504,28 @@ framebuffer_renderbuffer(struct gl_context *ctx,
       return;
    }

-   att = get_attachment(ctx, fb, attachment, NULL);
+   att = get_attachment(ctx, fb, attachment, &is_color_attachment);
    if (att == NULL) {
-      _mesa_error(ctx, GL_INVALID_ENUM,
-                  "%s(invalid attachment %s)", func,
-                  _mesa_enum_to_string(attachment));
+      /*
+       * From OpenGL 4.5 spec, section 9.2.7 "Attaching Renderbuffer Images to
+       * a Framebuffer":
+       *    "An INVALID_OPERATION error is generated if attachment is COLOR_-
+       *     ATTACHMENTm where m is greater than or equal to the value of
+       *     MAX_COLOR_- ATTACHMENTS ."

We usually have a new line before the quote block.

+       *
+       * If we are at this point, is because the attachment is not valid, so
+       * if is_color_attachment is true, is because of the previous reason.
+       */
+      if (is_color_attachment) {
+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                     "%s(invalid attachment %s)", func,
+                     _mesa_enum_to_string(attachment));

You probably want "invalid color attachment" here.

With these two comments addressed, the series is

Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>

+      } else {
+         _mesa_error(ctx, GL_INVALID_ENUM,
+                     "%s(invalid attachment %s)", func,
+                     _mesa_enum_to_string(attachment));
+      }
+
       return;
    }

@@ -3609,6 +3627,7 @@ _mesa_get_framebuffer_attachment_parameter(struct 
gl_context *ctx,
                                            GLint *params, const char *caller)
 {
    const struct gl_renderbuffer_attachment *att;
+   bool is_color_attachment;
    GLenum err;

    /* The error code for an attachment type of GL_NONE differs between APIs.
@@ -3676,12 +3695,27 @@ _mesa_get_framebuffer_attachment_parameter(struct 
gl_context *ctx,
    }
    else {
       /* user-created framebuffer FBO */
-      att = get_attachment(ctx, buffer, attachment, NULL);
+      att = get_attachment(ctx, buffer, attachment, &is_color_attachment);
    }

    if (att == NULL) {
-      _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid attachment %s)", caller,
-                  _mesa_enum_to_string(attachment));
+      /*
+       * From OpenGL 4.5 spec, section 9.2.3 "Framebuffer Object Queries":
+       *
+       *    "An INVALID_OPERATION error is generated if a framebuffer object
+       *     is bound to target and attachment is COLOR_ATTACHMENTm where m is
+       *     greater than or equal to the value of MAX_COLOR_ATTACHMENTS."
+       *
+       * If we are at this point, is because the attachment is not valid, so
+       * if is_color_attachment is true, is because of the previous reason.
+       */
+      if (is_color_attachment) {
+         _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid color attachment 
%s)",
+                     caller, _mesa_enum_to_string(attachment));
+      } else {
+         _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid attachment %s)", caller,
+                     _mesa_enum_to_string(attachment));
+      }
       return;
    }


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

Reply via email to