Repository navigation
fix(gpui): register bundled fonts on macOS - #93
Conversation
Core Graphics cannot load the bitmap-only (CBDT) Buzz Color Emoji font, and GPUI rejects a whole add_fonts batch when one font fails. On macOS that dropped Inter and JetBrains Mono too: every text style missed its font and fell through GPUI's fallback stack on each layout, which kept the main thread busy and made scrolling and typing choppy. Register each font on its own so one failure cannot drop the rest, and on macOS skip the bundled emoji font and fall back to Apple Color Emoji. Linux and Windows keep the bundled emoji font.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Platform-specific font configuration apps/buzz-gpui/crates/ui/src/fonts.rs |
The font list excludes EMOJI_FONT on macOS. FALLBACK_EMOJI selects Apple Color Emoji on macOS and Buzz Color Emoji elsewhere. register() registers fonts separately and logs failures per font. fallbacks() uses the platform-specific fallback name. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested labels: human review wanted
Suggested reviewers: tee8z
Merge Risk: 
27ce9
-
No concrete current-head defect was established; the change is ready to merge.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: registering bundled fonts on macOS. Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
tee8z
left a comment
tee8z
left a comment
There was a problem hiding this comment.
Approving.
- Registering one font per
add_fontscall is the right fix for GPUI's macOS all-or-nothingcollect::<Result<_>>(). It also stops one bad font from taking out the rest on Linux and Windows. #[cfg(not(target_os = "macos"))]on theEMOJI_FONTarray element is valid. macOS and Windows clippy (-D warnings) pass, soEMOJIdoesn't go dead on macOS.- The only other user of the bundled emoji, the avatar rasterizer (
settings/avatar/raster.rs), loadsEMOJI_FONTinto its ownfontdband looks upfonts::EMOJIthere. It doesn't go through GPUI, so macOS avatar emoji are unaffected.
Optional (doc): in the module comment, "macOS cannot load it, so there the fallback is the system's Apple Color Emoji. Two edits make that work…" now reads as if the two fontTools edits are what make Apple Color Emoji work. Moving the macOS sentence after the bullet list would fix that. That line is also longer than the surrounding wrap width.
Not verified: colour emoji on macOS through Apple Color Emoji (the PR notes this too).
Minimalism 9 · elegance 9 · correctness 9.
tee8z
left a comment
tee8z
left a comment
There was a problem hiding this comment.
Re-approving. The earlier approval was dismissed by the stale-review automation when the stack's merge commits propagated #90's fix (17:39:52Z); this PR's own diff is unchanged from what I reviewed. Findings and gates from the previous review stand.
Layer 4 of the macOS stack, on top of #91.
Problem
On macOS, startup logged
buzz-ui: failed to register bundled fonts: Could not load an embedded font.and the app held a CPU core while scrolling and typing.Core Graphics cannot load the bitmap-only (CBDT)
BuzzColorEmoji.ttf. GPUI's macOSadd_fontsloads the whole batch withcollect::<Result<_>>(), so that one failure dropped Inter and JetBrains Mono as well. Every text style then missed its font. GPUI caches the failed lookup, but it builds a new error on each call and walks its fallback stack (Lilex, IBM Plex Sans, then Helvetica) on every layout. WithRUST_BACKTRACE=1, each of those errors also captured a backtrace, which took about 23% of main-thread samples.Change
crates/ui/src/fonts.rs:add_fontscall, so one failure can't drop the rest.BuzzColorEmoji.ttfout of the GPUI fonts. Text styles fall back toApple Color Emojithere.EMOJI_FONTthrough fontdb on every platform.Validation
BuzzColorEmoji.ttffails.buzz-gpui-voltageon aarch64-darwin: the registration error is gone. Idle CPU is about 0.4%. A 4 ssampleof the main thread has nofont_id,resolve_fontor backtrace frames (before: 614 of 2,680).cargo fmt --all --checkpasses.cargo clippy -p buzz-ui --all-targets --locked -- -D warningspasses in the Nix dev shell.Summary by CodeRabbit