Skip to content

Possible issue with new readinto functionality #47

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
FoamyGuy opened this issue Mar 21, 2021 · 7 comments
Closed

Possible issue with new readinto functionality #47

FoamyGuy opened this issue Mar 21, 2021 · 7 comments

Comments

@FoamyGuy
Copy link
Contributor

I'm getting this error raised from the simpletest script in this repo:

Traceback (most recent call last):
  File "code.py", line 8, in <module>
  File "adafruit_imageload/__init__.py", line 57, in load
  File "adafruit_imageload/__init__.py", line 48, in load
  File "adafruit_imageload/bmp/__init__.py", line 59, in load
  File "adafruit_imageload/bmp/indexed.py", line 91, in load
TypeError: extra keyword arguments given

My device is:

Adafruit CircuitPython 6.2.0-beta.4 on 2021-03-18; Adafruit Feather RP2040 with rp2040
@FoamyGuy
Copy link
Contributor Author

I think we do have possibly two issues actually.

  1. It seems that the core release version made it to an "in-between" state where it does have bitmaptools.read_into but it's missing one of the arguments. This is causing the error mentioned above. I built from the current main branch and this error is not raised on that branch:
Adafruit CircuitPython 6.2.0-beta.4-53-ge084a9267-dirty on 2021-03-20; Adafruit Feather RP2040 with rp2040
  1. there may also be some issue loading images on the branches that do have this argument.
    Using the current release version of this library with read_into() I get this:
    image

using imageload edited not to use read_into()
image

The image in use is here: https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad/blob/master/examples/images/4bit.bmp

@FoamyGuy
Copy link
Contributor Author

Interestingly some images are okay:
image
this is one of the icons from the touch deck project. These are the ones I tested the changes with originally.

perhaps there is something specific about the 4bit.bmp image causing trouble. I'll try out a few other images to see if any other have similar things going on.

@kmatch98 when you have a chance to take a look, I'm curious to see if you have ideas about what could be causing the differences in final image shown for this one.

@kmatch98
Copy link
Contributor

@FoamyGuy I confirmed your issue. I was able to adjust the inputs to _bitmap_readinto to get the shapes to be correct, but some of the colors displayed are all wrong.

I verified that the loaded palette colors exactly the same with both versions. Perhaps the pixel index reads are off by a few bits somewhere? I'm not sure I'll have any time tomorrow to work on this, and anyway we may need to bring jepler into consult.

I added the swap_bytes_in_element = False as shown below and it corrected the shape, but not the colors.

            if _bitmap_readinto:
                _bitmap_readinto(
                    bitmap,
                    file,
                    bits_per_pixel=color_depth,
                    element_size=4,
                    reverse_pixels_in_element=True,
                    swap_bytes_in_element=False,
                    reverse_rows=True,
                )

@kmatch98
Copy link
Contributor

Just a little bit more information to confirm that some of the bitmap color indexes are off by 8 when using the new readinto function. Interestingly, only certain columns or rows seem to be affected.

Here are the bitmap color indexes when using the previous "standard speed" version:

5 5 5 11 11 6 6 6 14 14 14 3 3 4 4 4 5  
5 5 5 11 11 6 6 6 14 14 14 3 3 4 4 4 5  
5 5 5 11 11 6 6 6 14 14 14 3 3 4 4 4 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 15 15 1 1 1 9 9 9 0 0 12 12 12 5  
5 5 5 15 15 1 1 1 9 9 9 0 0 12 12 12 5  
5 5 5 15 15 1 1 1 9 9 9 0 0 12 12 12 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 2 2 10 10 10 8 8 8 13 13 7 7 7 5  
5 5 5 2 2 10 10 10 8 8 8 13 13 7 7 7 5  
5 5 5 2 2 10 10 10 8 8 8 13 13 7 7 7 5  

Here are the bitmap color index values with the new "fast" readinto version:

5 5 5 3 3 6 6 6 6 6 6 3 3 4 4 4 5  
5 5 5 3 3 6 6 6 6 6 6 3 3 4 4 4 5  
5 5 5 3 3 6 6 6 6 6 6 3 3 4 4 4 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 7 7 1 1 1 1 1 1 0 0 4 4 4 5  
5 5 5 7 7 1 1 1 1 1 1 0 0 4 4 4 5  
5 5 5 7 7 1 1 1 1 1 1 0 0 4 4 4 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5 5  
5 5 5 2 2 2 2 2 0 0 0 5 5 7 7 7 5  
5 5 5 2 2 2 2 2 0 0 0 5 5 7 7 7 5  
5 5 5 2 2 2 2 2 0 0 0 5 5 7 7 7 5 

@kmatch98
Copy link
Contributor

kmatch98 commented Mar 21, 2021

Ok, I found the issue, was an incorrect bitmask for BMPs with 4-bit indexes, along with the reverse_pixels_in_element=True.

This will require:

@kmatch98
Copy link
Contributor

I think the two things above were merged so all should be good to close out this issue.

@FoamyGuy
Copy link
Contributor Author

This was resolved by #48 , #49 , and #4454 in the core.

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

No branches or pull requests

2 participants