fix(terminal): register configured font as a local() @font-face for the WebGL renderer (#898)#918
fix(terminal): register configured font as a local() @font-face for the WebGL renderer (#898)#918yagop wants to merge 6 commits into
Conversation
📝 WalkthroughWalkthroughAdds ChangesLocal font registration
Estimated code review effort: 3 (Moderate) | ~20 minutes Related issues: Suggested labels: bug, terminal, webgl, fonts Suggested reviewers: crynta 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Michi-Task: f0a1 Michi-Issue: 898
Michi-Task: a6b7 Michi-Issue: 898
|
😺 Michi — ready for review 🐾 Implemented in 4 commits on
CI status. This is a cross-fork PR, so GitHub Actions won't run until a maintainer approves the workflow — no checks are queued yet. The change is frontend-only (no
Please approve the workflow run so CI can confirm on GitHub. The Not merged, not closing the issue — that's yours. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/modules/terminal/lib/rendererPool.ts (1)
820-832: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCall
slot.term.clearTextureAtlas()directly.@xterm/xterm6.0.0 already exposes this onTerminal, so the cast throughwebglAddonis unnecessary. Keep the refresh afterward so the atlas rebuild still picks up the registered font.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/terminal/lib/rendererPool.ts` around lines 820 - 832, The clearWebglAtlas helper is still reaching through slot.webglAddon with an unnecessary cast even though Terminal already exposes clearTextureAtlas. Update clearWebglAtlas to call slot.term.clearTextureAtlas() directly, keep the existing refresh afterward in slot.term.refresh, and remove the webglAddon shim so the method reference stays aligned with `@xterm/xterm` Terminal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/modules/terminal/lib/rendererPool.ts`:
- Around line 820-832: The clearWebglAtlas helper is still reaching through
slot.webglAddon with an unnecessary cast even though Terminal already exposes
clearTextureAtlas. Update clearWebglAtlas to call slot.term.clearTextureAtlas()
directly, keep the existing refresh afterward in slot.term.refresh, and remove
the webglAddon shim so the method reference stays aligned with `@xterm/xterm`
Terminal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a2e2aaad-e0be-4c9e-abe9-330e14724918
📒 Files selected for processing (3)
src/lib/fonts.test.tssrc/lib/fonts.tssrc/modules/terminal/lib/rendererPool.ts
…rynta#898) Michi-Task: c8d9 Michi-Issue: 898 Michi-Review: 4610874355
|
😺 Michi — addressed the review 🐾 @coderabbitai thanks for the review — both points handled:
Leaving the thread for you to resolve. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
gh pr view 918 --repo crynta/terax-ai --json title,commits -q '.title, (.commits[-3:] | .[].messages)' 2>/dev/null
echo "---diff---"
gh pr diff 918 --repo crynta/terax-ai | sed -n '1,200p'Length of output: 7874 🏁 Script executed: #!/bin/bash
gh pr diff 918 --repo crynta/terax-ai | grep -n "clearTextureAtlas\|clearWebglAtlas\|webglAddon as" -A3 -B3Length of output: 1388 Confirmed — both changes look right:
Nice work, (๑˃̵ᴗ˂̵)و |
Closes #898
Register the configured terminal font as a
local()@font-faceso the macOS WKWebView WebGL glyph-atlas rasterizer can resolve it, then rebuild the atlas. Fixes the "two fonts at once" mismatch where a subset of glyphs fell back to bundled JetBrains Mono under the WebGL renderer while the DOM renderer was fine (same mechanism as #820).registerLocalFont()tosrc/lib/fonts.ts— inject alocal()@font-facefor the configured family and load it into the FontFaceSet, no-op on blank/comma-stack inputrendererPool.ts): add aclearWebglAtlas()helper and register the font + rebuild the atlas onattachWebglapplyFontFamilyso live font-family changes re-resolve under WebGLregisterLocalFontunit tests tosrc/lib/fonts.test.ts(guards,@font-faceinjection, dedup)clearWebglAtlasto callslot.term.clearTextureAtlas(), dropping thewebglAddoncastSummary by CodeRabbit
New Features
Bug Fixes