search

adam7 / delugia-code

Can we add Nerd Fonts to Cascadia Code Font using a GitHub Action
MIT License
567 stars 34 forks source link

Bugfix/fix font logos #54

Closed Finii closed 3 years ago

Finii commented 3 years ago

In the Book face a lot of the icon glyphs look too far up. Compare #53

The reason is that font-patcher vertically centers most of the added glyphs. This can not be changed without patching font-patcher.

This MR changes the output fonts so, that the glyphs added from font logos and the git textual glyph from FontAwesome is not vertically centered anymore but is kept on the baseline. There are several pictures showing the effect in the aforementioned issue.

The downside is, that the symbols are not centered anymore in the middle of the caps-height but rather stay in the x-height. There might be people that do not like the effect:

image

It is possible to change that behavior only for the book face; but that is not foreseen in font-patcher, so that we end up with a more invasive fix. But maybe that is the way to handle it.

Are there any opinions? What about the other (untouched by the MR) symbol glyphs in the book face? Are they all defective? That would call for another action (disable all vertical alignment if a proportional font is the patch target).

Finii commented 3 years ago

Changed this PR such that

Here before, complete: completeold

before, book (note broken weather symbol top): boolold

After this PR, complete (unchanged): completenew

After this PR, book: booknew

Mellbourn commented 3 years ago

Will there be a new release? (I have a script that automatically downloads the latest release from the https://github.com/adam7/delugia-code/releases page)

  curl -sL github.com/adam7/delugia-code/releases/latest |
    grep "Delugia.Nerd.Font.Book.ttf\"" |
    head -1 |
    awk -F "\"" '{print "https://github.com" $2}' |
    wget --show-progress -qNi -
Vovcharaa commented 3 years ago

This fixed ubuntu icon image But it is still slightly higher in complete then other Nerd Fonts For example what it looks like in Sauce Code Pro Semibold Nerd Font Complete Mono Windows Compatible: image Is it correct behavior?

Mellbourn commented 3 years ago

I would really prefer that the Complete font was updated too. The width is different in Book.

Finii commented 3 years ago

I would really prefer that the Complete font was updated too. The width is different in Book.

Could you share an image of where you see it (possibly with the unicodes also given, or copy and paste the text itself). What be nice to have some capital letters in the image too, unlike Nazar above, it's hard to interpret otherwise. Do you use Complete or Complete Mono?

Finii commented 3 years ago

@Vovcharaa Well, have a look at the next image: image Top shows the Caskaydia Cove Regular Nerd Font Complete Mono directly from Nerd Fonts. Bottom shows Cascadia Mono PL directly from MS.

Clearly the vertical alignment of font-patcher (by Nerd Fonts, the same script we use) is wrong - too high. The Powerline triangular things are too high, also all other stuff that is aligned to match them.

So we align it the same way Nerd Font does (which depends on the source font, thus Sauce Code looks different).

But, if I remember correctly, we intended to use Microsofts PL variant and use their PL symbols, which is obviously not the case. Will investigate.

Finii commented 3 years ago

we intended to use Microsofts PL variant and use their PL symbols

Ah yes! We do. This is why our Powerline Triangular Stuff is right. But still the other icons we add are too high, as does Nerd Fonts.

Same as above but with Delugia Nerd Font Complete:

image

(All ubuntu logos are unicode F31B)

Mellbourn commented 3 years ago

I would really prefer that the Complete font was updated too. The width is different in Book.

Could you share an image of where you see it (possibly with the unicodes also given, or copy and paste the text itself). What be nice to have some capital letters in the image too, unlike Nazar above, it's hard to interpret otherwise. Do you use Complete or Complete Mono?

I may be confused. Now I can't reproduce any problems with the Book font that I download from the latest release, except that it is too high.

Finii commented 3 years ago

Created a new pull request that is not so invasive in the usual Nerd Font way, that people like @Vovcharaa expect. I would rather drop this PR, if there are no strong arguments for it. The new PR #55 could have a higher change to be merged into Nerd Fonts.

@Mellbourn Testing fonts is hard. I usually clear the font cache and restart the terminal application completely. Sometimes one or the other seems to be not needed, but not consistently.