[Mesa-dev] [PATCH] mesa/main: fix error checking logic on CopyImageSubData

[Nicolai Hähnle]

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

On 01.07.2016 19:03, Alejandro Piñeiro wrote:
> For the case (both src or dst) where we had a texobject, but the
> texobject target was not the same that the method target, this spec
> paragraph was appplied:
>
>   /* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
>    * Profile spec says:
>    *
>    *     "An INVALID_VALUE error is generated if either name does not
>    *     correspond to a valid renderbuffer or texture object according
>    *     to the corresponding target parameter."
>    */
>
> But for that case, the correct spec paragraph should be:
>   /* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
>    * Profile spec says:
>    *
>    *     "An INVALID_ENUM error is generated if either target is
>    *      not RENDERBUFFER or a valid non-proxy texture target;
>    *      is TEXTURE_BUFFER or one of the cubemap face selectors
>    *      described in table 8.18; or if the target does not
>    *      match the type of the object."
>    */
>
> specifically the last sentence: "or if the target does not match the
> type of the object".
>
> This patch fixes the error returned (s/INVALID/ENUM) for that case,
> and moves up the INVALID_VALUE spec paragraph, as that case (invalid
> texture object) was handled before.
>
> Fixes:
> GL44-CTS.copy_image.target_miss_match
> ---
>
> Note to Mark Janes: this patch causes the piglit test
> arb_copy_image-api_errors to start failing. I will send a patch to
> piglit in short.
>
>   src/mesa/main/copyimage.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
> index 67a795f..4d18bed 100644
> --- a/src/mesa/main/copyimage.c
> +++ b/src/mesa/main/copyimage.c
> @@ -139,6 +139,12 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum target,
>         struct gl_texture_object *texObj = _mesa_lookup_texture(ctx, name);
>
>         if (!texObj) {
> +         /*
> +          * From GL_ARB_copy_image specification:
> +          * "INVALID_VALUE is generated if either <srcName> or <dstName> does
> +          * not correspond to a valid renderbuffer or texture object according
> +          * to the corresponding target parameter."
> +          */
>            _mesa_error(ctx, GL_INVALID_VALUE,
>                        "glCopyImageSubData(%sName = %u)", dbg_prefix, name);
>            return false;
> @@ -155,12 +161,11 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum target,
>         /* Note that target will not be a cube face name */
>         if (texObj->Target != target) {
>            /*
> -          * From GL_ARB_copy_image specification:
> -          * "INVALID_VALUE is generated if either <srcName> or <dstName> does
> -          * not correspond to a valid renderbuffer or texture object according
> -          * to the corresponding target parameter."
> +          * From GL_ARB_copy_image_specification:
> +          * "INVALID_ENUM is generated if the target does not match the type
> +          * of the object."
>             */
> -         _mesa_error(ctx, GL_INVALID_VALUE,
> +         _mesa_error(ctx, GL_INVALID_ENUM,
>                        "glCopyImageSubData(%sTarget = %s)", dbg_prefix,
>                        _mesa_enum_to_string(target));
>            return false;
>