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

[Anuj Phogat]

On Fri, Apr 18, 2014 at 4:17 PM, Ian Romanick <idr at freedesktop.org> wrote:
> 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.
Yes.  I'll bump the comment.
>
>> +          *   "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);
>>
>