Bugfix monospaced glyph scaling #732
Conversation
|
I tried to track down when this first occured. But even when So obviously this has never been checked. This is one reason why the runtime check-and-warning is introduced. Furthermore this PR is rather big (in code change), unfortunately. There is only one symbol font, and only 24 (?) glyphs that are affected, so the question is legitimate: Do we want such a big change. Alternatively one could add Font Awesome piecewise, with different 'ordinary' ScaleGlyphs. But all in all that is not easier to comprehend, and so I ditched that way (and the commits). This is a hard decision, because for me it is a question of maintainability and understand-ability of the code, and it is not clear what is better (for 24 glyphs 😭). In the end my choice fell on the more complex data structure but more straight forward definition and use. But of course we can also have a look on the other design choice and or develop a completely different approach (that I could not come up with). Ugg, I just see that the CI does not run through... |
Finii
force-pushed
the
bugfix/monospaced-glyph-scaling
branch
from
424c9ac to
322c9f6
Compare
|
This exposes a scaling glitch that is in font-patcher:
These are glyphs that are not in the Edit: Ah, and this |
|
Scaling glitch fixed with a7f91ac The errors were always there, just invisible because we did not check :-D This PR is now ready :-) |
$ fontforge --script ~/git/nerd-fonts/font-patcher --mono --complete ~/Download/Overpass_Mono/static/OverpassMono-Regular.ttfLeft side: before this PR, right side after this PR. Gender / planet glyphs rescaled smaller (and independent from other glyphs):
Rounding error fix of non-
|
Finii
force-pushed
the
bugfix/monospaced-glyph-scaling
branch
from
ed0e270 to
363c7d4
Compare
|
Force push to remove merge-commit and do instead a rebase on master. |
[why] Sometimes fonts patched with --mono are not recognized as monospaced fonts. One reason can be that the inserted glyphs are too wide. This will show in the end in the font's advanceWidthMax property which is not congruent to the normal font width. [how] After all the scaling and jiggling we double check if the new glyph already in the to-be-patched is not wider than our design goal for the width. Normally one would expect that this always holds. An exemption could be if we insert ligatures, that are two spaces wide. But at the moment we can not anyhow (because there is no way to add information to the ligature tables right now). If a glyph is wider a warning is issued. No warning is issued if the glyph shall have some overlap. That overlap is taken into account of this check. Signed-off-by: Fini Jastrow <[email protected]>
[why]
While patching for --mono with Font Awesome we get glyphs that are
too wide, for example '_520' (0xF22B). In the symbol font original
it is about 1918 wide. According to ScaleGlyph FONTA_SCALE_LIST
it shall be scaled as 0xF17A - which is only 1664 wide.
This results in too wide symbols in the patched font.
[how]
The ScaleGlyph dict works like this:
- 'ScaleGlyph' Lead glyph, which scaling factor is taken
- 'GlyphsToScale': List of (glyph code) or (list of two glyph codes
that form a closed range)) that shall be scaled
Note that this allows only one group for the whole symbol font, and
that the scaling factor is defined by a specific character, which
needs to be manually selected (on each symbol font update).
The redesign drops 'ScaleGlyph' and changes the glyph list to:
- 'GlyphsToScale': List of ((lists of glyph codes) or (ranges of glyph codes)) that shall be scaled
Each item in 'GlyphsToScale' (a range or an explicit list) forms a group of glyphs that shall be
as rescaled all with the same and maximum possible (for the included glyphs) factor.
The old data structure is automatically rewritten to new entries.
[note]
This commit just changes the algorithm. As the new ScaleGlyph is not
used for any font, there is no change in the patched fonts.
Signed-off-by: Fini Jastrow <[email protected]>
[why] While patching for --mono with Font Awesome we get glyphs that are too wide, for example '_520' (0xF22B). In the symbol font original it is about 1918 wide. According to ScaleGlyph FONTA_SCALE_LIST it shall be scaled as 0xF17A - which is only 1664 wide. [how] Fill the ScaleGlyph of Font Awesome with groups of glyphs that shall be kept same-sized after scaling. Signed-off-by: Fini Jastrow <[email protected]>
[why] The ScaleGlyph just gives a lot of anonymous numbers. [how] Write down which glyphs are rescaled and group them to related groups. They are not changed to 'new' ScaleGlyph method. [note] Checked the current Devicons, and they are completely different to ours. Signed-off-by: Fini Jastrow <[email protected]>
[why] The symbol glyphs are rescaled (when --mono is specified) so that they have a predefined width after insertion in the source font (to be patched font, called 'target font' below). Sometimes the width of the glyph after insertion is off a bit, like 0.2% or so. This seems strange, as we calculate the target width exactly. [how] As expected this are rounding errors. In the old code we take the original width of the glyph when it is in the symbol font and rescale it when it is in the target font. The width of the glyph should be the same in the source and the target font, right? It fact it is not, because the coordinate systems of the two fonts can (and usually are) different. fontforge's magic scales the glyph into the new coordinate system on insertion, such that it is approx the same as before. But when the coordinate system is integer based we get some small rounding errors just from copy and paste. To solve this, we - first copy the glyph from the source into the target font - then determine the glyphs width - then rescale the glyph to the target width Signed-off-by: Fini Jastrow <[email protected]>
Finii
force-pushed
the
bugfix/monospaced-glyph-scaling
branch
from
363c7d4 to
e805b87
Compare
|
Ah, git tricked me also... Did the rebase again with Also changed the stuff from the review above. |
|
I meant to do a final review/test this weekend... maybe this week 🤞🏻 |
…ph-scaling Bugfix monospaced glyph scaling
Description
Even when patched for Monospace (with
-mono) fonts have a hard time to be recognized as monospaced font sometimes.Requirements / Checklist
What does this Pull Request (PR) do?
The first commit just adds a warning mechanism, that shows us when an inserted glyph is not scaled in the expected way. Where expected means: Fits into the normal font width. We do not patch doublewidth glyphs in, jet (afaik).
Then the scaling mechanism (
ScaleGlyph) is expanded to handle more complex situations and do so automatically.This new mechanism is than used for the Font Awesome glyphs.
With just the first commit you can see the extend of the problem. Only Font Awesome is affected:
How should this be manually tested?
FONTA_SCALE_LISTinfontforgefor example, if they scale to a reasonable size. They will be smaller then before, which is the goal of this, but people might notice.ttfdump FontName.ttf | grep 'advanceWidthMax|xAvgCharWidth'showttf FontName.ttf | grep 'advanceWidthMax|avgWidth'Keep in mind that this is just relevant for 'Nerd Font Mono' fonts, i.e. patched with
-mono. Without the parameter the behavior offont-patcheris not changed.Any background context you can provide?
Bottom of #695, for example #695 (comment)
Instead of taking ONE 'main glyph' that rules the scaling of all the special glyphs, we form groups of glyphs that have to be scaled together. Like all the selected (blue) glyphs here, but as individual groups:
The combined bounding box of the group is determined (like when all glyphs in the group are stamped on top of each other and then the bounding box is found), and all members of the group are scaled such that the combined bounding box is maximized to the allowed limits.
For example the group of four arrows (in line 5) is scaled together, but independently to all other glyphs.
Here an example that shows how Font Awesome is ...
buggystrange, the selected (blue) glyphs are clearly too big for the font itself, that does maintain good glyph size consistency otherwise.What are the relevant tickets (if any)?
For example #703 (part of)
Screenshots (if appropriate or helpful)