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

[Ian Romanick]

This patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

I think this patch could also be tagged for the 10.6.x series.

On 07/29/2015 07:01 AM, Samuel Iglesias Gonsalvez wrote:
> From: Eduardo Lima Mitev <elima at igalia.com>
> 
> Page 161 of the OpenGL-ES 3.1 (PDF) spec, and page 207 of the OpenGL 4.5 (PDF),
> both on section '8.6. ALTERNATE TEXTURE IMAGE SPECIFICATION COMMANDS', states:
> 
>     "An INVALID_ENUM error is generated if an invalid value is specified for
>      internalformat".
> 
> It is currently returning INVALID_OPERATION error because
> _mesa_get_read_renderbuffer_for_format() is called before the internalformat
> argument has been validated. To fix this, we move this call down the validation
> process, after _mesa_base_tex_format() has been called. _mesa_base_tex_format()
> effectively serves as a validator for the internal format.
> 
> Fixes 1 dEQP test:
> * dEQP-GLES3.functional.negative_api.texture.copyteximage2d_invalid_format
> 
> Fixes 1 piglit test:
> * spec at oes_compressed_etc1_rgb8_texture@basic
> ---
>  src/mesa/main/teximage.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 0b755ea..fe180d2 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -2645,13 +2645,6 @@ copytexture_error_check( struct gl_context *ctx, GLuint dimensions,
>        return GL_TRUE;
>     }
>  
> -   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;
> -   }
> -
>     /* OpenGL ES 1.x and OpenGL ES 2.0 impose additional restrictions on the
>      * internalFormat.
>      */
> @@ -2664,7 +2657,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_enum_to_string(internalFormat));
>           return GL_TRUE;
> @@ -2673,12 +2666,19 @@ copytexture_error_check( struct gl_context *ctx, GLuint dimensions,
>  
>     baseFormat = _mesa_base_tex_format(ctx, internalFormat);
>     if (baseFormat < 0) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION,
> +      _mesa_error(ctx, GL_INVALID_ENUM,
>                    "glCopyTexImage%dD(internalFormat=%s)", dimensions,
>                    _mesa_enum_to_string(internalFormat));
>        return GL_TRUE;
>     }
>  
> +   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;
> +   }
> +
>     rb_internal_format = rb->InternalFormat;
>     rb_base_format = _mesa_base_tex_format(ctx, rb->InternalFormat);
>     if (_mesa_is_color_format(internalFormat)) {
>