[Mesa-dev] [PATCH v3] mesa/main: Fix FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE for NONE attachment type

[Alejandro Piñeiro]

When the attachment type is NONE (att->Type),
FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE should be NONE always.

Note that technically, the current behaviour follows the spec. From
OpenGL 4.5 spec, Section 9.2.3 "Framebuffer Object Queries":

   "If the value of FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE is NONE, then
    either no framebuffer is bound to target; or the default
    framebuffer is bound, attachment is DEPTH or STENCIL, and the
    number of depth or stencil bits, respectively, is zero."

Reading literally this paragraph, for the default framebuffer, NONE
should be only returned if attachment is DEPTH and STENCIL without
being allocated.

But it doesn't makes too much sense to return DEFAULT_FRAMEBUFFER if
the attachment type is NONE. For example, this can happens if the
attachment is FRONT_RIGHT run on monoscopic mode, as that attachment
is only available on stereo mode.

With the current behaviour, defensive querying of the object type
would not work properly. So you could query the object type checking
for NONE, get DEFAULT_FRAMEBUFFER, and then get and INVALID_OPERATION
when requesting other pnames (like RED_SIZE), as the real attachment
type is NONE.

This fixes:
GL45-CTS.direct_state_access.framebuffers_get_attachment_parameters

v2: don't change the behaviour for att->Type != GL_NONE, as caused
    some ES CTS regressions
v3: simplify condition (Iago)
---

Simplified version based on our IRC chatting. Fixes the test without
any regression detected. Also expand the comment with the spec
quote to justify the lack of explicit DEPTH or STENCIL checks.

 src/mesa/main/fbobject.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 044bd63..6934805 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -3754,11 +3754,13 @@ _mesa_get_framebuffer_attachment_parameter(struct gl_context *ctx,
        *  either no framebuffer is bound to target; or the default framebuffer
        *  is bound, attachment is DEPTH or STENCIL, and the number of depth or
        *  stencil bits, respectively, is zero."
+       *
+       * Note that we don't need explicit checks on DEPTH and STENCIL, because
+       * on the case the spec is pointing, att->Type is already NONE, so we
+       * just need to check att->Type.
        */
-      *params = (_mesa_is_winsys_fbo(buffer) &&
-                 ((attachment != GL_DEPTH && attachment != GL_STENCIL) ||
-                  (att->Type != GL_NONE)))
-         ? GL_FRAMEBUFFER_DEFAULT : att->Type;
+      *params = (_mesa_is_winsys_fbo(buffer) && att->Type != GL_NONE) ?
+         GL_FRAMEBUFFER_DEFAULT : att->Type;
       return;
    case GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME_EXT:
       if (att->Type == GL_RENDERBUFFER_EXT) {
-- 
2.9.3