Skip to content

Change method of determining glyph height#1074

Open
aaronbell wants to merge 2 commits intogooglefonts:mainfrom
aaronbell:glyphHeight
Open

Change method of determining glyph height#1074
aaronbell wants to merge 2 commits intogooglefonts:mainfrom
aaronbell:glyphHeight

Conversation

@aaronbell
Copy link
Copy Markdown

Closes #1071

In order to make glyphsLib assign the correct glyph height to every glyph on conversion, it needs a more intelligent approach than simply copying the sTypo values. In this PR, I updated the logic to initially attempt to use data from a BASE table in the font .fea file to determine the correct height values. Barring that, evaluate if the sum of the sTypo metrics are larger than the font upm (which implies some variance from the 'standard' approach). And if all of the above is not usable, or False, then to fall back to the existing logic of using the sTypo metrics or master.ascender / master.descender values.

Implementing this logic will help avoid the need for a post-production script to address problems in the vmtx and vhea tables.

Notes

  1. Glyphs does not currently use this approach to determine glyph heights on output. However, I am proposing such a modification to it as well.

  2. This code is horribly un-optimized and could be greatly improved for speed. I am providing this PR primarily as a proof of concept of the approach, since plumbing it properly would be more involved.

- Updating name of function to drop the "typo" portion as it no longer automatically pulls the typo values
- Now attempts to pull values from the BASE table first
- Barring that, will evaluate if the sTypo values are larger than the font upm to determine padding and correct position
- otherwise, will follow previous approach
forgot to include default state
@schriftgestalt
Copy link
Copy Markdown
Collaborator

I didn't look at the code, yet. But it doesn't sound right to use the Base table for the. I would rather define a custom parameter (or even a proper metrics setting) for this. Something like defaultVertHeigth

@aaronbell
Copy link
Copy Markdown
Author

aaronbell commented Mar 16, 2025

@schriftgestalt there does need to be both a proper ascender and descender value set since they are used for both the advanceHeight and verOrigin entries, so do keep that in mind. I’m open to new custom parameters / metrics though.

The BASE table is a useful tool in that it usually provides these values directly, or easily determinable as the values are set by the designer rather than “guessing”.

@aaronbell
Copy link
Copy Markdown
Author

It is also worth noting that even if a new field is added, it is helpful to have a backwards compatible methodology to determine this correctly.

@schriftgestalt
Copy link
Copy Markdown
Collaborator

Then we add two values.
And for fonts that do not have the values, can just as well be produced as is. And if someone is complaining, set the values properly instead of applying some guesswork.

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.

VMTX table / glyph height determination.

2 participants