Rewrite font height calculation #1060
Merged
Conversation
This file contains 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
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] The -l option tries to improve (especially) the powerline glyphs by making the baseline to baseline height (cell height) an even number. But it does so only for 2 of the three possible metrics. [how] Assuming the hight is identical for all metrics we just need to add '1' to all ascender values. [note] I'm not sure this does anything. After rounding an odd height might create a 'sharper' triangle tip, not an even height? Do not understand the real reason for the -l option. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] The initial font-patcher used the WIN font metrics to determine the cell height. What has been found was forced into HHEA metrics but without observing the USE_TYPO_METRICS flag. That has been changed to use the TYPO metric instead of the WIN metric when the font wants that. For that the gap value becomes important. This is the current code. It still has problems to detect the correct cell height. A more rigorous approach seem to be needed. [how] The baseline to baseline distance is what we need as 'cell height', to fill it completely with the powerline glyphs. This is a little bit complicated and not really specified, each font rendering application or engine can handle the font metrics differently. But there are some common approaches. So we try to come up with the correct and congruent height, comparing different metrics and issuing a warning on problematic fonts. Afterwards we make all metrics equal (even if they were not before), because our goal is clear now and we impose it onto all platforms. [note] Useful resources: * https://glyphsapp.com/learn/vertical-metrics * https://github.com/source-foundry/font-line Fixes: #1056 Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Finii
force-pushed
the
bugfix/font-metrics
branch
from
9c2000b to
04c682f
Compare
|
Rebase on master, force push |
[why] When HHEA and (depending on USE-TYPO-METRIC) TYPO or WIN are not consistent it is unclear which metric we should trust. In #1056 the complete font bounding box (i.e. yMin and yMax) has been compared to the baseline to baseline distances, and in all these cases the WIN values seem to be best (preserve the glyph bounding box). font-line report fontname.ttf | grep metrics: ttfdump -t head fontname.ttf | grep "yM(in|ax)" [note] Roboto will still be clipped?! There seem to be ridiculously high glyphs in there. Did not check which. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Finii
force-pushed
the
bugfix/font-metrics
branch
from
80c1519 to
6210087
Compare
|
Important information is in #1056! |
Finii
changed the title
This was referenced Jan 27, 2023
Closed
This was referenced Feb 4, 2023
LNKLEO
pushed a commit
to LNKLEO/Nerd
that referenced
this pull request
Nov 24, 2023
Rewrite font height calculation
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.
Description
[why]
The initial font-patcher used the WIN font metrics to determine the cell
height. What has been found was forced into HHEA metrics but without
observing the USE_TYPO_METRICS flag.
That has been changed to use the TYPO metric instead of the WIN metric
when the font wants that. For that the gap value becomes important.
This is the current code. It still has problems to detect the correct
cell height. A more rigorous approach seem to be needed.
[how]
The baseline to baseline distance is what we need as 'cell height', to
fill it completely with the powerline glyphs. This is a little bit
complicated and not really specified, each font rendering application or
engine can handle the font metrics differently. But there are some
common approaches.
So we try to come up with the correct and congruent height, comparing
different metrics and issuing a warning on problematic fonts.
Afterwards we make all metrics equal (even if they were not before),
because our goal is clear now and we impose it onto all platforms.
[note]
Useful resources:
Fixes: #1056
Requirements / Checklist
What does this Pull Request (PR) do?
How should this be manually tested?
Any background context you can provide?
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)