fix(font): Apply glyph constraints before thickening and centering before quantizing - #8580
Conversation
danielwe
changed the title
8b19c0f to
6b493c4
Compare
6b493c4 to
efc685c
Compare
danielwe
changed the title
#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 🤞
|
Merged some of your other PRs first which caused a conflict, can you please rebase this one. |
a6f822e to
d11da07
Compare
|
Rebased and ready. Made a tiny change to avoid an unnecessary merge conflict between this #8847. |
There was a problem hiding this comment.
Why did you move the code that bails out for glyphs that are too small after all the (relatively) expensive constraint calculations?
Also, as a side note, I'm pretty sure if we're gonna move the FreeType outline emboldening code at all it should be moved before we measure the glyph (additional safety checks may need to be added to make sure that there are outlines to embolden in the first place).
The idea was (since I was reordering this flow anyway) to make this call based on the actual size of the glyph after it's possibly been scaled and emboldened. In any case, if the constraint is nontrivial it seems unlikely that there's going to be cause for bailing, i.e., we'd probably never actually go through the expensive calculations only to bail right after (except if we've ended up with a font size so small that we bail on every glyph, in which case the user probably has bigger things to worry about than this inefficiency). But happy to reorder and do this on the unscaled and unpadded rect if that's what you prefer! |
No, the point of this PR is precisely that constraints must be applied to the bbox of the unboldened/unthickened glyph to work correctly. With Coretext, this is important for everyday usage---you don't want the size of NF icons to depend on the amount of font thickening. With FreeType, it's perhaps a bit more academic, but it's totally possible for a user to specify a font that does not have a bold variant, but is a Nerd Fonts patched font. In that case, we will be generating synthetically emboldened NF icons (which is kind of dumb, but it is what it is), and an emboldened glyph should not be downscaled to fit in the same bbox as the unboldened glyph (imagine that for letters, it would look like the bold is a smaller font size than non-bold). |
|
Font thickening and emboldening are completely different things. |
|
I see your argument about bold symbols having the same basic proportions as non-bold ones with synthetic bold, but that's not the effect we'd get if a font did have a bold with symbols that have constraints applied, and it feels odd to me to be inconsistent about that. Realistically, the symbols I'm more concerned about aren't the ones that are always scaled, but the ones that are simply constrained to cells-- I don't want synthetic bold versions of those symbols to overhang the edge(s) of their cell(s). |
Which symbols are this? Normally, if a font specifies outlines that go outside the cells, they are just drawn outside the cells, right? There's probably some case I'm forgetting. |
|
ghostty/src/renderer/generic.zig Lines 3091 to 3097 in a5aff0e |
|
Thanks, I see.
|
|
In summary, let me know which changes you want me to push:
|
d11da07 to
2b6f3e8
Compare
|
Alright, I pushed a new version that makes a simpler and more minimal set of changes along the lines you indicated.
|
|
This looks good to me now, I think, but I don't have the energy to do a proper review tonight. Hopefully in the morning I can. |
In Freetype, measure rect after emboldening, so constraints apply to the true glyph size like in CoreText. In CoreText, don't let font smoothing affect the rect (only the canvas).
697b9c7 to
f245574
Compare
…fore quantizing (#8580) In CoreText, when thickening (font smoothing) is enabled or Ghostty is synthesizing a bold face, the glyph bounding box is padded to make sure the thicker glyph can fit. Currently, this happens before applying constraints (scaling and alignment), which makes the size and position of constrained glyphs dependent on font size, font thickening strength, and display DPI. With this PR, constraints are applied before any other adjustments, and padding is applied directly to the rasterization canvas without modifying any metrics. For consistency, I also moved constraint application above emboldening in the FreeType code, although under that API, the two operations are orthogonal as far as I can tell. Secondly, this PR moves glyph centering above bitmap quantization, as centering is generally fractional and will therefore undo the quantizing if done after. Supersedes #8552.
4 participants
In CoreText, when thickening (font smoothing) is enabled or Ghostty is synthesizing a bold face, the glyph bounding box is padded to make sure the thicker glyph can fit. Currently, this happens before applying constraints (scaling and alignment), which makes the size and position of constrained glyphs dependent on font size, font thickening strength, and display DPI.
With this PR, constraints are applied before any other adjustments, and padding is applied directly to the rasterization canvas without modifying any metrics.
For consistency, I also moved constraint application above emboldening in the FreeType code, although under that API, the two operations are orthogonal as far as I can tell.
Secondly, this PR moves glyph centering above bitmap quantization, as centering is generally fractional and will therefore undo the quantizing if done after.
Supersedes #8552.