adam7 / delugia-code

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

Nerd font icons too small / not centered in "complete" #53

Closed charlesroper closed 3 years ago

charlesroper commented 3 years ago

First let me say what an brilliant project - so useful! I was previously using the Nerd Font version of Cascadia and it was getting pretty old (2007.01 I think, which is from June last year and has been updated several times since).

The issue I want to report is that the icons appear too small in Delugia Nerd Font Complete. I almost ditched it completely to go back to the NF version, but thought I'd check for closed issues first. I found #31 which gave me the clue to try the Book version. This looks fine although icons sit a bit high perhaps - could do with aligning to center line rather than top?

Perhaps mention use of book for more usefully sized icons in the installation instructions?

Screenshots:

delugia-complete

delugia-book

Icons too high:

delugia-book-high

Visually adjusted manually - this is about where I think they should be:

delugia-book-center

charlesroper commented 3 years ago

Perhaps mention use of book for more usefully sized icons in the installation instructions?

More useful, that is, when used for command line theming that supports icons (such as Oh my Posh).

Not so useful in text editors 🙂

Mellbourn commented 3 years ago

I hope this gets looked at. To be honest, the Complete font is not usable in its current state. But I definitely agree that this is a brilliant project. So much fresher than the standard Nerd Fonts.

Finii commented 3 years ago

Is this not a duplicate of #31?

Did you try what is described in https://github.com/adam7/delugia-code/issues/31#issuecomment-586984820

Thanks, Fini

Finii commented 3 years ago

Sorry you mention that in your original post :->

I will have a look tomorrow.

Mellbourn commented 3 years ago

I've read https://github.com/adam7/delugia-code/issues/31 more carefully now, and realized that some may prefer the very small, single character icons (I don't). So I guess the main problem is that the icons of Book are above center. This can be seen if you compare Book with the font you get if you brew install font-caskaydia-cove-nerd-font

charlesroper commented 3 years ago

@Finii My thinking is that the book version should be mentioned in the installation instructions, especially if the intention is to use the font for CLI theming.

In forcing the icons into a mono box, it is making them too small to be useful for the CLI.

The book font’s larger but non-mono icons are much more useful for CLI theming, even though you do have to add in an extra space in places to prevent the icon overlapping with the character to its right.

Aside from that docs issue (happy to write something if it would help?), the other problem is that the book font icons ride too high as I’ve tried to illustrate in my screenshots. I don’t know enough about font editing to know what the solution is there. I ended up switching over the the JetBrains NF which doesn’t have the same problem - icons are nicely centered.

Finii commented 3 years ago

@charlesroper If you can come up with any concrete change of the README, I would be glad to insert it (or create a PR).

I will have a look into JetBrains NF, useful hint, tnx.

Ah, I work on the CLI (zsh) all day long, and for me the book face is not useful at all, because I use my editor on the command line / in the command line window (neovim, btw ;). But then, I do not use any of the glyphs with problems in your example (like, what is that ring? Ubuntu? and the 'git' glyph). In fact I do not even use the 'complete' version of Delugia, that might be the reason problems with other glyphs are not noticed by me.

Untitled

Lets see what we have... E73A ('uniE63A' of devicons) 23FC ('power on-off symbol' of Power Sybols) 0041 ('A' of Cascadia Code) F31B ('ubuntu' of font logos) 0041 ('A' of Cascadia Code) F31C ('ubuntu-inverse' of font logos)

image

Finii commented 3 years ago

Here are the glyph sources: image

And here after scaling: image

Let me think a bit about the scaling ...

Finii commented 3 years ago

Okay, here we have (from left to right): ordinary A - ubuntu now - ubuntu new - right bracket

image

The 'ring' is still a bit taller than the capital heights, but it rides on the baseline. Maybe this is a good compromise. I just changed the vertical alignment algorithm, scaling is untouched.

The vertical alignment is automatically done in font-patcher and I see no easy way (i.e. command line switch) to change that. The change in font-patcher itself is trivial for this (but of course will affect other glyphs as well). Let me stew it a bit in the back of my brain.

Mellbourn commented 3 years ago

Great work!

To clarify: At least some command line tools that use Nerd Fonts seem to assume the double width and take that into account when printing output. In particular https://github.com/Peltoche/lsd does this when listing files.

What is that cool program you are using to edit the glyphs?

Finii commented 3 years ago

It's the same program we use to patch the font ... fontforge on github.

I think I solved this, but to come up with a proper commit I need to clone nerd-fonts which takes ages :grimacing:

Finii commented 3 years ago

Here what font-patcher does with the font-logos glyphs automatic rescaling and automatic alignment... Top row original font, bottom row after insertion into Delugia.

image

Maybe not what ppl would expect. This SUSE animal walked on the baseline :cry:

It is possible to add font-specific scaling rules to font-patcher but the font-logos goes with the defaults.

I now added a rule to font-patcher to handle that font hopefully better.

Finii commented 3 years ago

Ok, here is the possible fix (bottom font):

1

Just scaled up therefore blurry detail:

2

@charlesroper @Mellbourn Please try it out, find here (top right: 'artifacts'). If you think that is it, we can pull it here and make new release.

Important That does not fix the 'git' glyph on the far right of the original post. Please send me the unicode of that glyph, or copy and past it directly into a comment window here. Even if it is not displayed correctly I will be able to extract the code.

Finii commented 3 years ago

Maybe its this.... FontAwesome... left: original, right: after font-patcher

image

Mellbourn commented 3 years ago

I have tested the new artifact. To me both the size and the vertical alignment of Complete looks perfect now!

When I compare it with CaskaydiaCove Nerd Font, Delugia is very slightly wider. Which one is "correct" I can't tell, but they are not identical in width.

I get CaskaydiaCove Nerd Font from brew install font-caskaydia-cove-nerd-font

EDIT: Maybe Delugia is the "right" width, because it seems to be the same as JetBrainsMono Nerd Font (from brew install font-jetbrains-mono-nerd-font)

Finii commented 3 years ago

Small interruption because of tee time here :coffee:

Ok, fixed also the git text thingy:

Top: before, bottom: after

1

Detail:

image

I'll create a PR now and after @charlesroper tested it successfully we can merge.

Finii commented 3 years ago

(Secretly I believe there is a more fundamental problem with font-patcher but due to the number of supported output fonts it is extremely difficult to be certain a bug in one font is not a feature in another.)

Finii commented 3 years ago

difficult to be certain a bug in one font is not a feature in another

Ah... well... in the monospaced fonts, where the icons are scaled down quite a but, the previous uplifting might make sense? Shown here the new monospaced face:

image

Well, for me to have the icons in the x-height (lower-case space) makes sense. Will people complain? Others might like to have the circle centered on the > tip or in the middle of the [?

Please comment in the PR #54

Vovcharaa commented 3 years ago

Sauce Code Pro Semibold Nerd Font Complete Mono Windows Compatible (or any other Nerd Font I tried) image image image Delugia Code from PR #54 image image image Why is this happening?

Finii commented 3 years ago

Well, I thought I explained it already in the PR description. And you are right, and I am just preparing the commit.

I reckon you are not using the 'book' face, right? (Because if you do I need to reconsider.)

Vovcharaa commented 3 years ago

Yes

charlesroper commented 3 years ago

Want me to try this @Finii? Is that run (this one) still the latest or have you updated since then?

Finii commented 3 years ago

At the moment I believe that there is a (fundamental) error in font-patcher's get_sourcefont_dimensions() function.

The problem seems to be that Cascadia uses all the advanced font stuff with scaling of characters by the font engine, and that does not play too well with the assumptions about sizes that font-patcher has. With more 'simple' fonts that is all well.

Please be patient, I'm not that familiar with these features myself and need to do some research.

Finii commented 3 years ago

And Cascadia has a bug (that gets us). The 'capital A ring acute' (unicode 01fA) is too big and bleeds into the line above, here shown with CascadiaMonoPL-Regular:

image

If we take the bounding box of all chars to determine 'the middle' this moves us a bit too high.

And then, the font size information for Windows and Mac differ. I have never seen this in a font:

image

Win Ascent should be equal to HHead Ascent and (minus)Win Descent equal to HHead Descent.

Mellbourn commented 3 years ago

Doesn't that mean that it will be a good thing to lower the glyphs?

Finii commented 3 years ago

From the 705 fonts files in nerd-fonts/src/unpatched-fonts only 44 have different sizes.

WHATEVER.

Finding out why we have the difference goes back to this commit in CascadiaCode: https://github.com/microsoft/cascadia-code/commit/5795d16e36748612b726164fd8b59f3c1c8b0788

If you read the commit message it says now we follow the Google Font design method. Which it does not, because that explicitely states that Win Ascend and HHead Ascend have to be the same :roll_eyes:

Let me try and fix our source font (which we touch anyhow for preparation) and see how our patched fonts look then.

Finii commented 3 years ago

@charlesroper I create yet another PR :roll_eyes: with another approach which I believe is right now. The artifacts are here.

Any reports welcome.

charlesroper commented 3 years ago

It's beautiful - perfect - thank you for your efforts on this! 👍😁

Clipboard02

Finii commented 3 years ago

Without your detailed report this would not have happened :+1: