Skip to content
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

Performance degradation for label #70

Closed
kmatch98 opened this issue Jul 15, 2020 · 8 comments
Closed

Performance degradation for label #70

kmatch98 opened this issue Jul 15, 2020 · 8 comments
Labels
bug Something isn't working

Comments

@kmatch98
Copy link
Contributor

kmatch98 commented Jul 15, 2020

#61

Performance degradation was noted by @kevinjwalters

@FoamyGuy
Copy link
Contributor

Thank you for sharing that example code Kevin! I am trying to put together a suite of tests to run whenever display_text is updated, I will add this one.

I am assuming the performance hit is due creating the bitmap and tilegrids, if so, I think we are likely stuck with it at least for custom fonts, since the bitmap is required to make the background look correct.

Something we could look into is adding logic that will revert to the previous background strategy when the default font is used instead of the newer bitmap+tilegrid. I think that should get the performance back to how it was for default font labels.

@kmatch98
Copy link
Contributor Author

Agreed on your comment about treating the builtin font differently. However when I first looked into it I couldn’t figure out how to detect which font type (builtin vs BDF lodes). I think that is the key to unlocking the ability to treat each differently.

@FoamyGuy
Copy link
Contributor

I was able to tinker with this for a bit tonight. I think we can use the font property and test against terminalio.FONT. This example seems to print the correct value for both types of fonts:

import board
import terminalio
from adafruit_display_text import label
from adafruit_bitmap_font import bitmap_font

#font = bitmap_font.load_font("/Helvetica-Bold-16.bdf")
font = terminalio.FONT

text = "Hello world"
text_area = label.Label(font, text=text)
text_area.x = 10
text_area.y = 10
board.DISPLAY.show(text_area)

print(text_area.font == terminalio.FONT)
while True:
    pass

@kmatch98
Copy link
Contributor Author

Looks like good progress. To me it’s a little odd that there is a separate class for type BuiltInFont https://circuitpython.readthedocs.io/en/5.3.x/shared-bindings/fontio/BuiltinFont.html

Does anyone know if there will ever be more BuiltInFont other than terminalio.FONT? If so does this need to be generic for all BuiltInFonts?

My guess is that your code will most cases but wanted to document this here.

@FoamyGuy
Copy link
Contributor

That is a good question. I don't know whether there will ever be others. I do think that some devices have a different built-in font, but it's still referenced with terminalio.FONT I think.

Just in case we could make this a little more generic by checking the type like this:

print(isinstance(text_area.font, fontio.BuiltinFont))

This one seems to be working as well when substituted in the above example.

@evaherrada evaherrada added the bug Something isn't working label Jul 17, 2020
@kmatch98
Copy link
Contributor Author

Some feedback from @tannewt regarding display performance here:
#71

@tannewt
Copy link
Member

tannewt commented Jul 23, 2020

BuiltInFont exists to make it easier to build in a font. If it was python, then it'd be trickier to have the object within the core build itself. It's mean to have the same API as other font classes and be treated the same.

I wouldn't expect more than one to exist in a build because they take a good chunk of space.

@kmatch98
Copy link
Contributor Author

kmatch98 commented Feb 3, 2021

The original issue that was linked was closed, so closing this issue also.

@kmatch98 kmatch98 closed this as completed Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants