[Mesa-dev] [PATCH 2/5] mesa: Fix error returned by glCopyTexImage2D() upon an invalid internal format

[Eduardo Lima Mitev]

On 06/05/2015 03:43 AM, Ben Widawsky wrote:
> On Wed, Mar 11, 2015 at 10:01:25AM +0100, Eduardo Lima Mitev wrote:
>>
>> Fixes 1 dEQP test:
>> * dEQP-GLES3.functional.negative_api.texture.copyteximage2d_invalid_format
>> ---
>>  src/mesa/main/teximage.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index 7b1a0e6..6382114 100644
>> --- a/src/mesa/main/teximage.c
>> +++ b/src/mesa/main/teximage.c
>> @@ -2619,7 +2619,7 @@ copytexture_error_check( struct gl_context *ctx, GLuint dimensions,
>>  
>>     rb = _mesa_get_read_renderbuffer_for_format(ctx, internalFormat);
>>     if (rb == NULL) {
>> -      _mesa_error(ctx, GL_INVALID_OPERATION,
>> +      _mesa_error(ctx, GL_INVALID_ENUM,
>>                    "glCopyTexImage%dD(read buffer)", dimensions);
>>        return GL_TRUE;
>>     }
> 
> Hmm. Now I wonder if I am reading the wrong docs.
> (https://www.khronos.org/opengles/sdk/docs/man/xhtml/glCopyTexImage2D.xml)
> 
> GL_INVALID_ENUM is generated if internalformat is not an accepted format.
> 
> internalformat
>                     Specifies the internal format of the texture.
>                     Must be one of the following symbolic constants:
>                     GL_ALPHA,
>                     GL_LUMINANCE,
>                     GL_LUMINANCE_ALPHA,
>                     GL_RGB, or
>                     GL_RGBA.
> 
> 
> Would have expected this:
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 7bc1da7..5ca774b 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -2648,7 +2648,7 @@ copytexture_error_check( struct gl_context *ctx, GLuint dimensions,
>        case GL_LUMINANCE_ALPHA:
>           break;
>        default:
> -         _mesa_error(ctx, GL_INVALID_VALUE,
> +         _mesa_error(ctx, GL_INVALID_ENUM,
>                       "glCopyTexImage%dD(internalFormat=%s)", dimensions,
>                       _mesa_lookup_enum_by_nr(internalFormat));
> 

Indeed. The exact issue addressed by this patch has been fixed already
at some point after the patch was sent back in March. That's why you see
a GL_INVALID_VALUE instead of the GL_INVALID_OPERATION from my patch.

However, the dEQP test in question keeps failing because this code was
added before checking for validity of internalformat:

   rb = _mesa_get_read_renderbuffer_for_format(ctx, internalFormat);
   if (rb == NULL) {
      _mesa_error(ctx, GL_INVALID_OPERATION, "glCopyTexImage%dD(read
buffer)", dimensions);
      return GL_TRUE;
   }

So, I will write a new patch to fix the failing test. Current patch is
no longer relevant.

Thanks!
Eduardo