fix(desktop): force WebGL repaint on load and fall back to DOM after context loss#3477
fix(desktop): force WebGL repaint on load and fall back to DOM after context loss#3477hyrness wants to merge 1 commit intosuperset-sh:mainfrom
Conversation
…context loss After the ensureSession change (superset-sh#3252), terminals can receive data (including wide/CJK characters) via the WebSocket before the WebGL renderer initializes (deferred to rAF). When WebGL takes over, the pre-rendered cells are not correctly re-rendered, causing intermittent garbling that fixes on mouse selection. Two fixes: 1. Call terminal.refresh() after loading the WebGL addon to force a full repaint of all buffered cells with the WebGL renderer. 2. Set suggestedRendererType = "dom" in the onContextLoss handler so that subsequent terminals in the same session skip WebGL after a GPU context loss. Fixes superset-sh#3406
📝 WalkthroughWalkthroughThis change fixes garbled wide and CJK character rendering in the terminal by ensuring the WebGL renderer repaints all content after addon initialization and by properly falling back to DOM rendering when WebGL context loss occurs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR applies two targeted fixes to the WebGL terminal renderer to eliminate intermittent CJK (and occasionally Latin) character garbling on initial load. The root cause is a timing window between WebSocket data delivery and WebGL's deferred
Confidence Score: 5/5Safe to merge — minimal, targeted changes with no risk of regressions. Both fixes are logically sound and well-understood. refresh(0, rows-1) after loadAddon is the canonical way to force a full repaint in xterm.js, and setting suggestedRendererType = dom on context loss matches the established VS Code pattern already used in the catch block. The same two-line delta is applied symmetrically to both code paths (v1 and v2 terminal helpers), and no existing behaviour is altered in the non-error path beyond one extra synchronous repaint call per terminal open. No new state, no async complexity, no API surface changes. No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "fix(desktop): force WebGL repaint on loa..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/lib/terminal/terminal-addons.ts (1)
46-65: Optional DRY cleanup: extract shared WebGL init/fallback block.This rAF WebGL setup/fallback logic now exists in both
apps/desktop/src/renderer/lib/terminal/terminal-addons.tsandapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts. A small shared helper would reduce drift risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/terminal/terminal-addons.ts` around lines 46 - 65, Extract the duplicated WebGL init/fallback logic into a shared helper function (e.g., initWebglRenderer) and replace the duplicated blocks in terminal-addons.ts and Terminal/helpers.ts with calls to that helper; the helper should accept the terminal instance and mutable state refs (webglAddon, suggestedRendererType, disposed) or return updated values, perform the requestAnimationFrame setup, create WebglAddon, register onContextLoss to dispose and set suggestedRendererType = "dom" and call terminal.refresh(0, terminal.rows - 1), call terminal.loadAddon(webglAddon), and catch errors to set webglAddon = null and suggestedRendererType = "dom" so both sites reuse the same robust init/fallback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/src/renderer/lib/terminal/terminal-addons.ts`:
- Around line 46-65: Extract the duplicated WebGL init/fallback logic into a
shared helper function (e.g., initWebglRenderer) and replace the duplicated
blocks in terminal-addons.ts and Terminal/helpers.ts with calls to that helper;
the helper should accept the terminal instance and mutable state refs
(webglAddon, suggestedRendererType, disposed) or return updated values, perform
the requestAnimationFrame setup, create WebglAddon, register onContextLoss to
dispose and set suggestedRendererType = "dom" and call terminal.refresh(0,
terminal.rows - 1), call terminal.loadAddon(webglAddon), and catch errors to set
webglAddon = null and suggestedRendererType = "dom" so both sites reuse the same
robust init/fallback behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 031225a3-ccab-4a8c-998a-9500b1aa2cc4
📒 Files selected for processing (2)
apps/desktop/src/renderer/lib/terminal/terminal-addons.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
1 participant
Description
Fixes intermittent garbling of CJK (and occasionally Latin) characters in the terminal that resolves when the text is selected with the mouse.
After #3252 (
54e8a1c8), terminal attachment callsensureSessionvia tRPC before connecting the WebSocket. This creates a timing window where the terminal can receive data — including wide/CJK characters — before the WebGL renderer has initialized (WebGL is deferred torequestAnimationFrame). During that window, cells are rendered by the DOM renderer. When WebGL takes over, it does not automatically repaint those pre-rendered cells, leaving wide characters in an inconsistent state in the texture atlas. Selecting the text forces a repaint which corrects the display.Two fixes in
terminal-addons.tsandhelpers.ts:terminal.refresh(0, terminal.rows - 1)immediately afterterminal.loadAddon(webglAddon)so all buffered cells are re-rendered through the WebGL renderer.suggestedRendererType = "dom"in theonContextLosshandler so subsequent terminals skip WebGL after a GPU context loss.Related Issues
Fixes #3406
Type of Change
Testing
Run the desktop app and output CJK characters in the terminal (e.g.
echo "こんにちは 你好"). Verify no garbling occurs on initial render and after switching workspaces/tabs.Screenshots (if applicable)
Additional Notes
The garbling was intermittent and depended on timing between WebSocket data delivery and WebGL initialization, making it more likely to occur on slower machines or under load.
Summary by cubic
Fixes intermittent CJK and wide-character garbling in the desktop terminal by forcing a full repaint when WebGL initializes and falling back to the DOM renderer after GPU context loss. Ensures correct rendering on first load and after context issues.
terminal.refresh(0, terminal.rows - 1)to repaint all buffered cells through WebGL.suggestedRendererType = "dom"so subsequent terminals skip WebGL.Written for commit c9ee064. Summary will update on new commits.
Summary by CodeRabbit