fix skewed PNG images caused by 'memoryview' bitmap filling

[RetiredWizard]

This PR implements the fix #74 workaround on images that have a width greater than 128 pixels.

I don't know if the PNG image format changes when the image width gets larger than 128 pixels or if there's some decoding error in the library but this workaround seems to resolve the problem I'm seeing for PNG files with a width > 128 pixels.

This PR adjust the memoryview copy used for mode 3 PNG images to avoid the skewing that occurred when the image width was not an even multiple of either 4 pixels (for 8 bit color), 8 pixels (for 4 bit color), 16 pixels (for 2 bit color) or 32 pixels (for 1 bit color).

128x128:

129x129:

fixes #88

[RetiredWizard]

#88 displays this same skew issue on an image that's only 50x50 so this PR is not a complete solution.

[RetiredWizard]

This should probably be closed until the associated issue is understood.

[RetiredWizard]

At least until the memoryview issue is understood, I believe this fixes #88 now.

[deshipu]

If the bitmap[x, y] calculations are wrong, the solution is not to fix it here by rolling back to memoryview and doing correct calculation, but to fix the patch where I introduced the error, so that when other code uses bitmap[x, y] it works correctly.

The patch in question is adafruit/circuitpython@2c7b11b

[RetiredWizard]

If the bitmap[x, y] calculations are wrong, the solution is not to fix it here by rolling back to memoryview and doing correct calculation, but to fix the patch where I introduced the error, so that when other code uses bitmap[x, y] it works correctly.

The bitmap[x, y] code wasn't wrong and worked fine, it's just much slower to copy a single byte at a time.

[RetiredWizard]

I realized that without figuring out how the workaround code worked, I never would have come up with the memoryview adjustment so I put the code block in as a comment for documentation and in case it's needed in the future for additional workarounds.

[deshipu]

The bitmap[x, y] code wasn't wrong and worked fine, it's just much slower to copy a single byte at a time.

That is true, but we need the workaround for the stable versions, because the bit alignment was only fixed recently.

[RetiredWizard]

Ah, good point. Is there a reason not to use the circuitpython release in sys.implementation[1] to select the workaround code or do we just have to wait until 9.1 is no longer supported?

Something like:

if (sys.implementation[1][0] == 9 and sys.implementation[1][1] < 2) or sys.implementation[1][0] < 9:
    # Use workaround
else:
    mem[dst : dst + scanline] = data_bytes[src : src + scanline]

[RetiredWizard]

This version reverts to the workaround for CircuitPython 9.1 and earlier. I tested on 8.2.10 as well as 9.2.

Edit: I guess testing 8.x was overkill since technically there's an 8.x library bundle so this update shouldn't impact CP 8.x. Since it worked with 8.x I would think 9.1 should also be fine though 😁
10/11 - I've confirmed CP 9.1/9.2 works with the updated library, including the optimizations below, on all my test images

[RetiredWizard]

you probably don't want to check this condition on every iteration of the loop, better move it outside the loop and duplicate the loop line

Thanks! Good catch, for all my staring at this section of code, I missed that one 😁

[RetiredWizard]

Okay, so I ran some benchmarks on the code section being updated.

Pico2:
230x219 Memoryview: 0.4 seconds
230x219 Workaround: 1.3 seconds

ESP32-S3:
230x219 Memoryview: 0.8 seconds
230x219 Workaround: 1.1 seconds
1023x1023 Memoryview: 37.5 seconds
1023x1023 Workaround: 18.5 seconds

All 8 bit images.

I have no idea why the workaround runs so much faster on the larger files and I couldn't really confirm the findings on the Pico2 since even a 420x420 image wouldn't load into memory.

Based on these results, I'm inclined to suggest we dump the Memoryview method and use the workaround in all cases. A 20 second difference for larger files on the S3 is going to be a lot more noticeable than a 1 second difference on the Pico2.

[deshipu]

Thanks! Good catch, for all my staring at this section of code, I missed that one 😁

Probably because I wrote it that way originally, and I wasn't thinking about speed at all.

[FoamyGuy]

I tested this successfully with the reproducer code and image from the issue.

Thanks for the fix @RetiredWizard and to @deshipu for looking into this.