font-patcher: Make Nerd Fonts Monospaced Again

[Finii]

This reverts commit 59c45ba.

In the following The Commit means the commit that this PR want to revert (remove).

[why]
The Commit breaks the non-mono Nerd Fonts for a lot of people.
The issue which is the reason for This Commit was not very good documented and investigation can not be
seen.

From The Commit's TITLE it should have affected the 2048-em Symbols font only, but in fact all patched fonts were changed.
Maybe that was intended, maybe not.

[how]
This will make the advance width again equal for all glyphs.
This enables the use of the non-mono variant in more terminals.
For wider symbols a space is now (again) needed after the symbol.
That is expected by a lot applications.

Requirements / Checklist

• Read the Contributing Guidelines
• Verified the license of any newly added font, glyph, or glyph set

What does this Pull Request (PR) do?

Make Non-Mono Nerd Fonts monospaced but allow added symbols to extend over the allotted space to the right (negative right side bearings).

How should this be manually tested?

Any background context you can provide?

The behavior has been changed with The Commit 59c45ba that has been written to fix this issue (at Arch) https://bugs.archlinux.org/task/66212.

Most of the fonts in patched-fonts/ were not updated after that change, so the problems with that change became not obvious until recently.

If it is a problem or not depends also on the used terminal application. Some scale individual glyphs (like Windows Terminal), some just write them out.

What has been the problem and why is the solution a problem

A User complained that in their use case (which is not described in detail, but can be in a non-terminal program that shows a status bar somewhere) the added wide symbols overlap with immediately following ordinary letters.

This is the case because obviously the program that shows the symbols and letters follows the advance width, which is the same for every symbol in a monospaced font. The bigger symbols extrude to the right more than the cursor will more, so this is expected.
This is the reason why applications that use the symbols typically add a blank after the symbol itself, to preserve monospace grid and prevent overlap. For example vim-devicons adds an ArtifactFix (i.e. blank ' ') when appropriate. Other programs like 'ls with filetype symbols' also add a blank space directly after a symbol that is to be displayed.

After That Commit the fonts are not monospaced anymore and terminal emulators do one of

• reject the font outright (not monospaced = not usable)
• rescale the too-big glyphs (effectively leaves the user with a badly rescaled version of Nerd Font Mono)
• work in proportional mode (and thereby destroy the vertical alignment of tables that have different symbols with different widths on each line)

The original problem poster should maybe have changed that application to include the customary blank after symbols.
But then it looks like at least some problem commenter use the font in proportional mode like this

The numbers-and-letter part of the font in this example is proportional, and so the poster (rightfully?) expects proportional symbols. I assume the symbols are not patched in but come from the Symbols-Only font (as described in The Commit message 59c45ba.

Lets define some nomenclature:

1. Nerd Font Mono: strictly monospaced font (i.e. all symbols fit into one 'space' (symbols scaled way down)
2. Nerd Font Classic: monospaced font (all 'letters' monospaced, symbols have monospace advance, but extend to right)
3. Nerd Font Proportional: monospaced letters, but proportional symbols (no negative bearings)

Nerd Fonts contained up to 59c45ba the fonts 1 and 2. Afterward the font-patch result is 1 and 3. This PR reverts / changes it back to 1 and 2. Typical use cases of Nerd Fonts expect the overwide symbols of 2 and handle them appropriately. Also 2 is useful in more terminal applications. People who want non-tiny symbols (i.e. not 1) need 2 or a proportional-font-allowed terminal to use 3.
Possibly the font 3 would also be nice to have (or at least allow users to self patch it with a command line option).

What are the relevant tickets (if any)?

• Konsole rendering messed up for non-mono version on master branch #746
• Fira code glyphs size reduced #731
• Version of Cascadia Mono? #520
• Iosevka font glyphs broken #754

The problem comes up in various guises, and the list is not complete.

Screenshots (if appropriate or helpful)

Font 1 example

Symbols scaled down to fit into one advance width (i.e. 1200 here), strictly monospaced, all glyphs have 1200 advance width.
Font taken from this repo, MASTER, patched-fonts/CascadiaCode/Regular/complete/Caskaydia\ Cove\ Regular\ Nerd\ Font\ Complete\ Mono.otf

Font 2 example

Symbols not scaled but changed advance width (i.e. 1200), monospaced, all glyphs have 1200 advance width, symbols have negative right bearing (extrude out to the right).
Font created from this repo, MASTER, fontforge patched-fonts/CascadiaCode/Regular/complete/Caskaydia\ Cove\ Regular\ Nerd\ Font\ Complete.otf patched-fonts/CascadiaCode/Regular/complete/Caskaydia\ Cove\ Regular\ Nerd\ Font\ Complete\ Mono.otf

Font 3 example

Symbols not scaled with original advance width (i.e. 2048 here), not strictly monospaced, all textual glyphs have 1200 advance width, symbols have any advance width.
Font taken from this repo, MASTER, patched-fonts/CascadiaCode/Regular/complete/Caskaydia\ Cove\ Regular\ Nerd\ Font\ Complete.otf

Edit: Make more clear what The Commit means and highlight the term as special

[Finii]

Some additional thoughts:

---

vim-devicons adds an ArtifactFix

There is some back and forth about this, and at one point you (Ryan) say 'proper fix is with the actual font patcher'.
But is it? Given the variety of terminal emulators and how they handle the glyphs.

---

Maybe somehow related: #747 #748 (for some fonts)

---

#746 (comment) (on color changes?! 👀 )
#746 (comment) (@xsrvmy's take on font flavors)
#731 (comment) (my take on flavors)
#746 (comment) (my take on flavors 2)
#731 (comment) (adding space applications)
#731 (comment) (some font theory)
#731 (comment) (and the results explained)

[Finii]

Here a visual representation of what the code does.

After this PR shown first

self.sourceFont.paste()
self.sourceFont[currentSourceFontGlyph].transform(align_matrix)
self.remove_glyph_neg_bearings(self.sourceFont[currentSourceFontGlyph])
self.set_glyph_width_mono(self.sourceFont[currentSourceFontGlyph])

Creates flavor 2

Wide Icon:

	width	advance	left bear.	right bear.
before	2048	2048	0	0
transformed	2048	1624	-424	0
de-bearinged	2048	2048	0	0
monospaced	2048	1200	0	-848

Small Icon:

	width	advance	left bear.	right bear.
before	496	2048	776	776
transformed	496	1624	352	776
de-bearinged	496	1624	352	776
monospaced	496	1200	352	352

And when we swap the last two lines, as done in 59c45ba

self.sourceFont.paste()
self.sourceFont[currentSourceFontGlyph].transform(align_matrix)
self.set_glyph_width_mono(self.sourceFont[currentSourceFontGlyph])
self.remove_glyph_neg_bearings(self.sourceFont[currentSourceFontGlyph])

Creates flavor 3

Wide Icon:

	width	advance	left bear.	right bear.
before	2048	2048	0	0
transformed	2048	1624	-424	0
monospaced	2048	1200	-424	-424
de-bearinged	2048	2048	0	0

Small Icon:

	width	advance	left bear.	right bear.
before	496	2048	776	776
transformed	496	1624	352	776
monospaced	496	1200	352	352
de-bearinged	496	1200	352	352

Comment

This is also to show that the code as it is does not really describe in a good way what is intended. because both set_glyph_width_mono() and remove_glyph_neg_bearings() change .width and .*bearings.
Implicitely, not in the code that one sees. But inside fontforge, because it HAS TO, becaue bearings + advance = width.
Maybe the code should be changed to be more specific about what it wants to achieve.

Appendix: Code

Code used to create the imagery above

diff --git a/font-patcher b/font-patcher
index 1c5c9693..991e33c1 100755
--- a/font-patcher
+++ b/font-patcher
@@ -809,12 +809,44 @@ class font_patcher:
                 if sym_attr['align'] == 'r':
                     x_align_distance += overlap_width
 
+            inserter = 0x5000 if currentSourceFontGlyph == 60036 else 0x5004
+            if currentSourceFontGlyph == 60036 or currentSourceFontGlyph == 58017:
+                print("\nA")
+                print(get_glyph_dimensions(self.sourceFont[currentSourceFontGlyph]))
+                print((self.sourceFont[currentSourceFontGlyph].width, self.sourceFont[currentSourceFontGlyph].left_side_bearing, self.sourceFont[currentSourceFontGlyph].right_side_bearing))
+                self.sourceFont.selection.select(currentSourceFontGlyph)
+                self.sourceFont.copy()
+                self.sourceFont.selection.select(inserter)
+                self.sourceFont.paste()
+                self.sourceFont[inserter].glyphname = "before"
+                inserter += 1
+
             align_matrix = psMat.translate(x_align_distance, y_align_distance)
             self.sourceFont[currentSourceFontGlyph].transform(align_matrix)
+            if currentSourceFontGlyph == 60036 or currentSourceFontGlyph == 58017:
+                print("A after transform")
+                print(get_glyph_dimensions(self.sourceFont[currentSourceFontGlyph]))
+                print((self.sourceFont[currentSourceFontGlyph].width, self.sourceFont[currentSourceFontGlyph].left_side_bearing, self.sourceFont[currentSourceFontGlyph].right_side_bearing))
+                self.sourceFont.selection.select(currentSourceFontGlyph)
+                self.sourceFont.copy()
+                self.sourceFont.selection.select(inserter)
+                self.sourceFont.paste()
+                self.sourceFont[inserter].glyphname = "transformed"
+                inserter += 1
 
             # Ensure after horizontal adjustments and centering that the glyph
             # does not overlap the bearings (edges)
             self.remove_glyph_neg_bearings(self.sourceFont[currentSourceFontGlyph])
+            if currentSourceFontGlyph == 60036 or currentSourceFontGlyph == 58017:
+                print("A after remove_glyph_neg_bearings")
+                print(get_glyph_dimensions(self.sourceFont[currentSourceFontGlyph]))
+                print((self.sourceFont[currentSourceFontGlyph].width, self.sourceFont[currentSourceFontGlyph].left_side_bearing, self.sourceFont[currentSourceFontGlyph].right_side_bearing))
+                self.sourceFont.selection.select(currentSourceFontGlyph)
+                self.sourceFont.copy()
+                self.sourceFont.selection.select(inserter)
+                self.sourceFont.paste()
+                self.sourceFont[inserter].glyphname = "de-bearinged"
+                inserter += 1
 
             # Needed for setting 'advance width' on each glyph so they do not overlap,
             # also ensures the font is considered monospaced on Windows by setting the
@@ -822,6 +854,16 @@ class font_patcher:
             # even the ones that are empty and didn't go through the scaling operations.
             # it should come after setting the glyph bearings
             self.set_glyph_width_mono(self.sourceFont[currentSourceFontGlyph])
+            if currentSourceFontGlyph == 60036 or currentSourceFontGlyph == 58017:
+                print("A after set_glyph_width_mono")
+                print(get_glyph_dimensions(self.sourceFont[currentSourceFontGlyph]))
+                print((self.sourceFont[currentSourceFontGlyph].width, self.sourceFont[currentSourceFontGlyph].left_side_bearing, self.sourceFont[currentSourceFontGlyph].right_side_bearing))
+                self.sourceFont.selection.select(currentSourceFontGlyph)
+                self.sourceFont.copy()
+                self.sourceFont.selection.select(inserter)
+                self.sourceFont.paste()
+                self.sourceFont[inserter].glyphname = "monospaced"
+                inserter += 1
 
         # end for
 

[necessary129]

Hi. Why is this not merged yet?

[Finii]

After re-reading the description (topmost comment) it is a bit clumbsily formulated. Will fix that.

But most probably it is not merged because it has heavy implications that need time to evaluate.

AND a decision is needed which of the 3 font types we want to generate (flavor 1, 2, 3 ?)

Edit: Add 'AND' paragraph

[jirutka]

@Finii, can you please rebase this PR on top of the current master?

[Finii]

Rebased, force push.

[Finii]

Rebase on master, force push.

Thanks to @saumyajyoti
#898 (comment) etc

[saumyajyoti]

Won't we need to update https://github.com/ryanoasis/nerd-fonts/blob/master/readme.md ?

[Finii]

Won't we need to update https://github.com/ryanoasis/nerd-fonts/blob/master/readme.md ?

Yes! I will add it.

[Finii]

Last 2 commits:
8f8776c
f67de4d

Incorporated part of #780 into this PR, because it is also needed to make the font correctly monospaced.

[Finii]

Thanks to everyone who helped sorting out the actual problem(s) and solving this irritating issue :-)
As this is mainly a regression, it will go into master now instead into 2.3.0, I guess :-}