Skip to content

label: Use new, optional 'ascent', 'descent' properties #102

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

Merged
merged 3 commits into from
Jan 8, 2021

Conversation

jepler
Copy link
Contributor

@jepler jepler commented Dec 5, 2020

Each time new glyphs have to be loaded from a font, it can take a long time (hundreds of milliseconds). Besides which, not all fonts have the "M" character that is used for estimating the ascent. Instead, use the ascent and descent properties of Font objects, if available. (they are available in bdf and pcf fonts since https://github.com/adafruit/Adafruit_CircuitPython_Bitmap_Font/releases/tag/1.3.0 but terminalio.FONT still doesn't have it)

In particular, before this change the "forkawesome" fonts can't be used, because they have only glyphs starting at Unicode 0xf000 (called the Private Use Area), and may affect webdings/wingdings type fonts as well. (forkawesome fonts converted to pcf are on my blog)

This changes the layout of text slightly. For instance, the height of the "M" glyph in GothamBlack-50.bdf is 35, but the Ascent of the font is 40 (and some characters, such as Å, are taller than the ascent at 45 pixels high). So in this case, text may be shifted down by about 2 to 3 pixels (half of the difference between 35 and 40, as fonts are positioned vertically relative to the half ascent).

To test it, grab https://media.unpythonic.net/emergent-files/01606790241/forkawesome-36.pcf and run the following code (I used a pyportal titano):

from displayio import Group

font = load_font("/forkawesome-36.pcf")
w,h,dx,dy = font.get_bounding_box()

glyphs = "".join(chr(0xf000 + i) for i in range(8))

group = Group()
label = Label(font=font, text=glyphs, background_color=0x111111)
label.anchor_point = (0, 0)
label.anchored_position = (0,0)

group.append(label)
board.DISPLAY.show(group)
while True:
    pass

@jepler jepler marked this pull request as draft December 5, 2020 22:35
Each time new glyphs have to be loaded from a font, it can take a long
time (hundreds of milliseconds).  In a parallel commit, 'ascent' and
'descent' properties will be added to BDF font objects, are essentially
free to compute, and are in any case much quicker than entering
load_glyphs.

This may change the layout of text slightly.  For instance, the height
of the "M" glyph in GothamBlack-50.bdf is 35, but the Ascent of the font
is 40 (and some characters, such as Å, are taller than the ascent at 45
pixels high)
@jepler jepler force-pushed the use-ascent-descent branch from c77ce56 to eaeed21 Compare December 28, 2020 15:26
@jepler jepler marked this pull request as ready for review December 28, 2020 15:51
@jepler jepler requested a review from FoamyGuy December 28, 2020 15:51
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with the forkawesome font and code provided on a PyPortal and that is working well.

Once we handle the one issue with BuiltinFont I think this is good to go.

@FoamyGuy
Copy link
Contributor

I will also follow up with @kmatch98 after this gets merged and we can apply similar changes in the BitmapLabel.

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a few commits, one that falls back safely for the builtin font. This resolves the issue with simpletest, and any other labels using the builtin font.

One that added extra spaces in a few docstrings inside of bitmap_label these lines were otherwise untouched by the PR, but I think it came up due to an update in Sphinx version. Had to update my local tool to replicate and fix.

I tested the current version successfully with the test scripts from this PR and the simpletest in the examples directory

@FoamyGuy FoamyGuy requested a review from a team January 8, 2021 00:49
@FoamyGuy
Copy link
Contributor

FoamyGuy commented Jan 8, 2021

I would like to get another librarian to take one more quick look at this if possible since I made the final commits.

My full test code with imports and all is:

import board
from displayio import Group
from adafruit_display_text import label
from adafruit_bitmap_font import bitmap_font

font = bitmap_font.load_font("/fonts/forkawesome-36.pcf")
w,h,dx,dy = font.get_bounding_box()

glyphs = "".join(chr(0xf000 + i) for i in range(8))

group = Group()
label = label.Label(font=font, text=glyphs, background_color=0x111111)
label.anchor_point = (0, 0)
label.anchored_position = (0,0)

group.append(label)
board.DISPLAY.show(group)
while True:
    pass

This is ready to merge after one more look-over. Thanks in advance!

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine to me. I didn't test it.

@jepler jepler merged commit 92733f5 into adafruit:master Jan 8, 2021
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jan 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants