[Mesa-dev] [PATCH 3/3] mesa: Fix glFramebufferTexture* error codes

[Chad Versace]

On 08/11/2016 10:22 AM, Ilia Mirkin wrote:
> On Thu, Aug 11, 2016 at 1:11 PM, Chad Versace <chad at kiwitree.net> wrote:
>> If check_textarget() determined that textarget was incorrect, it emitted
>> GL_INVALID_OPERATION.  This is the correct behavior when textarget is
>> a valid GLenum but an invalid parameter to the current variant of
>> glFramebufferTexture*().
>>
>> However, when textarget is not a GLenum at all, then the GL spec
>> requires that GL_INVALID_ENUM be emitted.
>>
>> Fixes test dEQP-GLES3.functional.negative_api.buffer.framebuffer_texture2d.
>>
>> Cc: Haixia Shi <hshi at chromium.org>
>> Change-Id: I86c492f228720ec8cf9939e741cfc99a5d9fa1bc
>> ---
>>  src/mesa/main/fbobject.c | 32 +++++++++++++++++++++-----------
>>  1 file changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index 2c01526..3360940 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -3029,20 +3029,30 @@ check_textarget(struct gl_context *ctx, int dims, GLenum target,
>>        err = true;
>>     }
>>
>> -   if (err) {
>> -      _mesa_error(ctx, GL_INVALID_OPERATION,
>> -                  "%s(invalid textarget %s)",
>> -                  caller, _mesa_enum_to_string(textarget));
>> -      return false;
>> +   if (!err) {
>> +      /* Make sure textarget is consistent with the texture's type */
>> +      if (target == GL_TEXTURE_CUBE_MAP) {
>> +         err = !_mesa_is_cube_face(textarget);
>> +      } else {
>> +         err = (target != textarget);
>> +      }
>>     }
>>
>> -   /* Make sure textarget is consistent with the texture's type */
>> -   err = (target == GL_TEXTURE_CUBE_MAP) ?
>> -          !_mesa_is_cube_face(textarget): (target != textarget);
>> -
>>     if (err) {
>> -      _mesa_error(ctx, GL_INVALID_OPERATION,
>> -                  "%s(mismatched texture target)", caller);
>> +      const char *enum_str;
>> +
>> +      if (_mesa_enum_to_string2(textarget, &enum_str)) {
>
> so ... if I feed it, say, GL_CLIP_PLANE0 then it's INVALID_OPERATION
> but if I feed it 0xffff then it's INVALID_ENUM?
>
> Probably should just list out the valid enums here. I wouldn't be
> surprised if such a list already existed elsewhere in the file.

I didn't find such a list. I'll resubmit the patch with those enums handled
by a switch at the top of the function.