Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fira code glyphs size reduced #731

Closed
fmir864 opened this issue Dec 21, 2021 · 34 comments
Closed

Fira code glyphs size reduced #731

fmir864 opened this issue Dec 21, 2021 · 34 comments

Comments

@fmir864
Copy link

fmir864 commented Dec 21, 2021

🎯 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

@Finii
Copy link
Collaborator

Finii commented Dec 21, 2021

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
Copy link
Collaborator

Finii commented Dec 21, 2021

(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
Copy link
Collaborator

Finii commented Dec 21, 2021

$ 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
Copy link
Author

fmir864 commented Dec 22, 2021

@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
Copy link
Author

fmir864 commented Dec 22, 2021

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
Copy link
Author

fmir864 commented Dec 22, 2021

I'm a noob, thought this might help you

f1
f2

@fmir864
Copy link
Author

fmir864 commented Dec 22, 2021

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
Copy link
Author

fmir864 commented Dec 27, 2021

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

@Finii
Copy link
Collaborator

Finii commented Dec 27, 2021

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
Copy link
Collaborator

Finii commented Dec 27, 2021

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 😒

Edit: Change the 'question' to bold

@Finii Finii closed this as completed Dec 27, 2021
@fmir864
Copy link
Author

fmir864 commented Dec 28, 2021

@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
Copy link
Collaborator

Finii commented Dec 28, 2021

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 Finii mentioned this issue Dec 28, 2021
5 tasks
@fmir864
Copy link
Author

fmir864 commented Dec 29, 2021

@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
Copy link
Collaborator

Finii commented Dec 29, 2021

Ok, thanks for the feedback.

@Finii
Copy link
Collaborator

Finii commented Dec 29, 2021

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

😒
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 Finii reopened this Dec 29, 2021
@Finii
Copy link
Collaborator

Finii commented Dec 29, 2021

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
Copy link
Collaborator

Finii commented Dec 29, 2021

tig blame font-patcher:

image

I did not change anything relevant here 😅

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
Copy link
Author

fmir864 commented Dec 29, 2021

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.

microsoft/terminal#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
Copy link
Author

fmir864 commented Dec 29, 2021

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
Copy link
Collaborator

Finii commented Dec 30, 2021

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
Copy link
Collaborator

Finii commented Dec 30, 2021

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*

microsoft/terminal#2066

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

@Finii
Copy link
Collaborator

Finii commented Dec 30, 2021

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

image

@Finii
Copy link
Collaborator

Finii commented Dec 30, 2021

Maybe here: 978f3b1, trying

@Finii
Copy link
Collaborator

Finii commented Dec 30, 2021

Can replicate...

image

@Finii
Copy link
Collaborator

Finii commented Dec 30, 2021

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 😬

@Finii
Copy link
Collaborator

Finii commented Dec 30, 2021

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. ✔️
We want the bearings to be non-negative. ❓

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
Copy link
Author

fmir864 commented Dec 30, 2021

Maybe here: 978f3b1, trying

That's the place

@Finii
Copy link
Collaborator

Finii commented Dec 30, 2021

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 (59c45ba) 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
Copy link
Collaborator

Finii commented Dec 30, 2021

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 😬

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
Copy link
Collaborator

Finii commented Dec 30, 2021

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
Copy link

xsrvmy commented Jan 2, 2022

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
Copy link

xsrvmy commented Jan 2, 2022

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
Copy link

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 neovide/neovide#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.

@Finii Finii mentioned this issue Feb 23, 2022
3 tasks
@Finii Finii mentioned this issue Apr 25, 2022
4 tasks
Finii added a commit that referenced this issue Sep 1, 2022
[why]
The commit
  59c45ba  Remove negative bearings on 2048-em glyphs
has been introduced to fix some problems with the symbols only font, at
least from the commit message.

That font is intended to be used in font-fallback situations, and so we
do not know the advance width of the current font anyhow. It does not
make sense to enforce an advance width with these.

[how]
Create the NerdFontsSymbolsOnly as if 59c45ba would be still in
effect, i.e. with variable advance width.

[note]
There have been a lot discussions about the reverted commit, some can be
found here:
* #900
* #764
* #731

Signed-off-by: Fini Jastrow <[email protected]>
Finii added a commit that referenced this issue Sep 1, 2022
[why]
The commit
  59c45ba  Remove negative bearings on 2048-em glyphs
has been introduced to fix some problems with the symbols only font, at
least from the commit message.

That font is intended to be used in font-fallback situations, and so we
do not know the advance width of the current font anyhow. It does not
make sense to enforce an advance width with these.

[how]
Create the NerdFontsSymbolsOnly as if 59c45ba would be still in
effect, i.e. with variable advance width.

[note]
There have been a lot discussions about the reverted commit, some can be
found here:
* #900
* #764
* #731

Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii closed this as completed Jan 13, 2023
@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants