-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Palette: accept tuples of RGB values #2574
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
Conversation
kattni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding this! @jepler If you'd like another test, please provide me with a test build for CLUE and I will do so.
kattni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested successfully on CLUE with display_text. Thanks for adding this!
8edac05 to
29615fd
Compare
This makes Palette more similar to pixelbuf, and lets e.g., the unmodified "wheel" function be used to set pixel values.
29615fd to
1458719
Compare
kattni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passed! 🌮 Thanks for adding this! Tested on CLUE.
| CFLAGS_INLINE_LIMIT = 15 | ||
| CFLAGS_BOARD = --param inline-unit-growth=12 --param max-inline-insns-auto=15 | ||
| else | ||
| ifeq ($(TRANSLATION), de_DE) | ||
| CFLAGS_INLINE_LIMIT = 15 | ||
| CFLAGS_BOARD = --param inline-unit-growth=12 --param max-inline-insns-auto=15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you test that the pinyin and german builds actually work? Try a simple I2C example or something. I found that setting inline limit below 35 or so can produce broken code.
|
I'm ok merging this in but I want to note why I didn't support tuples. I only supported ints because it's easy to provide a conversion library in Python and having only one type of input keeps the code slimmest. Ints are a better single thing to support because tuples cause at least one more allocation. |
|
I will give I2C a test on an M4 board with the specific optimization flags that we put on pewpew m4. I'll do the proposed merge of color value parsing (of pixelbuf and displayio palette) under a separate PR so that the clue lib doesn't have to wait on this. |
|
BTW, I just suggested I2C because the underlying ASF4 code uses a lot of potentially inlined stuff. There's nothing special about I2C per se. |
|
OK, I am going to close this and open a new PR for two reasons:
|
This makes Palette more similar to pixelbuf, and lets e.g., the unmodified "wheel" function be used to set pixel values.
Testing performed: I adapted an existing demo (pygamer) of mine to directly use wheel() tuples instead of converting them to 24-bit integers, and it worked.