[Mesa-dev] [PATCH 3/5] mesa/es: Validate glGetFramebufferAttachmentParameter pname in Mesa code rather than the ES wrapper

[Ian Romanick]

On 08/24/2012 02:21 PM, Eric Anholt wrote:
> Ian Romanick <idr at freedesktop.org> writes:
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index 77e9910..1d2047f 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -2366,8 +2366,7 @@ _mesa_GetFramebufferAttachmentParameterivEXT(GLenum target, GLenum attachment,
>>            if (_mesa_is_desktop_gl(ctx)) {
>>               *params = 0;
>>            } else {
>> -            _mesa_error(ctx, GL_INVALID_ENUM,
>> -                        "glGetFramebufferAttachmentParameterivEXT(pname)");
>> +            goto invalid_pname_enum;
>>            }
>>         }
>
> gles3 spec p233:
>
>      "If the value of FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE is NONE, no
>       framebuffer is bound to target. In this case querying pname
>       FRAMEBUFFER_ATTACHMENT_OBJECT_NAME will return zero, and all other
>       queries will generate an INVALID_OPERATION error."
>
> So we need "_mesa_is_desktop_gl(ctx) || _mesa_is_gles3(ctx)".  Also, the
> "err" value set up above needs "|| _mesa_is_gles3(ctx)"

Eh... FFS.  So much for GLES3 being backwards compatible with GLES2. :( 
  I'm going to wait to make that change.  I think doing this now will 
result in ES2 conformance failures on the gles3 branch.  Ugh.

>>            _mesa_error(ctx, err,
>> @@ -2476,9 +2474,10 @@ _mesa_GetFramebufferAttachmentParameterivEXT(GLenum target, GLenum attachment,
>>      case GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE:
>>      case GL_FRAMEBUFFER_ATTACHMENT_DEPTH_SIZE:
>>      case GL_FRAMEBUFFER_ATTACHMENT_STENCIL_SIZE:
>> -      if (!ctx->Extensions.ARB_framebuffer_object) {
>> -         _mesa_error(ctx, GL_INVALID_ENUM,
>> -                     "glGetFramebufferAttachmentParameterivEXT(pname)");
>> +      if ((ctx->API != API_OPENGL || !ctx->Extensions.ARB_framebuffer_object)
>> +          && ctx->API != API_OPENGL_CORE
>
> Since API_OPENGL_CORE implies ARB_fbo, this could potentially be the
> changed to the pattern I've seen elsewhere of
>
> "!_mesa_is_desktop_gl(ctx) || !ctx->Extensions.ARB_fbo"

I think I did it like that because it made for three lines of code that 
check against three different APIs.  I *think* I did the first version 
before the _mesa_is_gles3 helper.  Once I changed over to that, the 
pattern of the three lines was broken.  I'll change it here and back up 
by GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING.

The GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING set will get changed even 
more once we add GL_EXT_srgb.

> (possibly folding that !desktop negation)