Skip to content

Incorrect (skewed?) image displayed by image recorded by adafruit_imageload.load() #88

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
KavyanshKhaitan2 opened this issue Oct 8, 2024 · 15 comments · Fixed by #87
Closed

Comments

@KavyanshKhaitan2
Copy link

KavyanshKhaitan2 commented Oct 8, 2024

Expected Behavior

Python logo displays correctly.
Python logo in a ZIP archive
python logo

Behavior shown

Skewed python logo.
image

Please feel free to ask me any further questions!

Code

import board

import displayio
import digitalio
import adafruit_imageload # type: ignore

# Device: M5Stack Cardputer

display = board.DISPLAY
bitmap, pallete = adafruit_imageload.load('/untitled_app.png')

image = displayio.TileGrid(bitmap, pixel_shader=pallete, x=5, y=5)

root_group = displayio.Group()
display.root_group = root_group
root_group.append(image)

G0_btn = digitalio.DigitalInOut(board.BUTTON)
G0_btn.direction = digitalio.Direction.INPUT


# Forever loop so we can see what happened :D
while 1:
    pass

P.S. Original code in old version of this message.

@todbot
Copy link

todbot commented Oct 9, 2024

I cannot replicate this. Assuming the github doesn't modify the PNG you included, I used the following code on a Adafruit FunHouse ESP32-S3:

import time, board, displayio
import adafruit_imageload
display = board.DISPLAY
main_group = displayio.Group()
display.root_group = main_group
bitmap, palette = adafruit_imageload.load("/untitled_app.png")
# display image on root group
image = displayio.TileGrid(bitmap, pixel_shader=palette)
main_group.append(image) # shows the image
# put image in a group at a particular x,y 
image2 = displayio.TileGrid(bitmap, pixel_shader=palette)
image2_group = displayio.Group(x=60, y=70)
image2_group.append(image2)
main_group.append(image2_group)
while True:
    print("hi")
    time.sleep(1)

And the output was as expected:
IMG_4286 (1)

I think the problem is you are re-using TileGrid objects with slightly different dimensions. Instead of doing:

        bitmap, pallete = adafruit_imageload.load(f'/apps/icons/{applist[selection_text.text]}.png')
        group_1_img.bitmap = bitmap
        group_1_img.pixel_shader = pallete

try doing:

        bitmap, pallete = adafruit_imageload.load(f'/apps/icons/{applist[selection_text.text]}.png')
        group_1_img = TileGrid(bitmap, pixel_shader=pallete, x=group_1_img.x, y=group_1_img.y)

@KavyanshKhaitan2
Copy link
Author

KavyanshKhaitan2 commented Oct 9, 2024

@todbot
Hey, thanks! Maybe that is the issue, I'll be back soon and let you know...

@KavyanshKhaitan2
Copy link
Author

@todbot, The solution you proposed did not work. Here is the shortest and simplest code I could write to reproduce this error.

try: from board_definitions import m5stack_cardputer as board
except: import board

import displayio
import digitalio
import adafruit_imageload # type: ignore

# Device: M5Stack Cardputer

display = board.DISPLAY

bitmap, pallete = adafruit_imageload.load('/untitled_app.png')

image = displayio.TileGrid(bitmap, pixel_shader=pallete, x=5, y=5)

root_group = displayio.Group()
display.root_group = root_group
root_group.append(image)

G0_btn = digitalio.DigitalInOut(board.BUTTON)
G0_btn.direction = digitalio.Direction.INPUT


# Forever loop to see what is happening.
while 1:
   pass

The device I am using is the M5Stack Cardputer if that helps.

In this case, I get this output on the screen:
image

Here is the image (/untitled_app.png) I am trying to use in a zip file (so that no platform can put their hands on it :D):
untitled_app.zip

@KavyanshKhaitan2 KavyanshKhaitan2 changed the title Incorrect dimensions recorded by load() Incorrect (skewed?) image displayed by image recorded by adafruit_imageload.load() Oct 9, 2024
@todbot
Copy link

todbot commented Oct 9, 2024

Here is the image (/untitled_app.png) I am trying to use in a zip file (so that no platform can put their hands on it :D): untitled_app.zip

Using the PNG in that zip, I am seeing the same skewing are you are. This reminds me of #74 and it looks like the main way this PNG differs from the one at the top of this issue (recompressed by github) is that it's not palette-based but RGB.

Here's what PNG check shows for your original skewed image and the github one:

% pngcheck -cvtv untitled_app_orig.png  
File: untitled_app_orig.png (353 bytes)
  chunk IHDR at offset 0x0000c, length 13
    50 x 50 image, 8-bit palette, non-interlaced
  chunk sRGB at offset 0x00025, length 1
    rendering intent = perceptual
  chunk gAMA at offset 0x00032, length 4: 0.45455
  chunk PLTE at offset 0x00042, length 9: 3 palette entries
  chunk pHYs at offset 0x00057, length 9: 3779x3779 pixels/meter (96 dpi)
  chunk IDAT at offset 0x0006c, length 225
    zlib: deflated, 4K window, fast compression
    row filters (0 none, 1 sub, 2 up, 3 avg, 4 paeth):
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      (50 out of 50)
  chunk IEND at offset 0x00159, length 0
No errors detected in untitled_app_orig.png (7 chunks, 85.9% compression).

% pngcheck -cvtv untitled_app_github.png
File: untitled_app_github.png (512 bytes)
  chunk IHDR at offset 0x0000c, length 13
    50 x 50 image, 32-bit RGB+alpha, non-interlaced
  chunk sRGB at offset 0x00025, length 1
    rendering intent = perceptual
  chunk gAMA at offset 0x00032, length 4: 0.45455
  chunk pHYs at offset 0x00042, length 9: 3779x3779 pixels/meter (96 dpi)
  chunk IDAT at offset 0x00057, length 405
    zlib: deflated, 16K window, fast compression
    row filters (0 none, 1 sub, 2 up, 3 avg, 4 paeth):
      1 1 2 2 2 2 2 2 2 2 2 2 1 2 2 2 1 2 2 2 2 2 2 2 1
      1 1 2 2 2 2 2 2 2 2 2 2 2 1 2 2 2 2 2 2 2 2 2 2 1
      (50 out of 50)
  chunk IEND at offset 0x001f8, length 0
No errors detected in untitled_app_github.png (6 chunks, 94.9% compression).

So a workaround would be to recompress your images until you find a setting that works with adafruit_imageload. (adding to a github issue seems to work :-)

I tried re-encoding the PNG using ImageMagick with:

magick  untitled_app_orig.png -type palette -colors 64 untitled_app_new.png

And that provokes a new bug in adafruit_imageload:

Traceback (most recent call last):
  File "code.py", line 9, in <module>
  File "adafruit_imageload/__init__.py", line 92, in load
  File "adafruit_imageload/png.py", line 121, in load
IndexError: x must be 0-49

which is the same codeblock that was to fix #74. So I think we're in the right ballpark.

Looks like adafruit_imageload still has some issues with PNGs with palettes.

@RetiredWizard
Copy link
Contributor

RetiredWizard commented Oct 9, 2024

#87 might fix this as well

Edit: Well not exactly, I see your image is only 50x50 but I suspect I was seeing the same issue. I'm not sure if applying #74 to all 8 bit images is the correct fix or if we need to track down what's different in the png files for the skewed images.

@RetiredWizard
Copy link
Contributor

For more data points here are the working/not working images I've been using:
sbob.zip
and the PNG check results for both files:

retiredwizard@serenity:/media/retiredwizard/CIRCUITPY$ pngcheck -cvtv sbob128.png
File: sbob128.png (9394 bytes)
  chunk IHDR at offset 0x0000c, length 13
    128 x 128 image, 8-bit palette, non-interlaced
  chunk zTXt at offset 0x00025, length 194, keyword: Raw profile type exif
    (compressed zTXt text)
  chunk iCCP at offset 0x000f3, length 388
    profile name = ICC profile, compression method = 0 (deflate)
    compressed profile = 375 bytes
  chunk iTXt at offset 0x00283, length 3448, keyword: XML:com.adobe.xmp
    uncompressed, no language tag
    no translated keyword, 3427 bytes of UTF-8 text
  chunk PLTE at offset 0x01007, length 543: 181 palette entries
  chunk pHYs at offset 0x01232, length 9: 2835x2835 pixels/meter (72 dpi)
  chunk tIME at offset 0x01247, length 7:  7 Oct 2024 22:20:24 UTC
  chunk IDAT at offset 0x0125a, length 4676
    zlib: deflated, 32K window, maximum compression
    row filters (0 none, 1 sub, 2 up, 3 avg, 4 paeth):
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 (128 out of 128)
  chunk IEND at offset 0x024aa, length 0
No errors detected in sbob128.png (9 chunks, 42.7% compression).


retiredwizard@serenity:/media/retiredwizard/CIRCUITPY$ pngcheck -cvtv sbob129.png
File: sbob129.png (9278 bytes)
  chunk IHDR at offset 0x0000c, length 13
    129 x 129 image, 8-bit palette, non-interlaced
  chunk zTXt at offset 0x00025, length 192, keyword: Raw profile type exif
    (compressed zTXt text)
  chunk iCCP at offset 0x000f1, length 388
    profile name = ICC profile, compression method = 0 (deflate)
    compressed profile = 375 bytes
  chunk iTXt at offset 0x00281, length 3448, keyword: XML:com.adobe.xmp
    uncompressed, no language tag
    no translated keyword, 3427 bytes of UTF-8 text
  chunk PLTE at offset 0x01005, length 543: 181 palette entries
  chunk pHYs at offset 0x01230, length 9: 2835x2835 pixels/meter (72 dpi)
  chunk tIME at offset 0x01245, length 7:  7 Oct 2024 22:18:22 UTC
  chunk IDAT at offset 0x01258, length 4562
    zlib: deflated, 32K window, maximum compression
    row filters (0 none, 1 sub, 2 up, 3 avg, 4 paeth):
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 (129 out of 129)
  chunk IEND at offset 0x02436, length 0
No errors detected in sbob129.png (9 chunks, 44.2% compression).

@RetiredWizard
Copy link
Contributor

RetiredWizard commented Oct 9, 2024

Also replacing the < with a <= in the following line of png.py makes the untitled_app.png from the attached zip display properly...
Screenshot from 2024-10-09 16-35-24
Which of course applies the #74 fix todbot mentioned.

Edit: If like me you're trying to figure the work-around out from this screen shot, I chopped line 120, it should be:
bmp[x + pixel, y] = (byte >> ((pixels_per_byte - pixel - 1) * depth)) & (

@RetiredWizard
Copy link
Contributor

RetiredWizard commented Oct 9, 2024

Is it possible there's a bug in the Circuitpython memoryview/array copy functions? I haven't stared at the work-around long enough to be sure but it looks to me like at a depth of 8, it's just performing a manual copy of data_bytes directly to bmp, rather than using the memoryview technique.

@RetiredWizard
Copy link
Contributor

RetiredWizard commented Oct 10, 2024

I don't understand exactly how the memoryview function works but I don't think it's working as expected. I was able to get the untitled_app.png file from the attached zip to display properly by modifying the target memory location from dst = y * scanline to dst = y * scanline + y*2 (line 111). However this modification doesn't work for the sbob129.png file so the memoryview method of copying the byte data is doing something inconsistent possibly depending on the amount of data being moved.

I may try and dig in and figure out exactly how the memoryview function works but until then I think we should just enable the #74 work around for 8 bit images as well. This slows down the image display but moving the data into the bitmap object one byte at a time at least displays the images correctly.

@deshipu
Copy link
Contributor

deshipu commented Oct 10, 2024

Thew problem is that displayio uses uint32_t arrays internally, but the memoryview exposes that as uint8_t array.

In hindsight, and after reading the docs for https://docs.circuitpython.org/projects/imageload/en/latest/api.html#adafruit_imageload.load I think we should have never used memoryview in imageload in the first place, we should be using _load_row only.

@RetiredWizard
Copy link
Contributor

RetiredWizard commented Oct 10, 2024

I think I worked out the adjustment needed to get the memoryview to work. I've tested on 2, 4 & 8 bit mode 8 images and it seems to be working.

https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad/pull/87/files

@deshipu
Copy link
Contributor

deshipu commented Oct 10, 2024

The original code used bitmap[x, y] to fill the pixels, so if that was wrong, then the error is in the displayio bitmap code, not here, and fixing it here is the wrong thing to do.

@RetiredWizard
Copy link
Contributor

RetiredWizard commented Oct 10, 2024

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

@KavyanshKhaitan2
Copy link
Author

Also replacing the < with a <= in the following line of png.py makes the untitled_app.png from the attached zip display properly... Screenshot from 2024-10-09 16-35-24 Which of course applies the #74 fix todbot mentioned.

Edit: If like me you're trying to figure the work-around out from this screen shot, I chopped line 120, it should be: bmp[x + pixel, y] = (byte >> ((pixels_per_byte - pixel - 1) * depth)) & (

Why dont you create a pull request then?

Also, I had created this image using getpaint.net if anyone is wondering, its a great, lightweight image editor :)

@RetiredWizard
Copy link
Contributor

RetiredWizard commented Oct 10, 2024

Why dont you create a pull request then?

#87 resolves the issue. The <= change was one of the first commits to the PR but after working on it for many hours I was able to get the code to work without the work around which is noticeably faster for larger image files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants