[Mesa-dev] [PATCH] mesa: Disallow GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME on winsys FBO.

2016-03-10 Thread Kenneth Graunke
Fixes:
dEQP-GLES3.functional.negative_api.state.get_framebuffer_attachment_parameteriv

Apparently, GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME is not allowed when
GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE is GL_FRAMEBUFFER_DEFAULT, and
is expected to result in a GL_INVALID_ENUM error.

No GL specification actually defines what GL_FRAMEBUFFER_DEFAULT means.
It probably means the window system FBO.  It also doesn't mention the
behavior of any queries for that type.  Various ARB folks seem fairly
confused about it too.  For now, just do something vaguely like what
dEQP expects.

I think we probably need to check the visual bits against 0 for the
attachment, but we haven't been doing that thusfar, and given how
confusingly this is specified, I can't imagine anyone relying on it.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/main/fbobject.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 0eec9d9..b52a71b 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -3625,6 +3625,17 @@ _mesa_get_framebuffer_attachment_parameter(struct 
gl_context *ctx,
   }
   /* the default / window-system FBO */
   att = _mesa_get_fb0_attachment(ctx, buffer, attachment);
+
+  /* No credible spec text to cite, but see
+   * https://cvs.khronos.org/bugzilla/show_bug.cgi?id=12928#c1
+   * and https://bugs.freedesktop.org/show_bug.cgi?id=31947
+   */
+  if (pname == GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) {
+ _mesa_error(ctx, GL_INVALID_ENUM,
+ "%s(requesting GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME "
+ "when GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE is "
+ "GL_FRAMEBUFFER_DEFAULT is not allowed)", caller);
+  }
}
else {
   /* user-created framebuffer FBO */
-- 
2.7.2

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


Re: [Mesa-dev] [PATCH] mesa: Disallow GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME on winsys FBO.

2016-03-18 Thread Jordan Justen
On 2016-03-10 18:53:28, Kenneth Graunke wrote:
> Fixes:
> dEQP-GLES3.functional.negative_api.state.get_framebuffer_attachment_parameteriv
> 
> Apparently, GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME is not allowed when
> GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE is GL_FRAMEBUFFER_DEFAULT, and
> is expected to result in a GL_INVALID_ENUM error.
> 
> No GL specification actually defines what GL_FRAMEBUFFER_DEFAULT means.
> It probably means the window system FBO.  It also doesn't mention the
> behavior of any queries for that type.  Various ARB folks seem fairly
> confused about it too.  For now, just do something vaguely like what
> dEQP expects.
> 
> I think we probably need to check the visual bits against 0 for the
> attachment, but we haven't been doing that thusfar, and given how
> confusingly this is specified, I can't imagine anyone relying on it.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/main/fbobject.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 0eec9d9..b52a71b 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -3625,6 +3625,17 @@ _mesa_get_framebuffer_attachment_parameter(struct 
> gl_context *ctx,
>}
>/* the default / window-system FBO */
>att = _mesa_get_fb0_attachment(ctx, buffer, attachment);
> +
> +  /* No credible spec text to cite, but see
> +   * https://cvs.khronos.org/bugzilla/show_bug.cgi?id=12928#c1
> +   * and https://bugs.freedesktop.org/show_bug.cgi?id=31947
> +   */

For this comment, what about:

The specs are not clear about how to handle
GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME with the default framebuffer,
but deqp for OpenGLES 3.0 expects an INVALID_ENUM error. This has also
been discussioned in:

https://cvs.khronos.org/bugzilla/show_bug.cgi?id=12928#c1
and https://bugs.freedesktop.org/show_bug.cgi?id=31947

> +  if (pname == GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) {
> + _mesa_error(ctx, GL_INVALID_ENUM,
> + "%s(requesting GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME "
> + "when GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE is "
> + "GL_FRAMEBUFFER_DEFAULT is not allowed)", caller);
> +  }

Shouldn't this be added above the _mesa_get_fb0_attachment() call, and
have a return after _mesa_error like the other INVALID_ENUM cases
above?

With that, Reviewed-by: Jordan Justen 

> }
> else {
>/* user-created framebuffer FBO */
> -- 
> 2.7.2
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev