[Mesa-dev] [PATCH] mesa: return INVALID_OPERATION if there's no image in GetTex*Image

[Ilia Mirkin]

On Fri, Jul 24, 2015 at 9:55 AM, Brian Paul <brianp at vmware.com> wrote:
> The commit subject line doesn't seem to match the code.

It matches the code if you read the whole function... I think. Right
now if there's no image, it'll succeed, whereas after this change,
it'll return INVALID_OPERATION.

Happy to use a different subject line, let me know what you had in mind.

>
> The code looks good though.
>
> Reviewed-by: Brian Paul <brianp at vmware.com>
>
>
> On 07/23/2015 06:40 PM, Ilia Mirkin wrote:
>>
>> Commit 17f714836 (mesa: rearrange texture error checking order) moved
>> the width/height/depth == 0 allowance before checking if the image was
>> there. This was in part due to depth having to be == 1 for 2D images and
>> width having to be == 1 for 1D images. Instead relax the height/depth
>> checks to also accept 0 as valid.
>>
>> With this change,
>>
>>    bin/arb_direct_state_access-get-textures
>>
>> starts passing again.
>>
>> Fixes: 17f714836 (mesa: rearrange texture error checking order)
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>
>> ---
>>   src/mesa/main/texgetimage.c | 18 +++++++++---------
>>   1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
>> index cdbd618..c0ccce3 100644
>> --- a/src/mesa/main/texgetimage.c
>> +++ b/src/mesa/main/texgetimage.c
>> @@ -928,13 +928,6 @@ dimensions_error_check(struct gl_context *ctx,
>>      const struct gl_texture_image *texImage;
>>      int i;
>>
>> -   if (width == 0 || height == 0 || depth == 0) {
>> -      /* Not an error, but nothing to do.  Return 'true' so that the
>> -       * caller simply returns.
>> -       */
>> -      return true;
>> -   }
>> -
>>      if (xoffset < 0) {
>>         _mesa_error(ctx, GL_INVALID_VALUE, "%s(xoffset = %d)", caller,
>> xoffset);
>>         return true;
>> @@ -973,7 +966,7 @@ dimensions_error_check(struct gl_context *ctx,
>>                        "%s(1D, yoffset = %d)", caller, yoffset);
>>            return true;
>>         }
>> -      if (height != 1) {
>> +      if (height > 1) {
>>            _mesa_error(ctx, GL_INVALID_VALUE,
>>                        "%s(1D, height = %d)", caller, height);
>>            return true;
>> @@ -987,7 +980,7 @@ dimensions_error_check(struct gl_context *ctx,
>>                        "%s(zoffset = %d)", caller, zoffset);
>>            return true;
>>         }
>> -      if (depth != 1) {
>> +      if (depth > 1) {
>>            _mesa_error(ctx, GL_INVALID_VALUE,
>>                        "%s(depth = %d)", caller, depth);
>>            return true;
>> @@ -1086,6 +1079,13 @@ dimensions_error_check(struct gl_context *ctx,
>>         }
>>      }
>
> Reviewed-by: Brian Paul <brianp at vmware.com>
>
>
>>
>> +   if (width == 0 || height == 0 || depth == 0) {
>> +      /* Not an error, but nothing to do.  Return 'true' so that the
>> +       * caller simply returns.
>> +       */
>> +      return true;
>> +   }
>> +
>>      return false;
>>   }
>>
>>
>