Repository navigation
Conversation
The xterm.js WebGL addon can silently corrupt its glyph texture atlas, causing terminal characters to render as wrong glyphs. The existing fallback only triggers on complete WebGL context loss, missing partial corruption. This adds a localStorage-based preference check so users can set `terminal-renderer-type` to `"dom"` to bypass WebGL without needing `--disable-gpu` (which disables all GPU acceleration). Fixes ParadevAI#3794 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughTwo terminal initialization modules now pre-read 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 SummaryAdds a Confidence Score: 5/5Safe to merge — the The only finding is P2: the No files require special attention
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/lib/terminal/terminal-addons.ts | Adds IIFE-based localStorage read to initialise suggestedRendererType; core "dom" opt-out logic is correct, "webgl" value is functionally a no-op (P2 style note) |
| apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts | Applies the identical localStorage-preference initialisation to the v1 terminal path; same P2 note about "webgl" being equivalent to undefined |
Sequence Diagram
sequenceDiagram
participant LS as localStorage
participant Mod as Module init (IIFE)
participant RAF as requestAnimationFrame
participant WGL as WebglAddon
participant DOM as DOM renderer
Mod->>LS: getItem("terminal-renderer-type")
LS-->>Mod: "dom" | "webgl" | null
Note over Mod: suggestedRendererType = "dom" | "webgl" | undefined
Note over RAF: New terminal opened
RAF->>RAF: suggestedRendererType === "dom"?
alt "dom" (or disposed)
RAF-->>DOM: skip WebGL, use DOM renderer
else "webgl" or undefined
RAF->>WGL: new WebglAddon() + loadAddon()
alt WebGL succeeds
WGL-->>RAF: WebGL renderer active
WGL->>WGL: onContextLoss → dispose WebGL
else WebGL throws
RAF->>Mod: suggestedRendererType = "dom"
RAF-->>DOM: fallback to DOM renderer
end
end
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/terminal-addons.ts
Line: 19-25
Comment:
**`"webgl"` preference is functionally identical to `undefined`**
Setting `terminal-renderer-type` to `"webgl"` initializes `suggestedRendererType` to `"webgl"`, but the only branch that guards on this variable is `if (… suggestedRendererType === "dom") return` (line 55). Both `"webgl"` and `undefined` bypass that guard and attempt WebGL — so the two values are indistinguishable at runtime.
More importantly, if WebGL fails (the `catch` on line 66), `suggestedRendererType` is overwritten to `"dom"` in memory regardless of what localStorage says, so the user's explicit `"webgl"` preference is silently discarded for the remainder of the session. A user who sets `"webgl"` to "re-enable" WebGL on a broken machine will still end up on DOM after the first failure — exactly the same outcome as `undefined`.
Consider either documenting this limitation in a comment, or accepting only `"dom"` as a valid preference value (the only value that meaningfully changes behaviour). The same pattern applies to `helpers.ts`.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: allow users to disable WebGL termin..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let suggestedRendererType: "webgl" | "dom" | undefined = (() => { | ||
| try { | ||
| const pref = localStorage.getItem("terminal-renderer-type"); | ||
| if (pref === "dom" || pref === "webgl") return pref; | ||
| } catch {} | ||
| return undefined; | ||
| })(); |
There was a problem hiding this comment.
"webgl" preference is functionally identical to undefined
Setting terminal-renderer-type to "webgl" initializes suggestedRendererType to "webgl", but the only branch that guards on this variable is if (… suggestedRendererType === "dom") return (line 55). Both "webgl" and undefined bypass that guard and attempt WebGL — so the two values are indistinguishable at runtime.
More importantly, if WebGL fails (the catch on line 66), suggestedRendererType is overwritten to "dom" in memory regardless of what localStorage says, so the user's explicit "webgl" preference is silently discarded for the remainder of the session. A user who sets "webgl" to "re-enable" WebGL on a broken machine will still end up on DOM after the first failure — exactly the same outcome as undefined.
Consider either documenting this limitation in a comment, or accepting only "dom" as a valid preference value (the only value that meaningfully changes behaviour). The same pattern applies to helpers.ts.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/terminal-addons.ts
Line: 19-25
Comment:
**`"webgl"` preference is functionally identical to `undefined`**
Setting `terminal-renderer-type` to `"webgl"` initializes `suggestedRendererType` to `"webgl"`, but the only branch that guards on this variable is `if (… suggestedRendererType === "dom") return` (line 55). Both `"webgl"` and `undefined` bypass that guard and attempt WebGL — so the two values are indistinguishable at runtime.
More importantly, if WebGL fails (the `catch` on line 66), `suggestedRendererType` is overwritten to `"dom"` in memory regardless of what localStorage says, so the user's explicit `"webgl"` preference is silently discarded for the remainder of the session. A user who sets `"webgl"` to "re-enable" WebGL on a broken machine will still end up on DOM after the first failure — exactly the same outcome as `undefined`.
Consider either documenting this limitation in a comment, or accepting only `"dom"` as a valid preference value (the only value that meaningfully changes behaviour). The same pattern applies to `helpers.ts`.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (1)
64-73: Extract the localStorage preference reader to a shared helper.The IIFE at lines 67-73 is byte-identical to the one in
apps/desktop/src/renderer/lib/terminal/terminal-addons.ts(lines 19-25). Since the helper is used in 2+ places, it should be promoted to a shared module so the parsing/validation logic lives in one place — otherwise a future tweak (e.g., adding a"canvas"value, normalizing case, or migrating to a different storage key) has to be applied in two spots and can silently drift.A good home would be alongside the existing terminal utilities, e.g.
apps/desktop/src/renderer/lib/terminal/renderer-preference.ts:♻️ Proposed shared helper + call-site updates
New file
apps/desktop/src/renderer/lib/terminal/renderer-preference.ts:export type TerminalRendererType = "webgl" | "dom"; /** * Read the user's persisted terminal renderer preference. * Returns `undefined` if unset, invalid, or localStorage is unavailable. */ export function getStoredRendererPreference(): | TerminalRendererType | undefined { try { const pref = localStorage.getItem("terminal-renderer-type"); if (pref === "dom" || pref === "webgl") return pref; } catch {} return undefined; }Then in this file:
-// Once WebGL fails, skip it for all subsequent terminals (VS Code pattern). -// Users can also set localStorage "terminal-renderer-type" to "dom" to -// permanently disable WebGL (workaround for GPU texture atlas corruption). -let suggestedRendererType: "webgl" | "dom" | undefined = (() => { - try { - const pref = localStorage.getItem("terminal-renderer-type"); - if (pref === "dom" || pref === "webgl") return pref; - } catch {} - return undefined; -})(); +// Once WebGL fails, skip it for all subsequent terminals (VS Code pattern). +// Users can also set localStorage "terminal-renderer-type" to "dom" to +// permanently disable WebGL (workaround for GPU texture atlas corruption). +let suggestedRendererType: TerminalRendererType | undefined = + getStoredRendererPreference();(plus the matching change in
terminal-addons.ts).As per coding guidelines: "If a utility is used once, nest it under the parent's directory. If used 2+ times, promote it to the highest shared parent's directory or top-level
components/."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts` around lines 64 - 73, The IIFE that sets suggestedRendererType is duplicated and should be extracted into a shared helper; create a new exported function getStoredRendererPreference (returning "webgl" | "dom" | undefined) alongside the terminal utilities, move the localStorage read/validation logic there, and replace the IIFE in this file (and the matching one in terminal-addons.ts) by calling getStoredRendererPreference to set suggestedRendererType so parsing/validation lives in one place.apps/desktop/src/renderer/lib/terminal/terminal-addons.ts (1)
16-25: Same dedup applies here.This block is identical to the one in
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts(lines 67-73). Once the sharedgetStoredRendererPreference()helper is extracted (see comment on that file), this initializer should call it as well so both terminal init paths stay in lockstep on parsing/validation rules.Note: each file still keeps its own module-scoped
suggestedRendererTypefor the runtime WebGL-fallback latch, which is intentional — only the localStorage read needs to be shared.🤖 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 16 - 25, Extract the localStorage parsing logic into a shared helper getStoredRendererPreference() (as noted in the other file) and replace the module initializer for suggestedRendererType in this file so it calls that helper instead of duplicating the code; keep the module-level suggestedRendererType variable name and its type ("webgl" | "dom" | undefined) but set it by invoking getStoredRendererPreference(), ensuring the same validation rules (accept only "dom" or "webgl") are used in both places.
🤖 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 16-25: Extract the localStorage parsing logic into a shared helper
getStoredRendererPreference() (as noted in the other file) and replace the
module initializer for suggestedRendererType in this file so it calls that
helper instead of duplicating the code; keep the module-level
suggestedRendererType variable name and its type ("webgl" | "dom" | undefined)
but set it by invoking getStoredRendererPreference(), ensuring the same
validation rules (accept only "dom" or "webgl") are used in both places.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`:
- Around line 64-73: The IIFE that sets suggestedRendererType is duplicated and
should be extracted into a shared helper; create a new exported function
getStoredRendererPreference (returning "webgl" | "dom" | undefined) alongside
the terminal utilities, move the localStorage read/validation logic there, and
replace the IIFE in this file (and the matching one in terminal-addons.ts) by
calling getStoredRendererPreference to set suggestedRendererType so
parsing/validation lives in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1ad4bf5-2620-4e59-a2f5-b693a0a85135
📒 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}
"webgl" preference was functionally identical to undefined — the only guard checks for "dom", and if WebGL fails at runtime the catch block overwrites to "dom" regardless of localStorage. Simplify to only recognize "dom" and document the limitation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/lib/terminal/terminal-addons.ts (1)
16-25: Consider extracting the duplicated renderer-type initializer into a shared helper.The exact same IIFE (including the comment block) is duplicated in
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts(lines 64-73). Both v1 and v2 terminals will need to be kept in sync if the preference key, accepted values, or fallback logic ever changes (e.g., adding a settings UI as hinted in the PR objectives). A small shared util such asgetInitialSuggestedRendererType()co-located underapps/desktop/src/renderer/lib/terminal/would centralize the contract while preserving the independent module-levelsuggestedRendererTypevariables in each file.♻️ Example extraction
New file
apps/desktop/src/renderer/lib/terminal/renderer-preference.ts:// Users can set localStorage "terminal-renderer-type" to "dom" to permanently // disable WebGL (workaround for GPU texture atlas corruption). Only "dom" is // meaningful — any other value (including "webgl") behaves like the default. export function getInitialSuggestedRendererType(): "dom" | undefined { try { if (localStorage.getItem("terminal-renderer-type") === "dom") return "dom"; } catch {} return undefined; }Then in this file:
-// Once WebGL fails, skip it for all subsequent runtimes (VS Code pattern). -// Users can set localStorage "terminal-renderer-type" to "dom" to permanently -// disable WebGL (workaround for GPU texture atlas corruption). Only "dom" is -// meaningful — any other value (including "webgl") behaves like the default. -let suggestedRendererType: "webgl" | "dom" | undefined = (() => { - try { - if (localStorage.getItem("terminal-renderer-type") === "dom") return "dom"; - } catch {} - return undefined; -})(); +// Once WebGL fails, skip it for all subsequent runtimes (VS Code pattern). +let suggestedRendererType: "webgl" | "dom" | undefined = + getInitialSuggestedRendererType();🤖 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 16 - 25, The duplicated IIFE that initializes suggestedRendererType should be extracted into a single helper (e.g., getInitialSuggestedRendererType) colocated under the terminal lib and both modules should call that helper to initialize their module-level suggestedRendererType; keep the existing explanatory comment in the new helper, implement the function to return "dom" | undefined by reading localStorage with try/catch, and replace the IIFE in this file (symbol: suggestedRendererType) and the other helper file (the duplicate block in WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts) to call getInitialSuggestedRendererType().
🤖 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 16-25: The duplicated IIFE that initializes suggestedRendererType
should be extracted into a single helper (e.g., getInitialSuggestedRendererType)
colocated under the terminal lib and both modules should call that helper to
initialize their module-level suggestedRendererType; keep the existing
explanatory comment in the new helper, implement the function to return "dom" |
undefined by reading localStorage with try/catch, and replace the IIFE in this
file (symbol: suggestedRendererType) and the other helper file (the duplicate
block in WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts) to call
getInitialSuggestedRendererType().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 81223a31-f751-42c3-ac50-3127290e2e3c
📒 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
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
Summary
localStorage-based preference (terminal-renderer-type) to let users choose between WebGL and DOM rendering for the terminalhelpers.ts) and v2 (terminal-addons.ts) terminal initialization pathslocalStorage.setItem("terminal-renderer-type", "dom")in DevTools instead of launching the entire app with--disable-gpuProblem
The xterm.js WebGL addon's glyph texture atlas can silently become corrupted on some machines/GPUs, causing characters to render as wrong glyphs. The existing
onContextLossfallback only fires on complete context loss — partial texture corruption is not detected, so the DOM fallback never triggers.The only current workaround is
--disable-gpu, which disables all GPU acceleration (window compositing, CSS animations, etc.), which is too broad.Solution
Read a
localStoragepreference before initializingsuggestedRendererType. Ifterminal-renderer-typeis set to"dom", WebGL loading is skipped entirely. The default behavior (undefined→ try WebGL first) is unchanged for users not affected.This is a minimal, non-breaking change. A settings UI toggle can be added in a follow-up.
How to use
In DevTools console:
Then restart the app (or open a new terminal tab).
Test plan
localStorage.setItem("terminal-renderer-type", "dom")→ restart → confirm no garbled textlocalStorage.setItem("terminal-renderer-type", "webgl")→ restart → confirm WebGL is usedFixes #3794
🤖 Generated with Claude Code
Summary by cubic
Adds a
localStoragepreference to disable the terminal’s WebGL renderer so users with garbled text can force the DOM renderer without--disable-gpu. Default behavior stays the same for everyone else.localStoragekeyterminal-renderer-typeand honor"dom"to skip WebGL; any other value (including"webgl") is ignored.helpers.tsandterminal-addons.ts).xterm.jsWebGL glyph atlas corruption by using DOM when"dom"is set.Written for commit b4b59d5. Summary will update on new commits. Review in cubic
Summary by CodeRabbit