[Mesa-dev] [PATCH 5/7] mesa: Make TexImage error cases about internalFormat more informative.

[Ian Romanick]

On 02/11/2014 05:11 PM, Eric Anholt wrote:
> I tripped over one of these when debugging meta, and it's a lot nicer to
> just see the internalFormat being complained about.
> ---
>  src/mesa/main/teximage.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 5938f50..46b006d 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -2244,9 +2244,10 @@ compressed_texture_error_check(struct gl_context *ctx, GLint dimensions,
>  
>     /* This will detect any invalid internalFormat value */
>     if (!_mesa_is_compressed_format(ctx, internalFormat)) {
> -      reason = "internalFormat";
> -      error = GL_INVALID_ENUM;
> -      goto error;
> +      _mesa_error(ctx, GL_INVALID_ENUM,
> +                  "glCompressedTexImage%dD(internalFormat=%s)",
> +                  dimensions, _mesa_lookup_enum_by_nr(internalFormat));
> +      return GL_TRUE;

The only thing I don't like about this is now this is the only error
path in this function that just returns.  All of the others goto error.
 Maybe toss a comment after the error label like, "Don't put any clean
up here because not all paths exit here."

>     }
>  
>     switch (internalFormat) {
> @@ -2553,7 +2554,8 @@ copytexture_error_check( struct gl_context *ctx, GLuint dimensions,
>           break;
>        default:
>           _mesa_error(ctx, GL_INVALID_VALUE,
> -                     "glCopyTexImage%dD(internalFormat)", dimensions);
> +                     "glCopyTexImage%dD(internalFormat=%s)", dimensions,
> +                     _mesa_lookup_enum_by_nr(internalFormat));
>           return GL_TRUE;
>        }
>     }
> @@ -2561,7 +2563,8 @@ 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,
> -                  "glCopyTexImage%dD(internalFormat)", dimensions);
> +                  "glCopyTexImage%dD(internalFormat=%s)", dimensions,
> +                  _mesa_lookup_enum_by_nr(internalFormat));
>        return GL_TRUE;
>     }
>  
> @@ -2570,7 +2573,8 @@ copytexture_error_check( struct gl_context *ctx, GLuint dimensions,
>     if (_mesa_is_color_format(internalFormat)) {
>        if (rb_base_format < 0) {
>           _mesa_error(ctx, GL_INVALID_VALUE,
> -                     "glCopyTexImage%dD(internalFormat)", dimensions);
> +                     "glCopyTexImage%dD(internalFormat=%s)", dimensions,
> +                     _mesa_lookup_enum_by_nr(internalFormat));
>           return GL_TRUE;
>        }
>     }
> @@ -2596,7 +2600,8 @@ copytexture_error_check( struct gl_context *ctx, GLuint dimensions,
>        }
>        if (!valid) {
>           _mesa_error(ctx, GL_INVALID_OPERATION,
> -                     "glCopyTexImage%dD(internalFormat)", dimensions);
> +                     "glCopyTexImage%dD(internalFormat=%s)", dimensions,
> +                     _mesa_lookup_enum_by_nr(internalFormat));
>           return GL_TRUE;
>        }
>     }
>