search

ryanoasis / nerd-fonts

Iconic font aggregator, collection, & patcher. 3,600+ icons, 50+ patched fonts: Hack, Source Code Pro, more. Glyph collections: Font Awesome, Material Design Icons, Octicons, & more
https://NerdFonts.com
Other
54.08k stars 3.63k forks source link

Fira code glyphs size reduced #731

Closed fmir864 closed 1 year ago

fmir864 commented 2 years ago

🎯 Subject of the issue

The glyph size of Fira Code patched font is reduced

πŸ”§ Your Setup

β˜… Screenshots (Optional)

fira

Finii commented 2 years ago

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 commented 2 years ago

(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 commented 2 years ago 
$ 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.)

fmir864 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

I'm a noob, thought this might help you

f1 f2

fmir864 commented 2 years ago

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 commented 2 years ago

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

Finii commented 2 years ago

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 commented 2 years ago

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

fmir864 commented 2 years ago

@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.

Finii commented 2 years ago

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

fmir864 commented 2 years ago

@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

Finii commented 2 years ago

Ok, thanks for the feedback.

Finii commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

fmir864 commented 2 years ago

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 commented 2 years ago

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. πŸ€”

Finii commented 2 years ago

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:

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 commented 2 years ago

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 commented 2 years ago

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

image

Finii commented 2 years ago

Maybe here: 978f3b1b2d, trying

Finii commented 2 years ago

Can replicate...

image

Finii commented 2 years ago

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 commented 2 years ago

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.

fmir864 commented 2 years ago

Maybe here: 978f3b1b2d, trying

That's the place

Finii commented 2 years ago

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

After that commit we have now

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 commented 2 years ago

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

But what some people would like to have is

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 commented 2 years ago

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.

xsrvmy commented 2 years ago

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

I think this depends on the font as well. For example, Iosevka is 8px wide at 12pt (as opposed to 10px like most other fonts), and shrinking an icon that small makes it very hard to read, even on a hidpi screen.

xsrvmy commented 2 years ago

Hmm, Terminal seems to scale the glyphs either to 1 or 2 'widths': ![image](https://user-images.githubusercontent.com

The glyph showing as two-wide is F936 which is a CJK compatibility character, so terminal thinks it's two wide.

fredizzimo commented 2 years ago

I think there might be another problem at play here.

On Windows, the family name is the same for the mono and regular variants. And at least for me in Neovide https://github.com/neovide/neovide/issues/1135, that was the actual problem. And I solved it by just not installing any mono variants.

But the issue is still there in the Windows Terminal, and probably in a lot of other terminal emulators as well.

At least in the Neovim community the double width glyphs are the standard. And almost all plugins adds an extra space after the glyph by default or it's at least configurable. It's usually also configurable in other terminal programs like exa, where the help says

   EXA_ICON_SPACING
       Specifies the number of spaces to print between an icon (see the `--icons' option) and its file name.

       Different  terminals  display  icons  differently,  as they usually take up more than one character width on
       screen, so there’s no β€œstandard” number of spaces that exa can use to separate an icon from text.  One space
       may  place  the icon too close to the text, and two spaces may place it too far away.  So the choice is left
       up to the user to configure depending on their terminal emulator.

So I would say that the issue is a polybar issue, not a Nerd fonts issue, and the changes should be reverted.

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity (i.e. last half year) after it was closed. It helps our maintainers focus on the active issues. If you have found a problem that seems similar, please open a new issue, complete the issue template with all the details necessary to reproduce, and mention this issue as reference.