nerd-fonts icon indicating copy to clipboard operation
nerd-fonts copied to clipboard

Fira code glyphs size reduced

Open fmir864 opened this issue 3 years ago • 33 comments

🎯 Subject of the issue

The glyph size of Fira Code patched font is reduced

🔧 Your Setup

  • _Which font are you using? Fira Code Regular Nerd Font Complete
  • _Which terminal emulator are you using? Windows Terminal v1.11.3471.0
  • _Are you using OS X, Linux or Windows? And which specific version or distribution? Windows 10 20H2

★ Screenshots (Optional)

fira

fmir864 avatar Dec 21 '21 14:12 fmir864

You mean the symbols versus the letters comparing 5.2 with 6.x? What are these version numbers? (5.2, 6.1, 6.2)? Can you give the codes of the symbols (or copy and paste something into a comment window)? Did you use patched fonts from this repo or self-patched?

Finii avatar Dec 21 '21 16:12 Finii

(I ask specifically because at the moment I work on the glyph rescaling ;)

I guess 5.2 and 6.x are Fira Code version numbers. So you self-patch? I will download 5.2 and 6.2 from https://github.com/tonsky/FiraCode/releases and try to reproduce.

Finii avatar Dec 21 '21 16:12 Finii

$ fontforge --script ~/git/nerd-fonts/font-patcher --complete Fira_Code_v5.2/ttf/FiraCode-Regular.ttf
$ mv 'Fira Code Regular Nerd Font Complete.ttf' 'Fira Code Regular Nerd Font Complete 5.2.ttf'
$ fontforge --script ~/git/nerd-fonts/font-patcher --complete Fira_Code_v6.2/ttf/FiraCode-Regular.ttf
$ mv 'Fira Code Regular Nerd Font Complete.ttf' 'Fira Code Regular Nerd Font Complete 6.2.ttf'
$ fontforge Fira\ Code\ Regular\ Nerd\ Font\ Complete\ *ttf

image

(font-patcher is HEAD).

Windows is the only glyph I could find. Hope it is the correct Windows.

So to proceed we need the information I asked above.

(My hunch is that 5.2 is Nerd Font non-mono while 6.x is Nerd Font Mono, but you specifically say you use the non-Mono version *ponder* Maybe give exact file name.)

Finii avatar Dec 21 '21 16:12 Finii

@Finii Sorry been busy with a release today.

You mean the symbols versus the letters comparing 5.2 with 6.x? Yes, the symbols are too small What are these version numbers? (5.2, 6.1, 6.2)? Fira Code font versions Can you give the codes of the symbols (or copy and paste something into a comment window)? /uf936 radar symbol for example Did you use patched fonts from this repo or self-patched? this repo, I self patched also had the small issue. But now I use the fonts from this repo.

fmir864 avatar Dec 22 '21 12:12 fmir864

I used the font here in this repo, Fira Code Regular Nerd Font Complete.ttf file. I did self patched but went with this repo thinking I might done something wrong.

fmir864 avatar Dec 22 '21 12:12 fmir864

I'm a noob, thought this might help you

f1 f2

fmir864 avatar Dec 22 '21 13:12 fmir864

Another thing I noticed is in Windows Terminal Settings font drop down v5.2 FiraCode NF appears directly but the v6.2 FiraCode NF doen't show unless I select Show all fonts.

fmir864 avatar Dec 22 '21 13:12 fmir864

Same issue present in Caskaydia Cove latest also. Older one looks fine.

fmir864 avatar Dec 27 '21 14:12 fmir864

Hmm, checked Fira Code.

The font in the repo I can not even use with 'all fonts'.

But when I patch Fira anew with all on HEAD with

image

and install that font on Windows, and use it like this

image

(note: 'all fonts' checked)

I get this:

image

Which looks ok for me.

But you are right, the font in the repo is not working. But it will be fixed (at least to some degree) if the all fonts are updated with the current font-patcher.

Finii avatar Dec 27 '21 18:12 Finii

Ah, with #732 even the 'all fonts' is not needed ...

image

we see

image

So I deem this is already fixed but not updated.

If you tell me which font exactly you need I can prepare it for you. Will close now and hope for an update ;) Reopen if you have any objections.

Mind, that because of the naming bug (will be fixed with #717 eventually) you can not install mono and non-mono in parallel :unamused:

Edit: Change the 'question' to bold

Finii avatar Dec 27 '21 19:12 Finii

@Finii I'm currently on Caskaydia Cove Semilight complete. If I can have it it'll be great. Thanks for your time and effort mate, really appreciated.

fmir864 avatar Dec 28 '21 07:12 fmir864

Please try these fonts, they have all the pending fixes to font-patcher...

https://github.com/Finii/nerd-fonts/tree/feature/cascadia-2111.01/patched-fonts/CascadiaCode

Finii avatar Dec 28 '21 16:12 Finii

@Finii I downloaded Caskaydia Cove Nerd Font Complete Windows Compatible SemiLight.otf, the radar glyph looks okay but following are still same;  - e70c  - f296

image

Also I need to select all fonts to choose this font. And it looks tall compared to Cascadia Code Semilight.

image

fmir864 avatar Dec 29 '21 07:12 fmir864

Ok, thanks for the feedback.

Finii avatar Dec 29 '21 10:12 Finii

Oops, I left the old patched fonts in... but I reckon you choose the correct one (where 'Nerd Font' is before 'Semilight').

Regarding 'looks tall': Do you use the static Cascadia Code, or the variable font. There are some differences between the two. We base on the static Cascadia Cove. And then I'm not sure if you can really select the static Cascadia Cove, because Windows Terminal comes bundled with Cascadia IIRC, and it is a lot work to get rid of the font (or my Windows-fu is not good enough, at least I always struggle with that miserable font bundling and the Terminal app.

Looking into the Caskaydia font, it looks ok: image

And on Windows...

image

:unamused: Used this file (check file size)

-rw-rw-r-- 1 fini fini 2606228 Dez 29 12:33 'patched-fonts/CascadiaCode/SemiLight/complete/Caskaydia Cove Nerd Font Complete SemiLight.otf'

Finii avatar Dec 29 '21 12:12 Finii

Hmm, Terminal seems to scale the glyphs either to 1 or 2 'widths': image

This visual logo thing is only slightly wider than 'normal' (i.e. 1440 vs 1200), so scaling down is maybe understandable. But why is that fox thing (I know it, but what is it :->) scaled down...

And then I realized, while looking into font-patcher, that the symbols are only scaled when --mono is given. Need to find out if that was always the case. That is the reason 'visual' is so small, it's just not scaled up.

Finii avatar Dec 29 '21 12:12 Finii

tig blame font-patcher:

image

I did not change anything relevant here :sweat_smile:

All the scaling in only done when --mono (green line, single is then set) and there is even a comment that something could be done to the non-mono fonts for double width glyphs, but nothing implemented.

So the only question for me is... why is fox-thing scaled down.

Same font file works on non-monospaced terminals:

image

Finii avatar Dec 29 '21 12:12 Finii

So you think it's something to do with Windows Terminal misbehaving... Regarding the font I used is static or variable, I don't have a clue. 🦊 feels embarrassed.. 😂

Edit: Found this issue in terminal but can't really understand.

https://github.com/microsoft/terminal/issues/5095

Edit again: I'll try my pwsh away from Windows Terminal and see if the glyphs are sized correctly.

Edited again: if it's terminal, why old version of fonts sized correctly? 🤔 Fira and Caskaydia both old version sized okay.

fmir864 avatar Dec 29 '21 13:12 fmir864

I opened the pwsh in Windows Terminal and VS Code integrated terminal and found out VS Code integrated terminal works fine;

f4

So terminal doing something wrong. But I can't get head around how old font works. 🤔

fmir864 avatar Dec 29 '21 17:12 fmir864

Terminal is not misbehaving.

The problem is that from a terminal people expect that the text is on a grid, like 80 chars wide and 24 rows. Each char has one allotted space (one 'monospace' as they are all the same). So iii has the same width as XXX:

iii XXX

iii
XXX

If one uses a Nerd Font Mono variant, this is what is in the font. The problem is that the box that the letters need to fit into is usually rather thin and tall. For a round icon symbol that is not ideal (?) because it has to be scaled way down.... This is why there are 'non-mono' Nerd Fonts. Here the glyphs are not scaled down.

But they are wider than one space... How is an application expected to handle that? There are several possibilities:

  • Scale individual too-big glyphs down so they fit (seems Terminal does this)
  • Draw the glyphs with their size but advance only one space (so the next glyph overlaps) (my terminal does this)
  • Cut off the right part of a glyph that violates the allotted slot
  • Other ideas...

So what Terminal does is rather reasonable. It even 'warns' you, it lists only 'monospaced' fonts unless you tick 'all fonts'; then possibly problematic fonts can be selected ;)

It also seems that Terminal sometimes at least detects that glyphs are too wide and then puts them into an artifical two-spaces-wide slot. This happenes with the radar glyph. And that raises the question why radar is detected as double width glyph and Foxy is not.

I never test(ed) Terminal on Windows, just for the sole reason that it bundles Cascadia with it and that I'm too stupid to remove the font effortlessly. I want to be in control over the fonts that are installed and Terminal takes that from me.

So for me the question is still: Why does Terminal use a double width glyph for radar and not for Foxy? I will do some research quickly, and compare with the 'old' patched fonts.

Finii avatar Dec 30 '21 07:12 Finii

Ah.

Terminal has a function that determines the width of the glyph.

Codepoint Width Detector https://github.com/microsoft/terminal/blob/main/src/types/CodepointWidthDetector.cpp

Radar icon is U_F936 ... according to table Wide And the Foxy is U_F296... Ambibuous

        UnicodeRange{ 0xe000, 0xf8ff, CodepointWidth::Ambiguous },
        UnicodeRange{ 0xf900, 0xfaff, CodepointWidth::Wide },

Ambiguous means that the font has to be 'asked'... This is how the ask-function looks like:

image

*cough*

https://github.com/microsoft/terminal/issues/2066

Okay. That leaves the question why old fonts work? Checking.

Finii avatar Dec 30 '21 08:12 Finii

Hmm, the 5.2 font, where/how did you get that? I'm too stupid :-D

image

Finii avatar Dec 30 '21 09:12 Finii

Maybe here: 978f3b1b2d, trying

Finii avatar Dec 30 '21 10:12 Finii

Can replicate...

image

Finii avatar Dec 30 '21 11:12 Finii

When I use font-patcher that was current in 978f3b1, and run in on HEAD, I get the same (i.e. working) font as the 5.2 version, although this is just the script from back then, the source and glyph fonts are current.

Examining the script...

It is this commit 59c45ba that breaks it:

Remove negative bearings on 2048-em glyphs

Bisected an overlap issue in status bars to
https://github.com/ryanoasis/nerd-fonts/pull/283/files#diff-3b192ccaec850d73e6540b7eddd8b50cL710-R734

#283

Hmm.

Here is the relevant sub-part of the commit:

--- ../font-patcher-5.2-B7	2021-12-30 12:46:58.915031689 +0100
+++ font-patcher	2021-12-30 12:49:32.063374743 +0100
@@ -807,17 +807,16 @@
             align_matrix = psMat.translate(x_align_distance, y_align_distance)
             self.sourceFont[currentSourceFontGlyph].transform(align_matrix)
 
-            # Ensure after horizontal adjustments and centering that the glyph
-            # does not overlap the bearings (edges)
-            self.remove_glyph_neg_bearings(self.sourceFont[currentSourceFontGlyph])
-
             # 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
             # same width for all character glyphs. This needs to be done for all glyphs,
             # 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])
 
+            # Ensure after horizontal adjustments and centering that the glyph
+            # does not overlap the bearings (edges)
+            self.remove_glyph_neg_bearings(self.sourceFont[currentSourceFontGlyph])
+
         # end for
 
         if self.args.quiet is False or self.args.progressbars:

Note that the explicit statement that it shall come after the bearings correction is ignored and removed :grimacing:

Finii avatar Dec 30 '21 12:12 Finii

This is what the code is about:

image (Image taken from https://freetype.org/freetype2/docs/glyphs/glyphs-3.html)

We want the advance width to be the same for all glyphs. :heavy_check_mark: We want the bearings to be non-negative. :question:

The bounding-box width and the three values above always (have to) sum up. bbox-width = advance-width - left-bearing - right-bearing The statements above contradict each other. We can not have one-uniform-advance-width and zero-bearings and glyphs that are wider than one 'monospace slot' (i.e. advance width).

We do not change the bounding box, because ... we do not touch the glyph itself. If we change one of the values at least one other values has to be adapted - is automagically adapted.

We start with a glyph that is wider than one advance width. The width of the glyph is given by its bounding box, which is unchanged). In the new code we:

  1. Set the advance width to our monospace width -> at least one bearing MUST become negative
  2. Set negative bearings to zero -> advance width must increase

That is wrong. I guess that is the reason for the very strange looking Ubuntu font, for this issue, for others possibly.

Need to dig deeper WHY this has been changed.

Finii avatar Dec 30 '21 13:12 Finii

Maybe here: 978f3b1b2d, trying

That's the place

fmir864 avatar Dec 30 '21 14:12 fmir864

Okay, the reason for the change is most likely this (I even stated it above)

Bisected an overlap issue in status bars to ...

Before that commit (59c45ba4) we had

  • all glyphs (regardless of --mono or not) had the same uniform advance width
  • all left bearings were zero
  • the right bearing were whatever needed to make all values to sum up to the bounding-box-width

After that commit we have now

  • all glyphs have zero or positive bearings (i.e. are smaller or equal wide that the associated advance width)
  • have different advance width: maximum of ( our uniform width and bounding-box width )

The reason for the overlap issue (that I did not find (search for)) is most likely as follows.

People have a non-strictly-monospaced application like writer. You type a symbol like Foxy. The glyph is drawn to its full width, but the cursor only advances one advance width (of course). The next character you type in will OVERLAP (of course). The same behavior is in the terminal I use (Tilix):

image

image

Note the X overlapping Foxy. Or in writer with two colours for clarity:

image

Someone will have complained.

With the change the font is not monospaced anymore (i.e. has not equal advance width for all stuff), but it renders more nicely in proportional font aware contexts:

image

Note that the X on both lines do not line up anymore vertically (because there is this 1 1/2 wide glyph in the upper line).

Tilix on the other hand ignores it and looks the same as above, it forces all glyphs to be the same width.

Windows Terminal handles it in another, different way etc.

Finii avatar Dec 30 '21 14:12 Finii

This is the current code:

              align_matrix = psMat.translate(x_align_distance, y_align_distance)
              self.sourceFont[currentSourceFontGlyph].transform(align_matrix)
  
              # 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
              # same width for all character glyphs. This needs to be done for all glyphs,
              # even the ones that are empty and didn't go through the scaling operations.
              self.set_glyph_width_mono(self.sourceFont[currentSourceFontGlyph])
  
              # Ensure after horizontal adjustments and centering that the glyph
              # does not overlap the bearings (edges)
              self.remove_glyph_neg_bearings(self.sourceFont[currentSourceFontGlyph])
  
          # end for

First set_width_mono and then remove_negative_bearings. The later call (bearing) undos in most cases the former (width); only for very small icons it has an effect.

The question is what we want.

At the moment there are two variants possible

  • Nerd Font Mono: Strictly monospaced, symbols scaled down
  • Nerd Font: Not really monospaced, symbols can be bigger than one slot

But what some people would like to have is

  • Nerd Font MonoBigIcons: Strictly monospaced, symbols can be bigger than one slot

This analysis just points out the facts. I have no clue what 'the users' want. I personally always used Mono fonts; I can not stand icons that are bigger than one slot :grimacing:

So typically, if you are in a monospaced environment (like Terminal) you want a strictly monospaced font (i.e. Nerd Font Mono). That font will be selectable without 'all fonts' ticked.

But then, I have seen many issues regarding too small icons and ppl seem to expect big icons.

I think this is the point where @ryanoasis needs to say something :->

Maybe at some point one can drop the 'for Windows' variants, and instead add the third alternative from above. At the moment it is not possible to achieve that even with self-patching with option.

Finii avatar Dec 30 '21 15:12 Finii

This seems to be the original Issue report https://bugs.archlinux.org/task/66212

and it is exactly this... people who expect icon-adwance-width to be the full bbox-width. Which is contrary to people who expect monospaced stuff.

Finii avatar Dec 30 '21 18:12 Finii