[Mesa-stable] [PATCH] mesa: fix error type for GetFramebufferAttachmentParameteriv

[Emil Velikov]

Hi Tapani,

On 28 October 2015 at 13:27, Tapani Pälli <tapani.palli at intel.com> wrote:
> On 10/27/2015 06:42 PM, Ian Romanick wrote:
>>
>> On 10/27/2015 12:11 AM, Tapani Pälli wrote:
>>>
>>> Fixes following failing dEQP test:
>>>     dEQP-GLES3.functional.fbo.api.attachment_query_empty_fbo
>>>
>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92649
>>> Cc: "11.0" <mesa-stable at lists.freedesktop.org>
>>> ---
>>>   src/mesa/main/fbobject.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>>> index fe6bdc2..d91fb4a 100644
>>> --- a/src/mesa/main/fbobject.c
>>> +++ b/src/mesa/main/fbobject.c
>>> @@ -3540,8 +3540,9 @@ _mesa_get_framebuffer_attachment_parameter(struct
>>> gl_context *ctx,
>>>      const struct gl_renderbuffer_attachment *att;
>>>      GLenum err;
>>>   -   /* The error differs in GL and GLES. */
>>> -   err = _mesa_is_desktop_gl(ctx) ? GL_INVALID_OPERATION :
>>> GL_INVALID_ENUM;
>>> +   /* The error differs between GL/GLES3 and GLES 2.0. */
>>
>> Could we get quotations from the various specs here? That would have
>> saved me some time. The commit log for 000896c0 as the GLES2 reference.
>> For GLES 3.0,
>>
>> Section 6.1.13 (Framebuffer Object Queries) of the OpenGL ES 3.0.4 spec
>> says:
>>
>> "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."
>>
>> The GLES2 reference in the commit above is Section 6.1.3 (Enumerated
>> Queries).
>>
>> I feel a bit conflicted about this change.  Section F.2 (Differences in
>> Runtime Behavior) of the OpenGL ES 3.0.4 spec lists three subtle (but
>> possibly important) differences between ES2 and ES3.  This list does
>> *not* include changed errors.  I don't have any specific recollection
>> about these changes, but this causes me to believe that these are
>> corrects to ES2.
>>
>> What do the CTS and dEQP do if you...
>>
>> - Always generate GL_INVALID_OPERATION.
>>
>> - Only return an ES2 context.
>>
>> If all the GLES2 conformance tests and the dEQP-GLES2 tests still pass,
>> I would be inclined to just always do GL_INVALID_OPERATION.
>
>
> Yes it seems we can do that, there's no CTS or dEQP regressions when always
> return GL_INVALID_OPERATION.
>
Doesn't seem like this patch landed in master yet. Based on the above
discussion should we drop this off the list and look into a fix
elsewhere in mesa or we're just going to apply the behaviour for all
GL/GLES versions  ?

Thanks
Emil