Png transparency #82
Merged
Png transparency #82
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Initial testing of the transparency block.
Transparency checking works, but there still may be efficient methods.
Various testing shows using a for loop is the quickest method, beating enumeration and list comprehension.
deshipu
reviewed
Aug 2, 2024
| trns_data = file.read(size) | ||
| for i in range(len(trns_data)): | ||
| if trns_data[i] == 0: | ||
| pal.make_transparent(i) |
There was a problem hiding this comment.
Can we del trns_data once we are done with it?
There was a problem hiding this comment.
Sure can! Latest commit does so now.
Deletes the trans_data list after its no longer needed.
FoamyGuy
approved these changes
Aug 2, 2024
There was a problem hiding this comment.
This looks good to me. I tested it successfully by modifying the existing png example to remove the "manual" call to make_transparent(). My testing was on PyPortal Titano 9.1.1.
Thanks for working on this improvement @ch4nsuk3!
tannewt
added a commit
to adafruit/Adafruit_CircuitPython_Bundle
that referenced
this pull request
Aug 16, 2024
Updating https://github.com/adafruit/Adafruit_CircuitPython_ADS1x15 to 2.3.0 from 2.2.26: > Merge pull request adafruit/Adafruit_CircuitPython_ADS1x15#98 from RoaCode/comparator Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306 to 2.0.3 from 2.0.2: > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306#46 from EAGrahamJr/revert-sleep Updating https://github.com/adafruit/Adafruit_CircuitPython_FT5336 to 1.1.0 from 1.0.2: > Merge pull request adafruit/Adafruit_CircuitPython_FT5336#6 from adafruit/axis_swap Updating https://github.com/adafruit/Adafruit_CircuitPython_IS31FL3731 to 3.4.2 from 3.4.1: > Merge pull request adafruit/Adafruit_CircuitPython_IS31FL3731#56 from FoamyGuy/use_ruff Updating https://github.com/adafruit/Adafruit_CircuitPython_PyPortal to 6.3.5 from 6.3.4: > Merge pull request adafruit/Adafruit_CircuitPython_PyPortal#136 from tannewt/fix_gitattributes Updating https://github.com/adafruit/Adafruit_CircuitPython_RGB_Display to 3.12.5 from 3.12.4: > Merge pull request adafruit/Adafruit_CircuitPython_RGB_Display#126 from simonldwg/ssd1331-remove-prints Updating https://github.com/adafruit/Adafruit_CircuitPython_SCD4X to 1.4.3 from 1.4.2: > Merge pull request adafruit/Adafruit_CircuitPython_SCD4X#20 from kolcz/main Updating https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad to 1.23.0 from 1.21.0: > Merge pull request adafruit/Adafruit_CircuitPython_ImageLoad#84 from deshipu/bug-74 > Merge pull request adafruit/Adafruit_CircuitPython_ImageLoad#83 from deshipu/png-filters > Merge pull request adafruit/Adafruit_CircuitPython_ImageLoad#82 from ch4nsuk3/png-transparency Updating https://github.com/adafruit/Adafruit_CircuitPython_Logging to 5.5.0 from 5.4.0: > Merge pull request adafruit/Adafruit_CircuitPython_Logging#61 from FoamyGuy/formatters > Merge pull request adafruit/Adafruit_CircuitPython_Logging#62 from FoamyGuy/remove_extra_newline Updating https://github.com/adafruit/Adafruit_CircuitPython_MIDI to 1.4.19 from 1.4.18: > Merge pull request adafruit/Adafruit_CircuitPython_MIDI#52 from jordanhemingway-revvity/type_annotations Updating https://github.com/adafruit/Adafruit_CircuitPython_Ticks to 1.1.0 from 1.0.13: > Merge pull request adafruit/Adafruit_CircuitPython_Ticks#11 from adafruit/ticks-exception-like-micropython Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Added the following libraries: Adafruit_CircuitPython_RFM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #65
Handles reading of the chunk in the png file that handles transparency, the trns chunk. This data is based off of the palette indexes and simply indicates alpha values of 0 or 255. Index 0 in this chunk correlates to index 0 of the color palette and goes on from there.
This chunk can have fewer indexes than the palette, but it is not allowed to contain more. Because of this I implemented a ValueError to check for this, and provides a hopefully helpful message about what is specifically wrong. Without this the user will instead receive errors related to palette errors which will likely be confusing if you dont know whats going on under the hood when you load an image.
For performance checking I measured the average load time of 150 imageload calls on a test image: 120x120px, 256 color palette with transparency enabled for the first and last index as a sort of worst case scenario.
I settled on reading the entirety of the chunk at once into a list, then running a for loop through it looking for the interesting values, then setting the
Palette.make_transparentwith the correct index number(s). Reading the data into a list, then working on that list proved to be more efficient speed-wise rather than the alternative of reading a single byte at a time then working with that. Read times can add up quickly.Using a
forloop also proved to be faster than doing list comprehension or usingenumerate()by a noticeable margin as well. It has a bonus side effect of keeping the code easy to understand.Added the following to the pylint disable:
consider-using-enumerate: I did consider it, but it wasnt the best optiontoo-many-statements: The addedifstatements pushes over the limit of 50 statements, to 51. This could be avoided by removing the error check, but I feel it is important to include a useful error message.