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

[Alejandro Piñeiro]

Thanks for the review. And probably asking too much, would you mind to
take a look to the piglit patch that adapts the related piglit test to
the new behaviour?:

https://lists.freedesktop.org/archives/piglit/2016-July/020153.html

On 02/07/16 11:48, Nicolai Hähnle wrote:
> 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;
>>
>