[Mesa-dev] [Mesa-stable] [PATCH] mesa: Add an error condition in glGetFramebufferAttachmentParameteriv()

[Ian Romanick]

On 03/11/2014 05:33 PM, Anuj Phogat wrote:
>>From the OpenGL 4.4 spec page 275:
>   "If pname is FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE, param will
>    contain the format of components of the specified attachment,
>    one of FLOAT, INT, UNSIGNED_INT, SIGNED_NORMALIZED, or
>    UNSIGNED_NORMALIZED for floating-point, signed integer,
>    unsigned integer, signed normalized fixedpoint, or unsigned
>    normalized fixed-point components respectively. If no data
>    storage or texture image has been specified for the attachment,
>    param will contain NONE. This query cannot be performed for a
>    combined depth+stencil attachment, since it does not have a
>    single format."
> 
> Fixes Khronos CTS test: packed_depth_stencil_parameters.test
> 
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> Cc: <mesa-stable at lists.freedesktop.org>
> Khronos Bug# 9170

Couple small comments below.  With the first 2 fixed,

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

It looks like Brian also reviewed this patch.

> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>  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 a9dcc51..ac0d270 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -2765,6 +2765,17 @@ _mesa_GetFramebufferAttachmentParameteriv(GLenum target, GLenum attachment,
>     }
>  
>     if (attachment == GL_DEPTH_STENCIL_ATTACHMENT) {
> +      if (pname == GL_FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE) {
> +         /* From the OpenGL 4.4 spec page 275:

Was this text added in 4.4?  It would be nice to note either here or in
the commit message if / when this error was added.

> +          *   "This query cannot be performed for a combined depth+stencil
> +          *    attachment, since it does not have a single format."
> +          */
> +         _mesa_error(ctx, GL_INVALID_OPERATION,
> +                     "glGetFramebufferAttachmentParameterivEXT("
                                                              ^^^
Drop the EXT.

> +                     "GL_FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE"
> +                     " query can't be made for depth+stencil attachment)");

Man... that's a long error message.  Maybe
"GL_FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE is invalid for depth+stencil
attachment"?   Dunno.  You can ignore this suggestion if you like.

> +         return;
> +      }
>        /* the depth and stencil attachments must point to the same buffer */
>        const struct gl_renderbuffer_attachment *depthAtt, *stencilAtt;
>        depthAtt = get_attachment(ctx, buffer, GL_DEPTH_ATTACHMENT);
>