[Mesa-dev] [PATCH] meta/GetTexSubImage: Account for GL_PACK_SKIP_IMAGES on compressed textures

[Eduardo Lima Mitev]

On 11/14/2016 07:41 AM, Kenneth Graunke wrote:
> On Sunday, November 13, 2016 10:21:11 AM PST Eduardo Lima Mitev wrote:
>> This option is currently ignored when packing compressed textures.
>>
>> Fixes CTS test (on gen8+):
>> * GL45-CTS.gtf32.GL3Tests.packed_pixels.packed_pixels_pixelstore
>> ---
>>  src/mesa/drivers/common/meta.c | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
>> index 5ab1e6c..7f162c6 100644
>> --- a/src/mesa/drivers/common/meta.c
>> +++ b/src/mesa/drivers/common/meta.c
>> @@ -3255,9 +3255,16 @@ _mesa_meta_GetTexSubImage(struct gl_context *ctx,
>>              packing.SkipRows = 0;
>>              dst = _mesa_image_address3d(&packing, pixels, width, height,
>>                                          format, type, slice, 0, 0);
>> -         }
>> -         else {
>> -            dst = pixels;
>> +         } else {
>> +            /* Account for PACK_SKIP_IMAGES option, if supported */
>> +            if (_mesa_is_desktop_gl(ctx) || _mesa_is_gles3(ctx)) {
>> +               GLint imageStride =
>> +                  _mesa_image_image_stride(&ctx->Pack,width,height,
>> +                                           format, type);
>> +               dst = (GLubyte *) pixels + imageStride * ctx->Pack.SkipImages;
>> +            } else {
>> +               dst = pixels;
>> +            }
>>           }
>>           result = decompress_texture_image(ctx, texImage, slice,
>>                                             xoffset, yoffset, width, height,
>>
>
> Could we do this for all APIs?  I'm not very familiar with this code,
> but it /looks/ like SkipImages will default to 0, so the calculation
> will work out to be the same in both cases.
>

Yes, it should be 0 by default. I added the check to play on the safe 
side (look above there is a "packing.SkipRows = 0;", so implementations 
changing user-settings deliberately).
I will remove the check though. I don't specially like API versions 
checks all over, either.

Will submit a v2 in short. Thanks for the review.

Eduardo