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

[Iago Toral]

On Fri, 2017-01-13 at 12:15 -0200, Alejandro Piñeiro wrote:
> When the attachment type is NONE (att->Type),
> FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE should be NONE too.
> 
> 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.

Makes sense to me, assuming this is not causing regressions anywhere:
Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

That said, we should file a bug to Khronos so they fix the text
accordingly.

> 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
> ---
> 
> CCing Iago Toral as he implemented the current OBJECT_TYPE compute
> on commit cf4399.
> 
> FWIW, that commit mentions the following test:
> dEQP-
> GLES3.functional.state_query.fbo.framebuffer_attachment_x_size_initia
> l
> 
> And I confirmed that the test doesn't regress with this change.
> 
>  src/mesa/main/fbobject.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 044bd63..7c1b035 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -3756,7 +3756,7 @@
> _mesa_get_framebuffer_attachment_parameter(struct gl_context *ctx,
>         *  stencil bits, respectively, is zero."
>         */
>        *params = (_mesa_is_winsys_fbo(buffer) &&
> -                 ((attachment != GL_DEPTH && attachment !=
> GL_STENCIL) ||
> +                 ((attachment != GL_DEPTH && attachment !=
> GL_STENCIL) &&
>                    (att->Type != GL_NONE)))
>           ? GL_FRAMEBUFFER_DEFAULT : att->Type;
>        return;