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

[Tapani Pälli]

On 11/18/2015 08:59 PM, Emil Velikov wrote:
> 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  ?


This patch can be dropped now, I've noticed there's a bunch of other 
failures in this function (that make other dEQP tests fail) and I'd like 
to fix them all at the same time.

// Tapani