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

Feature: Allow symbol scaling in nonmono (down to 2 'widths') #748

Merged
merged 16 commits into from
Jan 12, 2023

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented Dec 31, 2021

Description

For non-single fonts (i.e. with no --mono) added symbol glyphs will be scaled down to be maximum 2 'slots' wide.

Usually the added Glyphs are about 1.6.. 1.8 the width of the normal 'Letters' we have in the target fonts. But there are tiny/tall fonts, where the added glyphs just look out of place.

Requirements / Checklist

What does this Pull Request (PR) do?

The first commit refactor the code with no effective change in result. This is needed in preparation to the actual intended change that would look rather complicated otherwise.

The actual change in the 2nd commit allows downscaling added glyphs in non-mono fonts, but still observes the ScaleGlyph data.

How should this be manually tested?

Fonts patched with --mono should come out unchanged.
Fonts patched without --mono but higher EM value (i.e. where the added symbols are smaller than 2 times the normal advance width) should come out unchanged.
Fonts patched without --mono but small EM shall have symbol glyphs not wider than two normal advance widths.

Any background context you can provide?

What are the relevant tickets (if any)?

Screenshots (if appropriate or helpful)

@Finii
Copy link
Collaborator Author

Finii commented Dec 31, 2021

There is still an issue, but I promised to create the PR this year ;-)

Please ignore until this is fixed.

It is regarding use of EM vs height for scaling. There is even this comment in the code:

        # font_dim['height'] represents total line height, keep our symbols sized based upon font's em
        # NOTE: is this comment correct? font_dim['height'] isn't used here

Unfortunately there is no explanation given in the commit that adds that. Before the commit height has been used.
Need some time to research, and now I have to celebrate ;-)

Happy New Year 🎉

 Fini

@Finii
Copy link
Collaborator Author

Finii commented Dec 31, 2021

Note to self: I believe height is correct, as is done again afterwards in the copy_glyphs() code... Strangeness.

@Finii Finii changed the title Feature: Allow symbol downscaling in nonmono (down to 2 'wdiths') Feature: Allow symbol downscaling in nonmono (down to 2 'widths') Dec 31, 2021
@Finii
Copy link
Collaborator Author

Finii commented Jan 1, 2022

Pulled my problem out into a new PR #749. I think it is important enough to have its own discussion etc pp

font-patcher Outdated
if scale_ratio_x > scale_ratio_y:
scale_ratio = scale_ratio_y
# Use the font_dim['height'] only for explicit 'y' scaling (not 'pa')
target_height = self.sourceFont.em if stretch == 'pa' else self.font_dim['height']
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If #749 is pulled, this would result in this change here:

-# Use the font_dim['height'] only for explicit 'y' scaling (not 'pa')
-target_height = self.sourceFont.em if stretch == 'pa' else self.font_dim['height']
+target_height = self.font_dim['height']

Which I think should be done.

@Finii
Copy link
Collaborator Author

Finii commented Jan 7, 2022

Ready for merge, but I would change, as detailed above:

-# Use the font_dim['height'] only for explicit 'y' scaling (not 'pa')
-target_height = self.sourceFont.em if stretch == 'pa' else self.font_dim['height']
+target_height = self.font_dim['height']

@Finii
Copy link
Collaborator Author

Finii commented Feb 25, 2022

Just a thought... Maybe the scaling should not be fixed to '2 widths', but take into account the unicode slot expected width like Windows Terminal here:

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

But well, this PR is at least a beginning to make sure the glyphs are maximum 2 widths wide and not like ... 3.
Some should be even smaller (i.e. only 1) according to this.

But that would leave us with yet another 'font flavor', "Nerd Font UnicodeMono" ;-D

@jirutka
Copy link

jirutka commented Jul 24, 2022

@Finii, can you please rebase this PR on top of the current master?

@Finii Finii force-pushed the feature/allow-downscaling-nonmono branch from c959861 to 579ed95 Compare July 26, 2022 13:33
@Finii
Copy link
Collaborator Author

Finii commented Jul 26, 2022

Rebase on master, force push.

This was a bit harder, because the scale glyph mechanics changed in master to be more powerful. On the other hand this code explicitly does not set out to handle scale glyph stuff.

I did only few tests on the code / patch result, because I'm in a hurry (vacation starts today evening and I'm packing in parallel). But I still believe it is correctly rebased. Take with a grain of salt and report back :-)

@Finii Finii mentioned this pull request Aug 22, 2022
3 tasks
@Finii
Copy link
Collaborator Author

Finii commented Aug 22, 2022

Would fix #888

Well, the bottom part which is not directly the issue itself.

@Finii Finii force-pushed the feature/allow-downscaling-nonmono branch from 579ed95 to bbdff59 Compare August 22, 2022 15:31
@Finii
Copy link
Collaborator Author

Finii commented Aug 22, 2022

Rebase on master (trivial), force push

@Finii Finii added this to the v2.3.0 milestone Aug 22, 2022
@lethefrost
Copy link

lethefrost commented Sep 5, 2022

Hi @Finii , I am wondering if exact multi-spaced widths would be possible. For example, if one spacing unit is 1024, then make all narrower glyphs have width of 1024; and all wider ones have 2048, 3072, or 4096, by rounding up the width to the closest multiple of the unit width, and centering the glyphs without scaling them? It would be very helpful if we want to patch some different fonts or icons together but still want to keep the codes aligned in editor using the patched font. I have scant knowledge about fonts. I would really appreciate if you could make this work or point out any tool/library/document that could help to me. Thank you so much!🥺🥺

@Finii
Copy link
Collaborator Author

Finii commented Sep 5, 2022

I'm not sure if you seek help for your own project or want to propose a Nerd Font change.

Of course it would be possible to have (in your example) 3072 or 4096 glyphs in a font that has a basic width of 1024.
Typically Nerd Fonts are used in a 'monospaced' environment like terminal emulators. Some widely used terminal emulators do not allow glyphs to be wider than '2 cells' - that is the reason for the PR. Those terminal emulators scale the glyphs down to fit into 2 cells, and usually not very nicely.

The second thing: 'Typically' wider than one cell glyphs are left-aligned. You propose center aligned. I guess people would not like that, as their setup will look different.

Maybe you can tell me more what you have in mind, I'm just fishing in the dark. Sorry.

@Finii Finii force-pushed the feature/allow-downscaling-nonmono branch from bbdff59 to e96e1dd Compare September 7, 2022 08:09
@Finii
Copy link
Collaborator Author

Finii commented Sep 7, 2022

Rebase on master, force push

@lethefrost
Copy link

I'm not sure if you seek help for your own project or want to propose a Nerd Font change.

Of course it would be possible to have (in your example) 3072 or 4096 glyphs in a font that has a basic width of 1024.
Typically Nerd Fonts are used in a 'monospaced' environment like terminal emulators. Some widely used terminal emulators do not allow glyphs to be wider than '2 cells' - that is the reason for the PR. Those terminal emulators scale the glyphs down to fit into 2 cells, and usually not very nicely.

The second thing: 'Typically' wider than one cell glyphs are left-aligned. You propose center aligned. I guess people would not like that, as their setup will look different.

Maybe you can tell me more what you have in mind, I'm just fishing in the dark. Sorry.

Hi @Finii , first I want to thank you soooo much for replying to me that quickly! It's very nice of you to help me out here with my stupid questions. Sorry for not checking notifications these days🥺

Thank you for the explanation! It helps to clarify some of my confusion. I was not so familiar with font designing. I previously thought most common glyphs are center aligned, because I had seen many of this type, like many symbols and characters in latin or cjk charsets. Thank you for pointing it out, letting me have the chance to correct my impression. I now notice that there are also many, maybe more of them left aligned.

What I was trying to do was just to completely 'monospacify' a font, by not allowing any glyph width to be not an integer multiple of the basic unit, say, a cell, or in this case 1024. To set every glyph to have width as exact multiple of 1024, I wanted to round it up. For example, make some glyph of original width 1000 to 1024, 1800 to 2048. Since the width is changed without scaling, I was worried about the aligning problem. That's why I was talking about center aligned (but I had no idea of the effect).

You are absolutely right! I indeed found that my terminal (iTerm2) does not allow characters to occupy wider than 2 cells spaces. It's weird, because even I cannot remember exactly, I sometimes do find fonts containing wider glyphs... just like the ones in issue #747 . But anyway, the 3072 and 7096 I mentioned before are just meant to form a series of integer multiples of 1024, which has no practical significance, supposing there are no glyphs actually taking more than 2 cells.

I asked this question based on personal needs. I guess it seems that no one else has such a need at present, so it may not be a good idea to request for this feature in this project. And you probably won't be interested or have the time to do this, so my original intention was to ask you how to do with a font and implement it myself later.

Thank you again for your help! In the past few days of groping, I seem to have come to understand a little bit of what I can do with a font using python fontforge library.. Please let me know if you have more suggestions and comments on the irrationality of my goal. What tools do you recommend or do you usually use to modify and design a font?

@Finii
Copy link
Collaborator Author

Finii commented Sep 8, 2022

What tools do you recommend or do you usually use to modify and design a font?

Well, I do not design fonts. Creating a good design is very very hard, and takes a lot of time. And there are so many good fonts out there already. I'm not shy actually paying for good fonts, as I know how much work it is. My go to source for buying is hide-advertisement

For modifying fonts, there are several alternatives. I use primarily https://fontforge.org, but it also has its quirks and bugs 😬 . Small tools are the pair showttf and ttfdump. For automation of modification one can use the fontforge Python interface https://fontforge.org/docs/scripting/python.html (but some features are missing there, even more than in the GUI version), the excellent https://fonttools.readthedocs.io have usually all the rest you need.

Turning to your goal. monospaced usually means that all glyphs have the same width, not some integer multiple of a given number. So just 1024 in your example. If there are glyphs 2048 wide, the font is NOT monospaced anymore.

This is usually overcome by letting the glyph 'stick out' to one side. Like the small-t in your name. It sticks out into previous 'cells' (which are not cells, but anyhow):

image

Here you see the glyph in detail

image

Ooops, thats the longer version. Anyhow, you see the 'cell'-like rectangular box. You can extend in all directions as much as you like (but some renderers might fail if you extend to much). You even might notice that it also extent into the next glyph (to the right) just a tiny bit.

So, to have a monospaced font, you need an 'advance width' that is the same for each glyph (the center box above), and then you can place what is displayed anywhere. (Well, if the renderer showing the font is capable of showing it).

What terminal emulators are able to render or choose to render is different all over. What you already have seen is that some do not allow glyphs to extend more then 1 advance width more to the right (i.e. '2 cells').

What I want to say is, you need to carefully set your goal, and check if that goal would be supported by any renderer.

The technical side is very simple, on the other hand. This is some python code that opens a font, and modifies the font

import psMat
import fontforge

font = fontforge.open(filename, 1)

for glyph in range(0x21, 0x17f):
    if not glyph in font:
        continue
    bb = font[glyph].boundingBox
    font[glyph].transform(psMat.scale(x_scale, y_scale))
    font[glyph].transform(psMat.translate(x_shift, y_shift))
    font[glyph].width = 1024
    font[glyph].right_side_bearing = ...
    font[glyph].left_side_bearing = ...

font.generate()

The Bounding Box is the total extend in absolute coordinates, you might base your scale on that.
Scale and shift as you like.
Keep in mind, that boundingBox[2] - boundingBox[0] is the total width.
The left, right, and width properties must equal the total width.
Ah, and don't forget to fetch the bb again after scale. There is some rounding in effect.

Hope that helps and gets you going for experiments :-)

Edit: Maybe this is also helpful: #731 (comment) It shows the correct terminology, that I did not use above

@Finii
Copy link
Collaborator Author

Finii commented Sep 8, 2022

What I was trying to do was just to completely 'monospacify' a font

Ah, and wanted to add ... it is almost impossible to take a font designed to be proportionally spaced and press it into monospace. With proportional space fonts the letters like 'W' are often very very wide, while 'i' is slender. When the advance width is the same for both letters, it must be the wider one, probably, the negative space (free white area) between consecutive small letters looks extremely ugly. Imagine 'Wii' looks like Wi i .
The hard work of a font designer is to make monospaced fonts look aesthetically pleasing despite the fact that some letters are naturally wider than other, but that is not allowed to show.

Compare

image

Edit: Change PR link to bullet point

@Finii
Copy link
Collaborator Author

Finii commented Jan 11, 2023

Here for example with Caskaydia Cove, comparison before and after PR:

image

The right outline window is before the PR, the left/bottom one is after the PR. Note that now the clouds fit into a '2 cell wide' space. This is even more dramatic with ProggyClean:

image

Branch icon does not leak anymore into neigboring cell. The cloud with F00B was almost 3000 wide, while the cell is only 896 wide. Now it fits into 2 cells.

Finii added 8 commits January 11, 2023 18:03
[why]
While the left-side-waveform gets 'xy' scaled the right-side gets 'pa'
scaled. This has been obviously forgotten.

[how]
Add specific scale rule for right-side-waveform.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
Obviously we can drop more code and shuffle scaling logic into the
scaling function.

Signed-off-by: Fini Jastrow <[email protected]>
No functional change.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
The overlap formula seems to be off sometimes. Although the shift is
correct (and thus the number of 'pixels' that overlap), but the non
overlapping part of the glyph is often not as wide as expected, off by
up to some percent.

[how]
The formula is too simple. It just calculates an additional scale factor
on top of the already existing factor. To get it 'pixel perfect' we need
to calculate first how much the glyph fills the cell - because we want
the overlap to be in 'cell percent' and not 'glyph percent'. That might
be sometimes the same (if the cell is filled completely), but usually it
is not completely full, and that means the overlap will be smaller than
intended.

[note]
To get the current glyph bounding box we pull some lines up in the code
that get the 'dim' variable.

Also use float constants to calculate with float variables.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
For the non-Mono variants ('Nerd Font' and 'Nerd Font Propo') the
Powerline symbols are scaled in Y but the width is just kept from the
symbol font, whatever that might be (and if it makes any sense).

If you have for example the triangular thing (`E0B0`) it is bigger than
'one cell' and extrudes into the following cell (on 'Nerd Font'). For
the other side (`E0B2`) it is even worse; it is right aligned in the
current cell and so (because it is wider than one cell) it protrudes
into the previous cell.

[how]
Just allow not only Y scaling but also X scaling for non-Mono fonts.

[note]
This is of relevance just for 'xy' scaling, and only the Powerline
symbols do that.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
Most Powerline glyphs have a little bit over overlap to the previous or
next glyph to prevent a 'break' in a colored prompt.

It does not make sense to have overlap with glyphs that can never
produce any of that issues, i.e. glyphs that are not filled to the
border. Like all the line-ish glyphs.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
When we scale all Powerline glyphs also horizontally (in X direction) to
'one cell' some might look a bit too small; especially because they were
very big previoulsy (before commit 'font-patcher: Do x-scale powerline
glyphs').

[how]
To get them to a reasonable and always equal width a new scale code is
introduced: '2'. It is evaluated in 'x' or 'y' scaling contexts and
doubles the target cell width (unless a "Nerd Font Mono" is generated
where all glyphs must be one cell wide).

Signed-off-by: Fini Jastrow <[email protected]>
[why]
We have two variables that hold the same data (sym_dim and dim), which
is confusing ('why do we have it?').

There is also the big 'if' on 'do we want to scale', which contains too
much. In the unlikely event that we have a glyph that needs to be scaled
by 1.0 AND have an overlap the code produces the wrong results.

[how]
Shuffle lines but no functional change (except that now we obey
'overlap' always (not that it has been a problem)).

Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii force-pushed the feature/allow-downscaling-nonmono branch from 9d61930 to 1637ef7 Compare January 12, 2023 11:50
@Finii Finii mentioned this pull request Jan 12, 2023
2 tasks
@Finii
Copy link
Collaborator Author

Finii commented Jan 12, 2023

Oh my ....
these double development waste of time ...

now:
eb396e6 font-patcher: Remove overlap from line-ish glyphs

last year (hidden in other PR):
2d53894 font-patcher: Remove overlap for line style powerline glyphs

@Finii
Copy link
Collaborator Author

Finii commented Jan 12, 2023

This closes PR #967 now.

Here some Issues that PR mentions:

@Finii Finii changed the title Feature: Allow symbol downscaling in nonmono (down to 2 'widths') Feature: Allow symbol scaling in nonmono (down to 2 'widths') Jan 12, 2023
@Finii Finii force-pushed the feature/allow-downscaling-nonmono branch from a9bc3d2 to eaed83b Compare January 12, 2023 12:26
Finii added 6 commits January 12, 2023 16:31
[why]
The vertical overlap has never been a problem (as far as I know). It is
maybe good to have some overlap for the terminal emulators that support
vertical overlap.
On terminals that truncate at the nominal cell border too much overlap
looks bad, i.e. the glyphs 'distorted'.

If we ever increase the overlap it is most likely be meant to be the
left-right overlap.

Note that the glyphs are usually valign='c' and the overlap is
distributed half top and half bottom. There are no other valign values
implemented (just 'not align' which is ... most likely bad).

[note]
Originally this has been part of commit fecda6a of #780.

[note2]
Originally this has been part of PR #967.
Although that had a bug 😬
It used max() instead of min() (T_T)

Signed-off-by: Fini Jastrow <[email protected]>

f
[why]
Somehow the option is mentioned but not implemented.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
The hexagons touch the left edge with a full body, so most likely people
do not want to have any visible gap there.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
The change introduced with commit
  Default some Powerline glyphs to '2 cells wide'

scales some Powerline glyphs to fit exactly into a 2 cell width. That
looks good on 'normal' fonts, but when the font becomes wider and less
tall at some point that is just too wide.

This is especially the case with the SymbolsOnly font which has a 1:1
aspect ratio. Two cell Powerline glyphs would have an aspect ratio of
2:1 which is unusable.

[how]
Check the destination font cell aspect ratio.
When a two-cell glyph would be wider than 1.6 times its height the
two-cell-mode is forbitten and all Powerline glyphs are scaled into one
cell width.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
The vertical overlap is still not 'pixel perfect', it is off by a small
amount that differs by font.

[how]
The reason is the wrong formula. We take the relative widths of the
glyph to calculate the factor needed to add an overlap in height.
Of course we need to take the relative heights *duh*.

Sometimes I think how dumb can a single person be? :-}
I would say this is copy-and-paste laziness.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
The trapezoids look very clumsy if scaled too wide.
No user request, just aesthetics.

Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii force-pushed the feature/allow-downscaling-nonmono branch from eaed83b to 6115c0b Compare January 12, 2023 16:11
@Finii Finii merged commit 1e446bb into master Jan 12, 2023
@Finii Finii deleted the feature/allow-downscaling-nonmono branch January 12, 2023 16:32
LNKLEO pushed a commit to LNKLEO/Nerd that referenced this pull request Nov 24, 2023
…ling-nonmono

Feature: Allow symbol scaling in nonmono (down to 2 'widths')
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants