Repository navigation
Conversation
…enderer fallback per terminal Every terminal ever opened held a live WebGL context while parked off-screen, and Chromium caps contexts at ~16 per process — past that it silently evicts one, which tripped a module-global latch that downgraded every future terminal to the slow DOM renderer for the rest of the session. Under agent streaming the DOM renderer saturates the UI thread and typed characters render late. - New shared webgl-renderer.ts controller: acquire the WebGL addon on attach, release on park. Parked terminals pause rendering anyway (xterm IntersectionObserver), so live contexts now equal visible panes. - Context loss falls back to DOM for that terminal only and retries WebGL on the next attach; load failure disables WebGL per instance, never globally. - Fallbacks are logged and tracked (terminal_webgl_fallback) instead of silent. - v1 (helpers.ts/v1-terminal-cache.ts) and v2 (terminal-runtime.ts) now share the one controller instead of duplicating the WebGL block.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR extracts WebGL renderer lifecycle management from ChangesWebGL renderer controller extraction and wiring
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalCache as v1-terminal-cache
participant Runtime as terminal-runtime
participant Controller as WebglRendererController
participant Addon as WebglAddon
participant XTerm
TerminalCache->>Runtime: attachToContainer(container)
Runtime->>Controller: acquire()
Controller->>Controller: requestAnimationFrame(createAddon)
Controller->>Addon: activate(xterm)
Addon-->>Controller: register onContextLoss
alt context loss
Addon--)Controller: context-loss event
Controller->>Addon: dispose()
Controller->>XTerm: refresh (fallback to DOM)
end
TerminalCache->>Runtime: detachFromContainer()
Runtime->>Controller: release()
Controller->>Addon: dispose() (if active)
TerminalCache->>Runtime: disposeRuntime()
Runtime->>Controller: dispose()
Controller->>Addon: dispose()
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Ready to review this PR? Stage has broken it down into 4 individual chapters for you:
Chapters generated by Stage for commit 595a3b8 on Jul 3, 2026 3:34am UTC. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/lib/terminal/webgl-renderer.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/terminal/webgl-renderer.ts:53">
P2: WebGL cleanup and telemetry failures can become invisible because several new catch blocks swallow errors without logging or an explicit best-effort rationale. Adding lightweight warning context (or an inline comment where silence is intentional) would keep fallback paths debuggable without changing behavior.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as Terminal UI Pane
participant V1Cache as v1-terminal-cache
participant V2Runtime as Terminal Runtime (v2)
participant Controller as WebglRendererController
participant XTerm as xterm.js Terminal
participant GPUCtx as WebGL Context (GPU)
Note over UI,GPUCtx: Terminal Lifecycle – Attach/Park/Dispose
rect rgb(240, 248, 255)
Note over UI,GPUCtx: ATTACH FLOW
UI->>V1Cache: attachToContainer(paneId)
V1Cache->>Controller: acquire()
Controller->>Controller: requestAnimationFrame (deferred load)
Controller->>XTerm: loadAddon(WebglAddon)
XTerm->>GPUCtx: create WebGL context
GPUCtx-->>XTerm: context active
XTerm-->>Controller: addon active
Controller-->>V1Cache: (idempotent, no return)
V1Cache->>UI: appendChild(wrapper)
UI->>XTerm: visible, IntersectionObserver fires
end
rect rgb(255, 245, 238)
Note over UI,GPUCtx: PARK FLOW (off-screen → terminal-parking)
UI->>V1Cache: detachFromContainer(paneId)
V1Cache->>Controller: release()
Controller->>GPUCtx: dispose() WebGL context
GPUCtx-->>Controller: context destroyed
Controller-->>V1Cache: (no pending load)
V1Cache->>XTerm: remove from DOM
XTerm->>XTerm: IntersectionObserver pauses rendering
end
alt Context Loss While Attached
GPUCtx->>XTerm: onContextLoss event
XTerm->>Controller: context loss callback
Controller->>GPUCtx: dispose() failed context
Controller->>Controller: mark fallback "context-loss"
Controller->>XTerm: refresh(0, rows-1) → DOM renderer
Controller->>Controller: console.warn + track("terminal_webgl_fallback")
Note over Controller: Next acquire() retries WebGL
end
alt WebGL Load Failure (no GPU support)
Controller->>Controller: createAddon() throws
Controller->>Controller: mark loadFailed = true (per-instance)
Controller->>Controller: console.warn + track("terminal_webgl_fallback")
Note over Controller: All future acquires() skip WebGL for this terminal
UI->>V2Runtime: attachToContainer()
V2Runtime->>Controller: acquire()
Controller->>Controller: loadFailed → return immediately
XTerm->>XTerm: stays on DOM renderer
Note over V1Cache: Other terminals unaffected (no global latch)
end
rect rgb(240, 248, 255)
Note over UI,GPUCtx: DISPOSE FLOW (terminal destroyed)
UI->>V2Runtime: disposeRuntime()
V2Runtime->>Controller: dispose()
Controller->>Controller: set disposed flag
Controller->>GPUCtx: release() → cancel pending + dispose context
GPUCtx-->>Controller: destroyed
Controller-->>V2Runtime: (blocked from future acquires)
end
Note over V1Cache,V2Runtime: v1 and v2 both use same controller instance
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,129 @@ | |||
| import { WebglAddon } from "@xterm/addon-webgl"; | |||
Contributor
There was a problem hiding this comment.
P2: WebGL cleanup and telemetry failures can become invisible because several new catch blocks swallow errors without logging or an explicit best-effort rationale. Adding lightweight warning context (or an inline comment where silence is intentional) would keep fallback paths debuggable without changing behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/lib/terminal/webgl-renderer.ts, line 53:
<comment>WebGL cleanup and telemetry failures can become invisible because several new catch blocks swallow errors without logging or an explicit best-effort rationale. Adding lightweight warning context (or an inline comment where silence is intentional) would keep fallback paths debuggable without changing behavior.</comment>
<file context>
@@ -0,0 +1,129 @@
+ // common path where WebGL never fails.
+ void import("renderer/lib/analytics")
+ .then(({ track }) => track("terminal_webgl_fallback", { reason }))
+ .catch(() => {});
+}
+
</file context>
Contributor
Greptile SummaryThis PR fixes terminal typing lag caused by Chromium's ~16 WebGL context cap being exhausted when many terminals were opened. Previously every terminal held a live WebGL context while parked off-screen, and context eviction triggered a module-global latch that permanently downgraded every future terminal to the slow DOM renderer.
Confidence Score: 4/5Safe to merge — the acquire/release state machine is well-guarded and the two minor findings are defensive-coding gaps rather than incorrect behaviour. The controller's idempotency guards, rAF cancellation, and per-instance load-failure isolation all work correctly. The only findings are: the IDisposable returned by onContextLoss is discarded (xterm cleans it up on dispose, but holding it would be more defensive), and a test for dispose() while a rAF is still pending is absent (though the code handles it correctly via the existing cancelAnimationFrame path). webgl-renderer.ts (context-loss IDisposable handling) and webgl-renderer.test.ts (pending-rAF dispose path)
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/lib/terminal/webgl-renderer.ts | New controller that acquires/releases the WebGL addon per attach/park cycle; well-guarded state machine with load-failure isolation and context-loss retry, with one minor defensive gap (IDisposable not stored) |
| apps/desktop/src/renderer/lib/terminal/webgl-renderer.test.ts | 7 deterministic unit tests covering the main lifecycle paths; missing explicit coverage for dispose() called while a rAF is still pending |
| apps/desktop/src/renderer/lib/terminal/terminal-runtime.ts | Replaced _disposeAddons with webglController; acquire() placed correctly before the same-container idempotency check to enable in-place context-loss recovery |
| apps/desktop/src/renderer/lib/terminal/terminal-addons.ts | Removed WebGL loading block and the module-global suggestedRendererType latch; dispose removed from LoadAddonsResult since WebGL is now owned by the controller |
| apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts | Removed duplicate module-global suggestedRendererType latch and inline WebGL setup; now creates webglController and threads it through to the caller |
| apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts | Correctly threads webglController into CachedTerminal, calls acquire() in attachToContainer and release() in detachFromContainer; dispose path covers cleanup via cleanupCreation() |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant C as Caller (attach/detach)
participant W as WebglRendererController
participant rAF as requestAnimationFrame
participant A as WebglAddon
participant X as xterm Terminal
Note over C,X: Terminal becomes visible (attach)
C->>W: acquire()
W->>rAF: schedule frame (frameId set)
rAF-->>W: frame fires
W->>A: createAddon()
W->>A: onContextLoss(handler)
W->>X: loadAddon(next) → switches to WebGL renderer
W->>W: "addon = next"
Note over C,X: Terminal parked (detach)
C->>W: release()
W->>rAF: cancelAnimationFrame (if pending)
W->>A: dispose() → xterm falls back to DOM renderer
W->>W: "addon = null"
Note over C,X: GPU context eviction (while attached)
A-->>W: onContextLoss fires
W->>A: dispose()
W->>W: "addon = null"
W->>X: refresh() — DOM renderer repaints
Note over W: loadFailed stays false → retry on next acquire()
Note over C,X: Load failure (no GPU support)
W->>A: createAddon() throws
W->>W: "loadFailed = true"
Note over W: All future acquire() calls are no-ops for this controller only
Note over C,X: Terminal disposed
C->>W: dispose()
W->>W: "disposed = true"
W->>A: dispose() via release()
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant C as Caller (attach/detach)
participant W as WebglRendererController
participant rAF as requestAnimationFrame
participant A as WebglAddon
participant X as xterm Terminal
Note over C,X: Terminal becomes visible (attach)
C->>W: acquire()
W->>rAF: schedule frame (frameId set)
rAF-->>W: frame fires
W->>A: createAddon()
W->>A: onContextLoss(handler)
W->>X: loadAddon(next) → switches to WebGL renderer
W->>W: "addon = next"
Note over C,X: Terminal parked (detach)
C->>W: release()
W->>rAF: cancelAnimationFrame (if pending)
W->>A: dispose() → xterm falls back to DOM renderer
W->>W: "addon = null"
Note over C,X: GPU context eviction (while attached)
A-->>W: onContextLoss fires
W->>A: dispose()
W->>W: "addon = null"
W->>X: refresh() — DOM renderer repaints
Note over W: loadFailed stays false → retry on next acquire()
Note over C,X: Load failure (no GPU support)
W->>A: createAddon() throws
W->>W: "loadFailed = true"
Note over W: All future acquire() calls are no-ops for this controller only
Note over C,X: Terminal disposed
C->>W: dispose()
W->>W: "disposed = true"
W->>A: dispose() via release()
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/lib/terminal/webgl-renderer.test.ts:187-198
**Missing test: `dispose()` while a rAF is still pending.** Test 3 covers `release()` cancelling a pending rAF, and test 7 covers `dispose()` after the addon is active, but `dispose()` called in the window between `acquire()` and `fireFrame()` is not exercised. Transitively this works (dispose calls release, which calls cancelAnimationFrame), but an explicit case would document that `dispose()` while pending blocks the rAF, sets `disposed = true`, and prevents any subsequent `acquire()` from re-scheduling it.
### Issue 2 of 2
apps/desktop/src/renderer/lib/terminal/webgl-renderer.ts:97-110
The IDisposable returned by `next.onContextLoss(...)` is silently discarded. While xterm's `WebglAddon.dispose()` does remove the listener, if cleanup is ever incomplete the stale callback would fire after an intentional `release()` — producing a spurious `console.warn` and `terminal_webgl_fallback` PostHog event. Holding the handle and self-disposing it inside the callback is the minimal defensive fix.
```suggestion
const contextLossDisposable = next.onContextLoss(() => {
// GPU reset or context eviction. Drop to the DOM renderer for
// this terminal; the next attach retries WebGL.
contextLossDisposable?.dispose();
if (addon === next) {
addon = null;
}
try {
next.dispose();
} catch {}
onFallback("context-loss");
if (!disposed) {
terminal.refresh(0, Math.max(0, terminal.rows - 1));
}
});
```
Reviews (1): Last reviewed commit: "fix(desktop): bound terminal WebGL conte..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+187
to
+198
| test("dispose releases the context and blocks further acquires", () => { | ||
| const { controller, created } = createHarness(); | ||
|
|
||
| controller.acquire(); | ||
| fireFrame(); | ||
| controller.dispose(); | ||
| expect((created[0] as FakeAddon).disposeCount).toBe(1); | ||
|
|
||
| controller.acquire(); | ||
| fireFrame(); | ||
| expect(created).toHaveLength(1); | ||
| }); |
Contributor
There was a problem hiding this comment.
dispose() while a rAF is still pending. Test 3 covers release() cancelling a pending rAF, and test 7 covers dispose() after the addon is active, but dispose() called in the window between acquire() and fireFrame() is not exercised. Transitively this works (dispose calls release, which calls cancelAnimationFrame), but an explicit case would document that dispose() while pending blocks the rAF, sets disposed = true, and prevents any subsequent acquire() from re-scheduling it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/webgl-renderer.test.ts
Line: 187-198
Comment:
**Missing test: `dispose()` while a rAF is still pending.** Test 3 covers `release()` cancelling a pending rAF, and test 7 covers `dispose()` after the addon is active, but `dispose()` called in the window between `acquire()` and `fireFrame()` is not exercised. Transitively this works (dispose calls release, which calls cancelAnimationFrame), but an explicit case would document that `dispose()` while pending blocks the rAF, sets `disposed = true`, and prevents any subsequent `acquire()` from re-scheduling it.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+97
to
+110
| next.onContextLoss(() => { | ||
| // GPU reset or context eviction. Drop to the DOM renderer for | ||
| // this terminal; the next attach retries WebGL. | ||
| if (addon === next) { | ||
| addon = null; | ||
| } | ||
| try { | ||
| next.dispose(); | ||
| } catch {} | ||
| onFallback("context-loss"); | ||
| if (!disposed) { | ||
| terminal.refresh(0, Math.max(0, terminal.rows - 1)); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
next.onContextLoss(...) is silently discarded. While xterm's WebglAddon.dispose() does remove the listener, if cleanup is ever incomplete the stale callback would fire after an intentional release() — producing a spurious console.warn and terminal_webgl_fallback PostHog event. Holding the handle and self-disposing it inside the callback is the minimal defensive fix.
Suggested change
| next.onContextLoss(() => { | |
| // GPU reset or context eviction. Drop to the DOM renderer for | |
| // this terminal; the next attach retries WebGL. | |
| if (addon === next) { | |
| addon = null; | |
| } | |
| try { | |
| next.dispose(); | |
| } catch {} | |
| onFallback("context-loss"); | |
| if (!disposed) { | |
| terminal.refresh(0, Math.max(0, terminal.rows - 1)); | |
| } | |
| }); | |
| const contextLossDisposable = next.onContextLoss(() => { | |
| // GPU reset or context eviction. Drop to the DOM renderer for | |
| // this terminal; the next attach retries WebGL. | |
| contextLossDisposable?.dispose(); | |
| if (addon === next) { | |
| addon = null; | |
| } | |
| try { | |
| next.dispose(); | |
| } catch {} | |
| onFallback("context-loss"); | |
| if (!disposed) { | |
| terminal.refresh(0, Math.max(0, terminal.rows - 1)); | |
| } | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/webgl-renderer.ts
Line: 97-110
Comment:
The IDisposable returned by `next.onContextLoss(...)` is silently discarded. While xterm's `WebglAddon.dispose()` does remove the listener, if cleanup is ever incomplete the stale callback would fire after an intentional `release()` — producing a spurious `console.warn` and `terminal_webgl_fallback` PostHog event. Holding the handle and self-disposing it inside the callback is the minimal defensive fix.
```suggestion
const contextLossDisposable = next.onContextLoss(() => {
// GPU reset or context eviction. Drop to the DOM renderer for
// this terminal; the next attach retries WebGL.
contextLossDisposable?.dispose();
if (addon === next) {
addon = null;
}
try {
next.dispose();
} catch {}
onFallback("context-loss");
if (!disposed) {
terminal.refresh(0, Math.max(0, terminal.rows - 1));
}
});
```
How can I resolve this? If you propose a fix, please make it concise.This branch has not been deployed
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
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.
Summary
Typing echo lagged in terminal panes (v1 and v2) once enough terminals had been opened. Root cause: every terminal ever opened held a live WebGL context while parked off-screen, and Chromium caps a renderer process at ~16 contexts — past that it silently evicts one, which tripped a module-global fallback latch (
suggestedRendererType) that downgraded every future terminal to the slow DOM renderer for the rest of the session. Under agent streaming the DOM renderer saturates the UI thread, so typed characters render late.webgl-renderer.tscontroller: the WebGL addon is acquired on attach and released on park. Parked terminals pause rendering anyway (xterm's IntersectionObserver), so live GPU contexts now equal visible panes — permanently under the cap.console.warn+terminal_webgl_fallbackPostHog event.helpers.ts/v1-terminal-cache.ts) and v2 (terminal-runtime.ts) now share the one controller instead of duplicating the WebGL loading block.Test Plan
tsc --noEmitandbun run lintclean<canvas>,#terminal-parkingterminals hold no WebGL contexts, typing stays crisp while agents stream in background tabsSummary by cubic
Fixes terminal typing lag by preventing WebGL context exhaustion. WebGL is now acquired only for visible terminals and released when parked, keeping contexts under Chromium’s cap and preserving the fast renderer.
@xterm/addon-webgl: acquire on attach, release on park.terminal_webgl_fallback.Written for commit 595a3b8. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes