Repository navigation
Scale non-mono icons to be at least as large as mono - #1926
Conversation
|
Thanks for the interesting PR! I remember pondering that problem some years (?) ago when I wrote the "scales down never up" comment in the code. |
I can imagine! Consider this my outcry about the current state 😅 You're free to take it or leave it, of course---just thought I'd bundle a suggested solution together with my complaint. I struggle to see why anyone would prefer upscaling that only applies in monospace (note again that in the screenshots I provided, the left is non-Mono and the right is Mono), but stranger things have happened. |
#8563) > This PR will probably need a rebase on the final outcome of #8550 and ~#8552~ #8580, but I'm putting it out here so folks can begin taking a look if they want. This is a rewrite of the code that applies scaling and alignment constraints. The main intention is to further improve how Nerd Font icons are matched to the primary font. This PR aligns the calculations more closely with how the Nerd Font `font-patcher` script works, except in two cases where we can easily do something unambiguously better (one because of what's arguably a bug in the script, and one because we do multi-cell alignment with knowledge of the pixel-rounded cell grid). A goal of the rewrite is to make the scaling and alignment calculations as clear and easy to follow as possible. I'll lead with some screenshots. First the status quo, then this PR. <img width="505" height="357" alt="Screenshot 2025-09-07 at 17 23 51" src="https://github.com/user-attachments/assets/8e3ff9fd-3b66-4d54-be38-d54cf3b6cc5b" /><img width="505" height="357" alt="Screenshot 2025-09-07 at 17 20 39" src="https://github.com/user-attachments/assets/84fbe076-2e3f-4879-b9b2-91ce86b9ef5f" /> Relevant specs: macOS; 1920x1080; Ghostty config: ```ini font-family = "CommitMono" font-size = "15" adjust-cell-height = "+20%" ``` **Points to note** * Icons are generally larger, making better use of the available space. * Icons are aligned nearly a pixel lower, better matching the text. This is because alignment is now calculated from face metrics/bearings, not the pixel-rounded cell. (See more below.) * Relative sizes are better matched. Note especially that tall and narrow icons, like the git branch symbol and icons depicting sheets of paper, look conspicuously small in the status quo. With this PR, they're better matched to other icons. * Look at the letter Z icon I use as prompt character for zsh. It's _tiny_ in the status quo, but properly sized with this PR. This demonstrates the most important and clear-cut improvement we make over `font-patcher`. (See more below.) * Icons wider than a single cell are now left-aligned rather than centered across two cells. I think this is preferable and makes better use of space in most relevant contexts. - Consider a Neovim bufferline showing the buffer title as a filetype icon followed by the file name. Padding on the left would be a waste of space, but having that extra space on the right can improve legibility. - In listings, such as in the screenshots, columns look tidier when their left edges are straight rather than ragged. - This is how `font-patcher` does alignment, and thus what Nerd Font users and UI designers expect. **Implementation details** I won't get too deep in the weeds here; see the code and comments. In brief: * `size_horizontal` and `size_vertical` are combined to a single `size`, which can be `.none, .stretch, .fit, .cover` or `.fit_cover1`. The latter implements the `pa` rule from `font-patcher`, except it works better for icons that are small before scaling, like the letter Z prompt in the screenshots. In short, it preserves aspect ratio while clamping the size such that the icon `.cover`s at least one cell and `.fit`s within the available space. See code comments and ryanoasis/nerd-fonts/pull/1926 for details. * An alignment mode `.center1` is added, implementing the centering rule from `font-patcher` that I explained/defended above. In short, we center the icon _in the first cell_, even it's allowed to span multiple cells. For icons wider than a single cell, the lower bound that prevents them from protruding to the left kicks in and turns this into left-alignment. We keep the regular `.center` rule around for use with emojis, et cetera. * Scaling and alignment calculations only use the unrounded face metrics and bearings. This ensures that pixel rounding of the cell and baseline, and `adjust-cell-{width,height}`, don't affect scaling or relative alignment; the icons are always scaled and aligned to the _face_. (The one place we need to use cell metrics in the calculations is when we use `cell_width` to obtain the inter-cell padding needed to correctly center or right-align a glyph across two cells.) - We can do this with impunity because we're blessed with sprite glyphs in place of the "icons" that are actually box drawing and block graphics characters 🙌 **Guide** The meat of the changes is 100 % in `src/font/face.zig` and `src/font/nerd_font_codegen.py`. Changes to other files only amount to a) adding/changing some struct fields to get numbers to where they need to be (see `src/font/Metrics.zig`), and b) collateral updates to make otherwise unchanged code and tests work with/take advantage of the modified structs. Most files should have a clear and friendly diff. The exception is the bottom half of `src/font/face.zig`, where the diff is meaningless and the new code should just be reviewed on its own merits. This is the part where the `constrain` function is rewritten and refactored. Scarred by countless hours perusing `font-patcher`, I tried hard to make the math and logic easy to follow here. I hope I have succeeded 🤞
|
Thanks for the nice PR. I added some related ... cleanup or how you would call that, that is all related somehow. Unfortunately most of your lines were touched which I find a pity in Hope you are ok with the additions. And then afterwards I did a rebase on master to get a more linear history and an empty merge commit. Just a tick of mine, linear history makes browsing the history so much easier. Force push of course needed afterwards |
|
To verify your changes and also my additions to them I ran these 5 patch runs and used |
|
@allcontributors please add @danielwe for code |
|
I've put up a pull request to add @danielwe! 🎉 |
|
One drawback could be that upscaled neighboring icons do not match in size anymore; but in principle all relevant 'groups' of icons should be in the If people complain we can add more glyphs to groups. 🤔 |
|
What I eventually realized is that the most correct way to treat these icons would be to put them in a scale group with all the framed/inverted numbers, i.e.,
I don't think this is true? There are no scale groups at all in the entire material design set, it seems like it was just too daunting of a task to get started on given the sheer number of icons? |
|
See this discussion for a better explanation/illustration: ghostty-org/ghostty#9172 (comment) |
|
Regarding MDI Yes.. and there are more, see #773 (comment) (the scaling result is shown further down). In the beginning we had all icons in one group (as they seem nicely scaled to each other in the source, so just keep that for all MDI icons), but that has been dropped with Hmm, Lets examine some samples... 
Chevrons 
Triangles, note all numbers-in-boxes seem ok (because they are not scaled up and not down) 
As in the linked issue above 
Small symbols and numbers/letters 
Arrows 
Roman numerals 
F-keys and sizes 
Rhombus and ... what? 
Again arrows and triangles 
Counting marks These are all that I noticed when patched into Cascadia Code. Other fonts may have other problematic glyphs. |
|
And if we put all md-numerics into one scale group of course the person who uses 
|
|
yeah you can't win... =/ |
|
That said, with the glaring inconsistency between mono and regular, the case for fixing it one way or the other seems pretty solid and should be straightforward to defend from a principled standpoint, whichever way you choose. (That doesn't mean people won't have feelings, of course.) |
|
I added some DetailsMDI_SCALE_LIST = {'ShiftMode': '', 'ScaleGroups': [
[*range(0xf00da, 0xf00dc + 1), *range(0xf0f61, 0xf0f68 + 1)], # moons
range(0xf013c, 0xf0143 + 1), # chevrons
range(0xf035d, 0xf0360 + 1), # triangles
[*range(0xf0387, 0xf038d + 1), 0xf0f71, 0xf0f72, *range(0xf0f74, 0xf0f77 + 1)], # musical notes
[*range(0xf0712, 0xf0716 + 1), 0xf0a6f ], # cellular signals
range(0xf0718, 0xf071e + 1), # commit symbols
[0xf09dd, 0xf09de, 0xf09df, 0xf0a10, 0xf0a13, 0xf014, 0xf0a15, 0xf14dc, 0xf1554], # small circles etc (first glyph is size reference)
range(0xf0aee, 0xf0b07 + 1), # alpha
range(0xf0b39, 0xf0b42 + 1), # numerics
range(0xf0bb5, 0xf0bbe + 1), # pan buttons
range(0xf1088, 0xf1091 + 1), # roman numerals
range(0xf12ab, 0xf12b3 + 1), # f-keys (only 1-9, scale f10 etc independently)
range(0xf13a2, 0xf13a9 + 1), # sizes
range(0xf19b0, 0xf19b7 + 1), # arrows
range(0xf1abc, 0xf1ac0 + 1), # tally marks
]}
I guess that is because of our hard cut at scale 1.0... the slimmer glyphs fall into another category than the wider ones. Within a ScaleGroup there should be no switch in scale-up or not-scale-up 🤔 Examining... Edit: Oh no, they share glyphs for different codepoints: Excerpts of *sigh* |
|
Groan! And of course other code does also not work with multi-codepoint-symbol-fonts, so the 
More fixes coming. |
[why]
They have the same glyphs on two different codepoints with different
meanings, but of course only one name. This will break the cheat sheet.
[how]
Add some hand-picked names for the problematic glyphs/codepoints.
Rerun to update the library file:
./generate-glyph-info-from-set.py \
--font ../../src/glyphs/materialdesign/MaterialDesignIconsDesktop.ttf \
--start f0001 --end f1af0 --offset 0 --prefix md > lib/i_md.sh
On release that will update the glyphnames and cheat sheet databases.
[note]
Noticed when working on PR ryanoasis#1926.
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
|
Already applied bugfix commit 1911114 to |
Originally from PR ryanoasis#1926 Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
No functional change. Also pull 'main' condition to the front of the if, i.e. 'this is a non-mono target'. Also rename function parameter that might be confused where the function is actually used and there is a variable with the same name (but different contents). Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
We have no rule with 'pa' and 'overlap' that does not also have '!' (and '1'), but technically 'overlap' and '!' should be distinct. I.e. this change does not affect any existing case. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] The icon height is not a fixed size anymore for a given patch run, but we need to detect it depending on situation: - normally it is the old iconheight - full height in regular and propo - a bit less tall in mono - with regular and propo the icon height is now - upper limit is full height - lower limit (upscale to) is a bit less tall like mono The property is now used only for a debugging output and can leave the wrong expression that there is a fixed iconheight per patch run. Better remove it to avoid confusion. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
after changing the icon size of Regular and Propo fonts Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] Sometimes we want to have some special feature to try out before we fully commit to it. Adding a real command line flag for that would probably be misleading. [how] Add --experimantal which takes unspecified switches that can come and go, explicitely mentioned to be an experiment. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
|
Rebase on master, force push |
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
|
Rechecked the products of
with
combined with
Which also checks that I guess we can merge this now, having the algorithm in place and could all gain experience together if this is what is preferable. Or if additional size limits are needed, or more scale groups etc. |
|
Thank you again @danielwe! Lets first gain more experience, now we have some guide rails set up. |
|
Great effort! I haven't studied the details, but looking forward to seeing the user feedback. |
…t (#8563) > This PR will probably need a rebase on the final outcome of #8550 and ~#8552~ #8580, but I'm putting it out here so folks can begin taking a look if they want. This is a rewrite of the code that applies scaling and alignment constraints. The main intention is to further improve how Nerd Font icons are matched to the primary font. This PR aligns the calculations more closely with how the Nerd Font `font-patcher` script works, except in two cases where we can easily do something unambiguously better (one because of what's arguably a bug in the script, and one because we do multi-cell alignment with knowledge of the pixel-rounded cell grid). A goal of the rewrite is to make the scaling and alignment calculations as clear and easy to follow as possible. I'll lead with some screenshots. First the status quo, then this PR. <img width="505" height="357" alt="Screenshot 2025-09-07 at 17 23 51" src="https://github.com/user-attachments/assets/8e3ff9fd-3b66-4d54-be38-d54cf3b6cc5b" /><img width="505" height="357" alt="Screenshot 2025-09-07 at 17 20 39" src="https://github.com/user-attachments/assets/84fbe076-2e3f-4879-b9b2-91ce86b9ef5f" /> Relevant specs: macOS; 1920x1080; Ghostty config: ```ini font-family = "CommitMono" font-size = "15" adjust-cell-height = "+20%" ``` **Points to note** * Icons are generally larger, making better use of the available space. * Icons are aligned nearly a pixel lower, better matching the text. This is because alignment is now calculated from face metrics/bearings, not the pixel-rounded cell. (See more below.) * Relative sizes are better matched. Note especially that tall and narrow icons, like the git branch symbol and icons depicting sheets of paper, look conspicuously small in the status quo. With this PR, they're better matched to other icons. * Look at the letter Z icon I use as prompt character for zsh. It's _tiny_ in the status quo, but properly sized with this PR. This demonstrates the most important and clear-cut improvement we make over `font-patcher`. (See more below.) * Icons wider than a single cell are now left-aligned rather than centered across two cells. I think this is preferable and makes better use of space in most relevant contexts. - Consider a Neovim bufferline showing the buffer title as a filetype icon followed by the file name. Padding on the left would be a waste of space, but having that extra space on the right can improve legibility. - In listings, such as in the screenshots, columns look tidier when their left edges are straight rather than ragged. - This is how `font-patcher` does alignment, and thus what Nerd Font users and UI designers expect. **Implementation details** I won't get too deep in the weeds here; see the code and comments. In brief: * `size_horizontal` and `size_vertical` are combined to a single `size`, which can be `.none, .stretch, .fit, .cover` or `.fit_cover1`. The latter implements the `pa` rule from `font-patcher`, except it works better for icons that are small before scaling, like the letter Z prompt in the screenshots. In short, it preserves aspect ratio while clamping the size such that the icon `.cover`s at least one cell and `.fit`s within the available space. See code comments and ryanoasis/nerd-fonts/pull/1926 for details. * An alignment mode `.center1` is added, implementing the centering rule from `font-patcher` that I explained/defended above. In short, we center the icon _in the first cell_, even it's allowed to span multiple cells. For icons wider than a single cell, the lower bound that prevents them from protruding to the left kicks in and turns this into left-alignment. We keep the regular `.center` rule around for use with emojis, et cetera. * Scaling and alignment calculations only use the unrounded face metrics and bearings. This ensures that pixel rounding of the cell and baseline, and `adjust-cell-{width,height}`, don't affect scaling or relative alignment; the icons are always scaled and aligned to the _face_. (The one place we need to use cell metrics in the calculations is when we use `cell_width` to obtain the inter-cell padding needed to correctly center or right-align a glyph across two cells.) - We can do this with impunity because we're blessed with sprite glyphs in place of the "icons" that are actually box drawing and block graphics characters 🙌 **Guide** The meat of the changes is 100 % in `src/font/face.zig` and `src/font/nerd_font_codegen.py`. Changes to other files only amount to a) adding/changing some struct fields to get numbers to where they need to be (see `src/font/Metrics.zig`), and b) collateral updates to make otherwise unchanged code and tests work with/take advantage of the modified structs. Most files should have a clear and friendly diff. The exception is the bottom half of `src/font/face.zig`, where the diff is meaningless and the new code should just be reviewed on its own merits. This is the part where the `constrain` function is rewritten and refactored. Scarred by countless hours perusing `font-patcher`, I tried hard to make the math and logic easy to follow here. I hope I have succeeded 🤞
2 participants
Description
For icons with the
pastretch rule,font_patchercurrently scales icons up to fit the cell in monospace patching, but only scales down for non-monospace. This makes some icons significantly smaller with non-monospaced patching than with monospaced, which is rather unexpected. Case in point: thend-md-alphaalphabetic icons are tiny in non-monospaced fonts, but substantial in monospaced.With this PR, non-monospaced patching never makes smaller icons than monospaced. For
pa, if monospaced patching would scale up, non-monospaced scales up by the same amount, but no more.Requirements / Checklist
Issue number where discussion took place: #xxx
What does this Pull Request (PR) do?
iconheightout into a separate function that takes an optionalsingle_widthparametersingle_widthwhen neededpaicons that don't need downscaling, sets the scale factor to the maximum of 1 and the single width scale factor.How should this be manually tested?
(nd-md-alpha_z)in both variants at the same font sizeAny background context you can provide?
See explanation.
What are the relevant tickets (if any)?
NA.
Screenshots (if appropriate or helpful)
See above.