Repository navigation
Conversation
Member
|
I don't know that this is the correct solution. I think maybe we'd be better off just calculating the constraints on the original bounding box and then adding 1px of padding to the raster surface if thickening is enabled. I do agree a lot of things look better without thickening, but I don't know whether we want to make that judgement call for users or not. I'll have to give it some thought. |
Contributor
Author
|
Adding the pixel padding after applying the constraints certainly works to fix the scaling issue. I guess if users were using a patched font in a less fancy terminal, thickening would apply equally to icons and other glyphs, so I definitely see the case for not being too clever with this. Let me know what you decide, and I'll push whatever changes are needed. Do you agree with the synthetic bold/italic opt-out, or would you rather just give the user what they asked for there as well? |
Contributor
Author
|
Alternative branch ready to go here: main...danielwe:ghostty:constraints_before_thickening. No opting out of anything, only reordering operations to avoid the impact on scaling and alignment. |
Contributor
Author
|
Maybe let's look at some screenshots. Here's a file listing with a selection of more or less detailed icons. Ghostty is running with config
|
Contributor
Author
|
How about we just go for the other version without any special-case opt-out, at least for now? That's a plain bugfix and should be uncontroversial. The argument against trying to be too clever is growing on me. I'll submit that branch as a new PR and close this one. |
mitchellh
added a commit
that referenced
this pull request
Sep 29, 2025
#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 🤞
mitchellh
added a commit
that referenced
this pull request
Oct 3, 2025
…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.
mitchellh
added a commit
that referenced
this pull request
Oct 6, 2025
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This disables CoreText font smoothing for glyphs with nontrivial constraints, which, for the most part, means Nerd Font icons (also emojis, but they are bitmaps, so they were never smoothed anyway).
This is the right thing to do because icons aren't like other glyphs, and font smoothing may make their details less crisp (see, for example, the Nix snowflake and the TeX logo). Perhaps even more importantly, font smoothing interferes badly with icon scaling because it adds a 1-pixel border to the glyph bounding box, causing the scaling algorithm to perceive the icon as larger than it is and therefore scale it to be smaller than it should be. The magnitude of this effect depends on font size and monitor DPI, and is greater on lower-DPI monitors using smaller font sizes.
I also disabled synthetic bold and italic for icons (except italic in CoreText, as it applies per face, and I didn't see a way to opt out per glyph). This is a more hypothetical concern, but it's possible to set the primary font to a patched nerd font that lacks bold and/or italic, in which case you wouldn't want the synthesizing to apply to the icon glyphs.
Lastly, a fly-by bugfix: quantizing bitmap glyphs should be done after glyph centering, as the latter is generally fractional and would negate the quantizing.