-
Notifications
You must be signed in to change notification settings - Fork 38
Consider offering control over creation of background bitmap to reduce memory usage #61
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
Comments
This might also be the cause for my audio becoming a little more choppy. I'm moving around some bitmaps with text underneath them while playing samples in my applicaiton. I'd shrunk them down a bit in size to reduce the pauses in playback to an acceptable amount but it's possible they got worse a week or two ago. I'd assumed this was my imagination but it's possible it coincided with me doing a library refresh and picking up a later |
@kevinjwalters if you have a moment try testing out the branch from this PR: kmatch98#2 It should avoid making the bitmap if background color is not used. |
I tried https://github.com/FoamyGuy/Adafruit_CircuitPython_Display_Text/blob/51e13b2e619c9069f74c60eeb4c2f9f64b052247/adafruit_display_text/label.py with the plotter program from my guide, same error
I think that's expected as it sets a blue backround which triggers the creation of the bitmap even when the monospaced |
Ahh, yeah I think you are right. It would still be making the bitmap now. I'm going to work on getting it finalized later today. I will look into the possibility of skipping the bitmap for termainli.FONT text. |
This was resolved in #59 The default for background color is |
I've tested three different versions of the library from February, June and the most recent one in July and there's both performance differences and rendering differences using That's running on 5.3.0 btw. |
@kevinjwalters Thank you for the video, this is good insight. Can you clarify how you are generating this demo? Are your recreating the text label from scratch or moving its x,y location? If moving x,y are you doing this in a higher level group or relocating this box? If possible please post the demo code, I want to be sure to understand the specific issue that is causing the performance change. |
From the video description: https://github.com/kevinjwalters/circuitpython-examples/blob/master/clue/clue-label-test.py |
Thanks. Sorry I overlooked it in the description. Was too fixated on watching the complex storyline of the video :) |
@FoamyGuy just adding you to this so you will be aware of the performance change listed above. This probably should be added as a new issue. So far I haven’t dug into this yet. |
This appears to have been closed, but I just hit the same issue. code.py is the clue-plotter.py program from this guide: Using library bundle Adafruit CircuitPython 5.3.1 on 2020-07-13; Adafruit CLUE nRF52840 Express with nRF52840
>>> from adafruit_display_text import label
>>> label.__version__
'2.8.0'
>>>
soft reboot
Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.
code.py output:
Traceback (most recent call last):
File "code.py", line 195, in <module>
File "code.py", line 162, in popup_text
File "plotter.py", line 786, in info
File "adafruit_display_text/label.py", line 128, in __init__
File "adafruit_display_text/label.py", line 303, in _update_text
File "adafruit_display_text/label.py", line 212, in _update_background_color
File "adafruit_display_text/label.py", line 169, in _create_background_box
MemoryError: memory allocation failed, allocating 1616 bytes |
@caternuson I think this was closed in favor of #76. |
Thanks @makermelissa. So this is still an open issue, just moved? |
As far as I can tell, that's correct. |
@caternuson @makermelissa the bitmap_label noted in #76 would likely help with this issue on the CLUE. But there are also some potential gains to be made in the existing label by changing the background not to use a fullsized bitmap. Possible alternatives include |
@caternuson and @FoamyGuy Since this could be an urgent issue with this example for the Clue, is there something we can do in the Clue learn guide program to shave a few bytes to make this work, maybe turn off background? See PR #74 that reduces memory if background is set to I don’t have a Clue so I’m unable to assist other than with suggestions. |
Setting it to none would save whatever memory is used by the bitmap background. But the CLUE program probably needs the background to remain legible easily. The only way that I could think of to solve the memory problem but keep the colored background was in #71 but it led to too much complexity within the label. It may be possible to in the CLUE example code to set the background to none (avoiding the bitmap memory) and then "manually" take control of the palette in order to set the color the way that it did before bitmap backgrounds. I can give that a try. |
I think using |
Looking a bit closer the label is inside the Plotter module which is part of the example code. Maybe as a temporary fix it could changed there to use |
Okay I made an attempt to solve it by modifying the plotter module within the example to set the label to use a adafruit/Adafruit_Learning_System_Guides#1192 I'm not sure if this is an ideal solution, but it does seem to be working to allow the example to run on the CLUE. |
I've not followed the recent changes to this library but somewhere since https://github.com/adafruit/Adafruit_CircuitPython_Bundle/releases/tag/20200327 an extra bitmap is created for a background feature. This is the likely candidate for the primary cause of a
MemoryError
exception that now occurs using this library for the application in Adafruit Learn: CLUE Sensor Plotter in CircuitPython. An example of this exception output can be seen in the description of the second issue in Adafruit Forums: Clue_Plotter Problem.This may affect other existing applications too. I'd guess ones with lots of text on screen, large (possibly scaled) text and/or memory tight ones are more at risk.
I'm not sure what the new bitmap is for but a decision on whether this feature should be off by default would be useful and dictate whether existing applications need testing/reviewing/changing.
The text was updated successfully, but these errors were encountered: