Skip to content

perf(terminal): re-enable WebGL rendering on the xterm beta line - #873

Merged
Juliusolsson05 merged 11 commits into
mainfrom
perf/reenable-xterm-webgl
Sep 11, 2026
Merged

Juliusolsson05 merged 11 commits into
mainfrom
perf/reenable-xterm-webgl

Conversation

@Juliusolsson05

@Juliusolsson05 Juliusolsson05 commented Sep 11, 2026 •

Copy link
Copy Markdown
Owner

TL;DR

Turns xterm's WebGL renderer back on by pinning the 0.20.0 beta line (the only one with the atlas-corruption fix), and locally patches a real upstream bug in that beta's core that would otherwise duplicate/drop terminal output on resize. Everything is exact-pinned, guarded by a test that fails if the patch goes missing, and documented where the next person bumping xterm will look.

Problem

What this does

  1. Exact pins (a caret on a prerelease would float to a newer beta on the next lockfile refresh and move the core out from under the patch): @xterm/xterm 6.1.0-beta.304, @xterm/addon-webgl 0.20.0-beta.300 (both = VS Code's lockfile), @xterm/addon-fit 0.12.0-beta.301 (its peer range requires the new core). Lockfile changes only those three entries.
  2. Local core patch — scripts/patch-xterm.mjs, run by postinstall AND at electron.vite.config.ts evaluation (so installs that skipped lifecycle scripts cannot bundle the unpatched core; the renderer also excludes @xterm/xterm from Vite's dev pre-bundle so a pre-patch cache can never be served): removes this._writeBuffer.flushSync() from resize() in both shipped bundles, restoring stable 6.0.0 resize semantics. The maintainer-endorsed narrow form rather than Superset's larger flushSync/_innerWrite rewrite (that targets an async image-decoder case we don't have; src/ registers no parser handlers). The script edits one exact string, verifies the installed version against the pin, is idempotent and self-verifying, and fails the install loudly on any other version or bundle shape. A script rather than patch-package because the bundles are minified (one 391 KB line) — a diff would be about 1.1 MB of unreviewable patch across both bundles. Its header is the documentation: the bug, the trigger, the app impact, and exactly what to do on a bump (including when upstream fixes it).
  3. Guard test — xtermResizeFlushPatch.test.ts: patch present in both bundles + three deterministic cases (resize inside a write callback; empty write + resize; resize after the writer yielded), run against the real installed lib/xterm.mjs and lib/xterm.js. All 7 fail on the unpatched beta, pass patched; the three behaviours pass on 6.0.0.
  4. WebGL switch on (WEBGL_RENDERER_ENABLED = true, still the one-line rollback for renderer issues — it does NOT roll back the core patch). The 0.19.0 atlas bridge is deleted.
  5. Refit on renderer change: WebGL floors cell width, DOM keeps it fractional, so a context-loss fallback kept a grid wider than the pane (clipped). attachXtermWebglRenderer(term, { onRendererChange }) fires after WebGL takes over and after a real fallback; all three hosts route it into their existing ownership-gated fit.

Review rounds

Round 2 (two orchestrated reviewers: A runtime/Codex, B docs+tests/Codex; B's original Claude session hit the account spend limit and was replaced on the same brief)

  • A — no correctness defect in the patch or the refit. Removing the flush is safe on beta.304: upstream's separate resize-during-write fix (#5598) is present, and nothing newer relies on resize draining the queue. Patched stress run: 650 resizes, 1,126 callbacks, each fired exactly once. It found two enforcement gaps; both were reproduced and fixed in b00fc5d1:
    • installs that skip lifecycle scripts bundled the unpatched core → electron.vite.config.ts now runs the idempotent patch at config evaluation;
    • Vite's dev pre-bundle cache is keyed on the lockfile, not file contents → the renderer excludes @xterm/xterm from optimizeDeps.
  • B1 Minor: the removal procedure didn't mention the config gate or the test's source read, so following it would have caused ENOENT in dev/build and in test collection. It now lists all five pieces (18ba1932).
  • B2 Minor: refit behaviour was only tested on the inline host. There are now behavioural cases for AgentTerminalLeaf and TerminalLeaf plus a non-owner "must not resize" case. They are mutation-checked: no-op wiring fails the refit cases, and a fence-bypassing callback fails the non-owner case (6bf3e648).
  • B3 Minor: patch-package size corrected to about 1.1 MB across both bundles.

Round 1 (A runtime/Codex, B integration/Claude)

  • A1 Critical — the core resize bug → item 2 + 3. Independently confirmed by source reading, a web/GitHub research pass (upstream timeline, VS Code stable 1.109–1.137 ship affected betas), and differential integration runs.
  • A2 Important — context-loss refit → item 5.
  • B1 Important — the Electron pixel harness (scripts/smoke-terminal-renderer.mjs) required --control to reproduce corruption, guaranteed to fail on the fixed addon; both modes now must render clean.
  • B2–B4 Minor — header no longer claims VS Code ships addon-fit; lockfile numbers corrected; the two TerminalLeaf suites mock the renderer module. B5 (upstream-watch entry for xterm) not taken — the watcher is provider-CLI specific; the patch script's version guard is the trigger on any bump.

Linked issues

Fixes #871
Refs #768, #789, #841, xtermjs/xterm.js#6154

Verification

Local, Node 24, isolated worktree node_modules:

  • Fresh npm ci --include=dev — exit 0; postinstall applied the patch to both bundles (what CI and packaging do).
  • npm run typecheck — exit 0. npm run test:contract — ok.
  • Guard + terminal unit suites 30/30; host renderer suites (AgentTerminalLeaf.*, TerminalLeaf.*) 36/36 at round 1; after round 2 the ownership suite is 13/13 with renderer-change refit cases for all three hosts plus a non-owner case (mutation-checked); agentTerminalFollow.system 1/1; xtermWebglRenderer.test.ts 15/15 (refit cases fail before the implementation). npm run typecheck re-run after round 2 — exit 0.
  • Integration evidence (scratchpad harnesses, not committed): differential Node stress runs (6.0.0 vs beta vs patched beta), the real-forwarder attach simulation (numbers above), and upstream research.
  • CI quality-gate (full suites, coverage, test:package production build) — see checks.

Before merging: manual check (tests cannot see pixels; agents don't launch the app)

  1. Optional objective pre-check: node scripts/smoke-terminal-renderer.mjs and … --control — both should exit 0 (differences: [], referenceDifference: 0). Launches a throwaway Electron window, not the app.
  2. Run the branch; push heavy Claude Code / Codex output in terminal view, OpenCode Terminal, and a plain shell; resize panes while output streams, switch lanes, open a new pane on a busy agent, sleep/wake.
  3. Watch for garbled glyphs, repeated output after resizes, a pane that stops updating. If glyphs look wrong: copy the text — clean clipboard = renderer (set WEBGL_RENDERER_ENABLED = false); dirty = data path.

Risks / follow-ups

🤖 Generated with Claude Code

https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k

Juliusolsson05 and others added 3 commits September 11, 2026 09:49
The WebGL renderer has been off since 2026-09-08 because the stable
addon-webgl 0.19.0 corrupts its texture atlas and the upstream fix exists
only on the 0.20.0 beta line. VS Code ships exactly that beta line, which
removes the main argument for waiting on a stable release. The plan records
the exact pins, the worktree-isolated install, and the manual soak gate.

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
WebGL has been off since 2026-09-08 because the stable addon-webgl 0.19.0
corrupts its texture atlas under provider TUI output, and the upstream fix
(xterm.js #5883) exists only on the 0.20.0 beta line, which peer-requires a
beta @xterm/xterm. VS Code depends on exactly these builds, so waiting for a
stable release was costing every raw terminal the DOM renderer's per-row
rebuilds without buying real safety.

Pins @xterm/xterm 6.1.0-beta.304, @xterm/addon-webgl 0.20.0-beta.300 and
@xterm/addon-fit 0.12.0-beta.301 exactly, because a caret on a prerelease
would let an unrelated lockfile refresh move the core terminal. The switch
is on again and remains the one-line rollback. The 0.19.0 atlas bridge,
which reached into private renderer fields, is deleted with its tests, as
the switch's comment instructed.

The lockfile changes only the three xterm entries: npm's own rewrite also
pruned vitest's nested esbuild, an unrelated dedupe that is not shipped.

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
…choice

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Merge blocked — converted to draft. Independent review (orchestrated reviewer A, runtime) found a critical regression in the pinned core beta, reproduced and independently confirmed:

  • @xterm/xterm 6.1.0-beta.304 duplicates already-parsed output — and fires the first write callback twice — when the terminal resizes between write batches. Upstream commit bf7c95b6 ("Flush writes on resize", 2026-01-09) added flushSync() to resize; it ignores WriteBuffer._bufferOffset, so a partially consumed queue (heavy output exceeding the 12 ms parse budget) is re-parsed from index 0.
  • Minimal repro (Node, no DOM): write FIRST whose callback exceeds the budget and schedules resize(81, 24), then write SECOND. 6.0.0 → FIRST, SECOND, callbacks 1/1. beta.304 → FIRST, FIRST, SECOND, callbacks 2/1.
  • It is in core, independent of WebGL, so the renderer switch cannot roll it back. Heavy provider output + frequent pane/lane resizes is exactly this app's workload, and terminalInputForwarder's replay latch assumes exactly-once write callbacks.
  • No safe beta exists: the regression (2026-01-09) predates the atlas fix #5883 (2026-05-21), so every beta that contains the WebGL fix also contains it. Later WriteBuffer.ts commits (#5912 fix, dispose cleanup, 2026-05/06) do not address the offset. A sibling issue from the same change is open upstream: resize() while an async parser handler is pending leaves the parser in FAIL; terminal stops rendering xtermjs/xterm.js#6154. The duplicate-output case does not appear to be reported upstream yet.

Second finding (Important, our code): on WebGL context loss the fallback keeps the WebGL grid (WebGL floors cell width, DOM keeps it fractional) and no host refits, so text can be clipped until the next layout change. Must be fixed before WebGL is re-enabled on any version.

main is unaffected (core 6.0.0, WebGL off). Decision on the path forward is with the user; details in #871.

Juliusolsson05 and others added 8 commits September 11, 2026 11:26
…rites

The pinned @xterm/xterm 6.1.0 beta calls WriteBuffer.flushSync() from
resize() (upstream bf7c95b6, xterm.js #5599). flushSync drains from index 0
with a truthiness loop, so it re-applies chunks the async writer already
parsed (duplicate output, write callbacks fired twice) and stops at an empty
chunk, discarding everything queued behind it. It is open upstream as part
of xterm.js #6154 and predates the atlas fix, so no beta avoids it.

Pane attach replays one chunk of up to 512 KiB and then re-fits, which is
the trigger: with the real terminalInputForwarder, 7 of 20 simulated attaches
leaked terminal query replies into the agent's stdin on the unpatched beta,
0 on 6.0.0 and 0 patched.

A postinstall script removes the flush from resize() in both shipped bundles,
restoring 6.0.0 resize semantics as the upstream maintainer suggests. It
edits one exact string, verifies the installed version against the exact pin,
and fails the install loudly on anything else; its header documents the bug
and what to do on a bump. A script rather than patch-package because the
bundles are minified and a diff would be about 0.8 MB. The guard test fails
on the unpatched beta and passes on 6.0.0 and patched, in both bundles.

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
WebGL floors the device cell width while the DOM renderer keeps it
fractional, so the same columns occupy a different width on each. xterm's
fallback after a WebGL context loss keeps the current grid, so text could
be clipped by the host's overflow-hidden, and the ResizeObserver never
notices because the container did not change size.

attachXtermWebglRenderer now takes an options object with onRendererChange,
fired after WebGL takes over and after a real fallback (never when disabled,
failed, or after teardown), and each host routes it into its existing
coalesced, ownership-gated fit. The renderer header also records that the
pinned core is locally patched and no longer claims VS Code ships addon-fit.

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
… addon

With WebGL on, every TerminalLeaf mount in these two suites ran the real
renderer wrapper and evaluated the real addon bundle in happy-dom, coverage
they never assert on and whose failures the wrapper's fallback swallows.
They now mock the renderer module like the AgentTerminalLeaf suites do.

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
…smoke run

The Electron pixel harness required --control (the bare addon) to reproduce
atlas corruption, which held on addon-webgl 0.19.0 but is guaranteed to fail
on the fixed 0.20.0 beta line. Both modes now must render clean, so a
corrupt control reports a regression in the pinned addon.

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
…e plan

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
…pre-bundle

Round-2 review reproduced two ways to run the unpatched core despite the
postinstall patch. An install that skips lifecycle scripts leaves both
bundles unpatched and dev/build bundle them without error. And Vite's dev
optimizer caches a pre-bundled xterm keyed on the lockfile rather than file
contents, so a copy pre-bundled before patching keeps being served.

electron.vite.config.ts now runs the idempotent patch script at config
evaluation, covering every electron-vite dev/build/preview including direct
invocations and aborting on a version or bundle mismatch. The renderer
excludes @xterm/xterm from optimizeDeps so dev always serves the patched
file; the bundle is plain ESM with no imports, so nothing else changes.

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
Round-2 review of PR #873 found only the inline host's onRendererChange
was exercised behaviourally; AgentTerminalLeaf and TerminalLeaf were
checked only for "some function was passed". Each has its own scheduler
and ownership/visibility fence, so a no-op or fence-bypassing callback
would have passed.

- Parameterized case: invoking the captured callback on the agent pane
  and the shell pane coalesces into one frame, one fit, and one resize
  with the new cols sent to that session.
- Non-owner case: an agent pane that lost dimension ownership to the
  fullscreen editor's inline terminal neither fits nor resizes.

Mutation-checked: no-op wiring fails the two refit cases; a callback
that skips the ownership fence fails the non-owner case.

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
Round-2 review of PR #873: the "WHEN THIS FAILS" checklist predated the
electron.vite.config gate added in b00fc5d, so following it after an
upstream fix would have left dev/build executing a deleted script and
the guard test reading it at collection time (both ENOENT).

- Removal (a) now names all five pieces: the script, postinstall, the
  config gate plus the optimizeDeps exclusion, the test's source read
  and patch-presence case (keeping the three behaviour cases), and the
  renderer header paragraph.
- patch-package size corrected to about 1.1 MB across both bundles
  (782 KB CJS + 350 KB ESM), not 0.8 MB.
- Plan records review round 2 and how it was resolved.

Refs #871

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVRSHn7c7qnBoUQo1e2X4k
@Juliusolsson05
Juliusolsson05 marked this pull request as ready for review September 11, 2026 21:41
@Juliusolsson05
Juliusolsson05 merged commit 6a19e4e into main Sep 11, 2026
2 checks passed
@Juliusolsson05
Juliusolsson05 deleted the perf/reenable-xterm-webgl branch September 11, 2026 21:41
Juliusolsson05 added a commit that referenced this pull request Sep 12, 2026
…undary

Brings the wheel-boundary fix (#791) onto current main, 379 commits ahead,
including the xterm 6.1.0-beta.304 / WebGL re-enable (#873), terminal
session parity (#872) and OpenCode Terminal (#882).

Conflicts, all at the three xterm hosts:
- AgentInlineTerminal.tsx: main moved the WebGL attach below `scheduleFit`
  (onRendererChange); keep only the boundary attach right after open().
- AgentTerminalLeaf.tsx: imports - keep the boundary import, drop
  AgentTitleHeader (removed on main in favor of PaneHeader); body - boundary
  attach after open(), then main's onRendererChange WebGL attach.
- TerminalLeaf.tsx: imports - keep both sides; body - same as above.

Re-checked against the installed beta: the scrollable element still only
consumes a wheel it scrolled (alwaysConsumeMouseWheel and
consumeMouseWheelIfScrollbarIsNeeded default false, Viewport sets neither),
and MouseService's passive wheel handler returns without preventDefault
while the buffer has scrollback, so the boundary leak persists and the
bubbling host listener remains the right place to cancel it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S4AoJJUBex14FAEZZfHPsV
Juliusolsson05 added a commit that referenced this pull request Sep 12, 2026
Main moved xterm to exact-pinned 6.1.0-beta.304 (#873). A version bump is
exactly when a helper like this silently becomes redundant or wrong, so the
WHY now cites where the beta's wheel listeners live, when they consume, and
what to re-check (and when to delete the helper) on the next bump. Records
the re-validation and its tsc/test results in the branch plan.

Refs #791

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S4AoJJUBex14FAEZZfHPsV
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(terminal): re-enable WebGL rendering by moving xterm to the beta line VS Code ships

1 participant