-
Notifications
You must be signed in to change notification settings - Fork 15
Add zoom animation to icon_widget presses #18
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
haha this has strong ps1 vibes |
@ladyada If we can get a color wheel into ColorConverter/OnDiskBitmap, maybe we upgrade to a PS2 vibe? https://cdn.discordapp.com/attachments/682423270083002468/821526384677617704/video0.mov |
ya gotta have it make the crash bandicoot sfx 😂 |
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.
This is looking pretty good to me. Thanks for all of your work on it @kmatch98.
I found a few small comment and other minor changes, but this looks ready to merge after that to me. I'll come back later tonight to test it out on a device as well. I haven't run the latest version yet.
I could have sworn I submitted this review last night but I must have forgotten to click the final button.
Argh! I pushed the wrong branch last night. This commit has the version with the following removed:
Ok, I updated the docstrings and excess print statements and repushed. Sorry for the confusion on this! Thanks for your support and patience on this one. |
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.
I have a few more suggestions after trying this out:
It seems like some images are not supported by imageload. I got this error:
Traceback (most recent call last):
File "code.py", line 190, in <module>
File "/lib/adafruit_displayio_layout/widgets/icon_widget.py", line 240, in zoom_out_animation
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 50, in load
NotImplementedError: True color BMP unsupported
I think we should catch this error and just warn the user that the image they have does not support animation.
Thinking about this a bit more I think it will be better to refactor the animation into a subclass “AnimatedIcon”. There is a lot of baggage that will be unnecessary if someone wants a simple icon. I’ll give it another shot at reorganizing it. |
I refactored this into a subclass of The Important: The Only one icon can be animated at a time. So, all The demo version of the touch_deck using |
@kmatch98 I like the way you refactored this into a subclass. Nice way to add functionality for those that want it, but allow those that don't to skip it. I'm working on testing this out and I'm getting this error when I attempt to do the class initialization:
My line 20 is this initialization call:
I'm not sure if I've ever used init_class before, perhaps I am misunderstanding how that works? |
Thanks for the great catch on the documentation, that was a big one. I updated and it's ready for your further review. Thanks again for checking this out and helping improve it to where it is now. |
I think that we may have some trouble with anochored positioning in IconWidget. While working on setting up a test for this I placed a single IconAnimated with anchored positioning and noticed that the If I position it with |
I think this also affects the main branch, but it's ok to discuss here since it will need to be merged into this PR too. I think the issue can be setting Right now the current definition of self.touch_boundary = (
self.x,
self.y,
image.width,
image.height + _label.bounding_box[3],
) I think this should be updated to only local coordinates: self.touch_boundary = (
0, # local coordinates
0, # local coordinates
image.width,
image.height + _label.bounding_box[3],
) I'll push a PR on this and you can evaluate. Here is the PR: #24 |
This IconAnimated subclass references the IconWidget superclass for setting the So, this PR should be ok for |
I just realized I missed this issue: #18 (review) |
I should be able to give this one a try again tonight. My last few attempts led into finding issues unrelated to this PR with imageload and IconWidget. I did manage to get a test in last night of the IconAnimated but the animation was looking odd to me. Like the animated image got layered on top of the "real" image but the "real" image wasn't hidden, so during the animation both were visible, albeit with the top one mostly obstructing the bottom one. I want to reset my libraries and CircuitPython version to make sure I have a clean environment for testing. I am somewhat suspicious because I've been testing out various different PRs in the core, imageload, and FeatherWing libraries with it. Some of my libraries are modified from their current state. Need to get back to a clean slate. |
I updated this to better handle possible error conditions, including when the Also, I modified the input parameter names to clarify the difference between the Class variables and the Instance variables. Please note that the key instance inputs have changed to I also added an example simpletest along with the "play" button icon. Here are some other icon files: The |
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.
I successfully tested the newest version with:
Adafruit CircuitPython 6.2.0-beta.4 on 2021-03-18; Adafruit PyPortal with samd51j20
Everything is working well.
This looks good to me.
Thanks for all of your work on this @kmatch98! This is a really great addition.
Updating https://github.com/adafruit/Adafruit_CircuitPython_BNO08X to 1.1.0 from 1.0.6: > Merge pull request adafruit/Adafruit_CircuitPython_BNO08x#19 from adafruit/patch-test > Merge pull request adafruit/Adafruit_CircuitPython_BNO08x#14 from yugyesh/patch-3 Updating https://github.com/adafruit/Adafruit_CircuitPython_CAP1188 to 1.2.7 from 1.2.6: > Merge pull request adafruit/Adafruit_CircuitPython_CAP1188#20 from adafruit/linting > "Increase duplicate code check threshold " Updating https://github.com/adafruit/Adafruit_CircuitPython_EMC2101 to 1.1.5 from 1.1.4: > Merge pull request adafruit/Adafruit_CircuitPython_EMC2101#7 from rpavlik/slim-lut > Merge pull request adafruit/Adafruit_CircuitPython_EMC2101#6 from rpavlik/split > Merge pull request adafruit/Adafruit_CircuitPython_EMC2101#9 from rpavlik/fix-lut > "Increase duplicate code check threshold " Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI to 3.5.8 from 3.5.7: > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#130 from adafruit/anecdata-patch-2 > "Increase duplicate code check threshold " Updating https://github.com/adafruit/Adafruit_CircuitPython_TLC5947 to 1.3.4 from 1.3.3: > Merge pull request adafruit/Adafruit_CircuitPython_TLC5947#24 from adafruit/REUSE > "Increase duplicate code check threshold " > Re-added pylint install to build.yml > Removed pylint process from github workflow > Hardcoded Black and REUSE versions > Added pre-commit-config file > Added pre-commit and SPDX copyright Updating https://github.com/adafruit/Adafruit_CircuitPython_TLV493D to 1.2.4 from 1.2.3: > Merge pull request adafruit/Adafruit_CircuitPython_TLV493D#11 from lubarb/setup-edit > "Increase duplicate code check threshold " Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_Cycling_Speed_and_Cadence to 1.1.4 from 1.1.3: > Merge pull request adafruit/Adafruit_CircuitPython_BLE_Cycling_Speed_and_Cadence#6 from adafruit/pre-commit > "Increase duplicate code check threshold " > Re-added pylint install to build.yml > Removed pylint process from github workflow > Hardcoded Black and REUSE versions > Added pre-commit-config file > Added pre-commit and SPDX copyright Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_Layout to 1.6.0 from 1.5.1: > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_Layout#18 from kmatch98/icon_palette > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_Layout#25 from jposada202020/main > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_Layout#24 from kmatch98/touch_fix Updating https://github.com/adafruit/Adafruit_CircuitPython_HID to 4.2.0 from 4.1.7: > Merge pull request adafruit/Adafruit_CircuitPython_HID#62 from jfurcean/add-led-status > "Increase duplicate code check threshold " Updating https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad to 0.14.1 from 0.14.0: > Merge pull request adafruit/Adafruit_CircuitPython_ImageLoad#48 from FoamyGuy/readinto_fallback > Merge pull request adafruit/Adafruit_CircuitPython_ImageLoad#49 from kmatch98/readinto_fix Updating https://github.com/adafruit/Adafruit_CircuitPython_MIDI to 1.3.4 from 1.3.3: > Merge pull request adafruit/Adafruit_CircuitPython_MIDI#34 from adafruit/test-lint > "Increase duplicate code check threshold " Updating https://github.com/adafruit/Adafruit_CircuitPython_PIOASM to 0.3.0 from 0.2.3: > Merge pull request adafruit/Adafruit_CircuitPython_PIOASM#19 from adafruit/add-tests > Merge pull request adafruit/Adafruit_CircuitPython_PIOASM#18 from adafruit/jepler-rxuart-example > "Increase duplicate code check threshold "
Adds animation to the icon_widget, including
zoom_animation
andzoom_out_animation
.Now accommodates the OnDiskBitmap loading to save RAM. Utilizes an class variable as a bitmap buffer. Current definition of the class variables gives room for an 80x80 icon bitmap with a maximum zoom of 1.5x, and color depth up to 512.
My updated test files for the Touch Deck are here: https://github.com/kmatch98/Touch_Deck_Working_Files
(Note: This PR does not include the palette colorwheel demo when "unpressed". I will need additional time to understand how the combination of OnDiskBitmap and
ColorConverter()
works so that we can coopt it for this rainbow-y purpose.)