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

[Eduardo Lima Mitev]

On 11/15/2016 10:58 AM, Kenneth Graunke wrote:
> On Tuesday, November 15, 2016 9:27:00 AM PST Eduardo Lima Mitev wrote:
>> On 11/15/2016 12:25 AM, Kenneth Graunke wrote:
>>> From: Eduardo Lima Mitev <elima at igalia.com>
>>>
>>> This option was being ignored when packing compressed 3D and cube textures.
>>>
>>> Fixes CTS test (on gen8+):
>>> * GL45-CTS.gtf32.GL3Tests.packed_pixels.packed_pixels_pixelstore
>>>
>>> v2: Drop API checks.
>>> v3 (Ken): Just apply the existing code in more cases.
>>> ---
>>>  src/mesa/drivers/common/meta.c | 20 +++++++++++++++++---
>>>  1 file changed, 17 insertions(+), 3 deletions(-)
>>>
>>> Hey Eduardo,
>>>
>>> It looks like the existing code already tries to handle SkipImages - but
>>> we weren't applying it for 3D and cubemap textures.  I found a spec quote
>>> in the narrative for GetTexImage that indicates we need to do it for those
>>> as well.
>>>
>>> What do you think of this version?  I preserved your authorship as I wanted
>>> you to get the credit for this bugfix - I just typed it up so that I could
>>> make sure it actually fixed the test, and since I had it typed up, I figured
>>> I'd send it out to save you some time...
>>>
>>
>> Hi Kenneth,
>>
>> Great, this is the correct solution. I tried to make my original patch 
>> work with _mesa_image_address3d() for all targets, as yours, but I 
>> missed resetting SkipPixels and SkipRows for TEXTURE_3D, so it was 
>> regressing some subcases.
>>
>> Patch is:
>>
>> Reviewed-by: Eduardo Lima Mitev <elima at igalia.com>
>>
>> I don't mind you taking authorship. Knowing the correct solution is way 
>> more useful to me :). If you want, take the authorship, put my R-b and 
>> push; otherwise let me know and I'll put your R-b and push it myself.
>>
>> Thanks!
>>
>> Eduardo
> 
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> 
> Go ahead and push it :) Thanks!
> 

Pushed, thanks Ken!