Skip to content

displayio.Bitmap is now byte-aligned for depth < 8 #9479

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

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

deshipu
Copy link

@deshipu deshipu commented Aug 3, 2024

The traditional layout of pixels in bitmaps of depth less than eight is with the order of values in a byte reversed, but with bytes in the same order as the pixels on the line.

Before now, displayio.Bitmap did reverse the values, but did it on a word (four bytes) basis, not byte, which resulted in groups of four bytes being in the order opposite to the screen order.

This patch fixes this, by making processing of pixels in bitmaps of depth less than 8 bits based on bytes, not words. Since the internal details are changing, any code that accessed bitmaps through the memoryview buffer, or that created bitmaps directly from raw data, and that used depth of less than 8 bits will be affected.

@deshipu
Copy link
Author

deshipu commented Aug 3, 2024

This fixes #6675.

@deshipu
Copy link
Author

deshipu commented Aug 3, 2024

I need to fix the code that generates the bitmaps for the fonts and the blinka logo, but I can't find it. I tried changing gen_display_resources.py, but it doesn't seem to have any effect. I'd really appreciate any help with this.

@deshipu deshipu marked this pull request as draft August 3, 2024 21:21
@deshipu
Copy link
Author

deshipu commented Aug 3, 2024

There are more problems with how the bitmap is ordered. I need to work on this some more.

@deshipu
Copy link
Author

deshipu commented Aug 4, 2024

It was the gen_display_resources.py file after all, I just didn't realize that I also had to change the shift and mask in the data, since they are now computed for a single byte, not a word.

This should be ready for review now.

@deshipu deshipu marked this pull request as ready for review August 4, 2024 23:57
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One header thing. I think this is ok to consider a fix. The memory layout is an implementation detail of bitmap rather than an API.

The traditional layout of pixels in bitmaps of depth less than eight
is with the order of values in a byte reversed, but with bytes in
the same order as the pixels on the line.

Before now, displayio.Bitmap did reverse the values, but did it on a
word (four bytes) basis, not byte, which resulted in groups of four
bytes being in the order opposite to the screen order.

This patch fixes this, by making processing of pixels in bitmaps of
depth less than 8 bits based on bytes, not words. Since the internal
details are changing, any code that accessed bitmaps through the
memoryview buffer, or that created bitmaps directly from raw data,
and that used depth of less than 8 bits will be affected.

Therefore, the gen_display_resources.py script also had to be modified
to account for the changes.
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@tannewt tannewt merged commit 7f92099 into adafruit:main Aug 6, 2024
528 of 529 checks passed
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 this pull request may close these issues.

2 participants