Skip to content

fix(terminal): smooth WebGL renderer transitions - #117

Merged
howdeploy merged 5 commits into
howdeploy:mainfrom
Yt4aZaveta:codex/fix-webgl-grid-refit
Oct 1, 2026
Merged

howdeploy merged 5 commits into
howdeploy:mainfrom
Yt4aZaveta:codex/fix-webgl-grid-refit

Conversation

@Yt4aZaveta

@Yt4aZaveta Yt4aZaveta commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

What changed

Terminal switches between the DOM and WebGL renderers no longer hide the entire .xterm element with opacity: 0.

The transition now keeps a snapshot of the last rendered frame while the new renderer is initialized and fitted. Once the new frame is ready, the snapshot fades out smoothly. The temporary layer is also cleaned up on WebGL errors, context loss, and teardown.

Validation

  • Full test suite on origin/main plus this patch: 1296 passed, 0 failed, 2 skipped;
  • npm run typecheck;
  • production build;
  • macOS startup smoke test and signed packaged app validation;
  • targeted renderer/session tests: 46 passed.

The PR is based on the current main, including merged PRs #114 and #115.

Copy link
Copy Markdown
Owner

Reviewed head 0683bc3202dae190ed1c07dbb5dc1a18916c083d against current main 9f5aec879a0fd557b019b2c07d63e5790410e931.

The snapshot approach addresses the blank interval during renderer changes. DOM snapshot styles are preserved: xterm's style elements are inside the cloned screen, and copying the renderer owner class keeps them applicable. Context-loss/error and unmount cleanup paths are also present.

P2: The timeout does not bound snapshot cleanup

In src/renderer/src/features/terminal/TerminalCard.tsx:315-327, the 250 ms fallback calls ready(), which clears the fallback and then waits for two animation frames. If ready() already scheduled a frame, the timeout cannot help either.

Consider minimizing the window after snapshot creation but before those frames run. Animation frames can be suspended while the window is hidden, so the overlay remains despite the intended fallback. On return, an old snapshot can cover output that has already advanced until the queued frames and fade finish. This is a lifecycle/cleanup gap identified from the code, not a locally reproduced UI failure.

Please make the fallback cleanup independent of another animation frame, or explicitly cancel the transition when the surface/window becomes hidden. Preserve paint-aligned fading for normal visible transitions. Electron's background throttling defaults to enabled, and animation frames may pause in hidden documents: Electron WebPreferences, requestAnimationFrame.

Validation and performance

The additions in tests/webgl-context-pool.test.mjs:349-350 only check that the source contains data-renderer-transition and requestAnimationFrame. They cannot detect a stale snapshot, missing cleanup, or an incorrect transition. Please add behavioral coverage for the normal reveal and the no-render/no-frame cleanup path, including cancellation on teardown. Document a hide/restore visual check.

At TerminalCard.tsx:669, new WebglAddon(true) enables preserveDrawingBuffer for the entire context lifetime, not just the snapshot interval. That makes sense for copying a previously painted canvas, but changes the ongoing rendering cost. Please verify the tradeoff with sustained output/multiple visible cards, or explain an alternative capture strategy. I have not measured a slowdown; this is a performance risk to validate.

Integration with current main

An actual Git merge rehearsal finds a conflict in TerminalCard.tsx. Keep both the transition state and main's surface lifecycle/pinned-input refresh state. Do not replace the component with the older branch version: retain full-stream parsing for hidden terminals, the selection redraw guard, and coalesced pinned-input updates.

CI is green for this head, but the branch predates the recent main merges. Please resolve against current main and rerun CI on the resulting head.

Review method: source tracing through xterm/WebGL and Git merge checks. No local tests, benchmarks, or UI checks were run.

@Yt4aZaveta

Copy link
Copy Markdown
Contributor Author

Update after syncing the branch with the current main:

  • Resolved the merge conflict while preserving the current surface lifecycle, hidden-terminal output handling, selection redraw guard, and coalesced pinned-input updates.
  • Bounded renderer-transition cleanup with an independent 250 ms fallback.
  • Added cancellation/cleanup for hidden documents, WebGL errors and context loss, suspension, and unmount.
  • Added behavioral regression coverage for normal reveal and no-frame/teardown paths, plus a manual hide/restore guide.

Validation completed locally: renderer-transition tests, typecheck, production build, Go vet/tests, native-helper checks, macOS packaging and signature verification, packaged CLI resolution, and hidden-terminal/browser smoke checks passed. The available manual hide/restore and WebGL preserve-drawing-buffer spot checks showed no visible regression; a sustained performance benchmark was not part of this validation.

PR #118 separately fixes the resumed OpenCode session.status:busy lifecycle issue. This branch was validated against main before that fix landed; after #118 merges, this PR should be synced once more and CI rerun so the runtime check is evaluated on the final combined main.

Copy link
Copy Markdown
Owner

Follow-up review of 9104e1eeaab7e041e13fa4e6e333eca43108b0f8:

The reported snapshot cleanup issue is addressed. The 250 ms deadline now calls cleanup directly and remains independent of animation frames. Document visibility and non-live surface transitions also cancel the snapshot. The normal reveal, no-render/no-frame deadline, visibility, teardown, and reduced-motion behavioral checks all passed in the Linux and Windows CI logs. The previous merge conflict is resolved, with main's lifecycle and pinned-input state retained.

The current Linux verify failure is tests/agent-runtime-gateway.test.mjs:430, where the resumed OpenCode session misses working/session.status:busy. The same failure is present on current main's run 36877121830, and PR #118 targets it. Windows and macOS jobs are green for this head.

Please sync again after #118 lands and obtain green CI on the resulting head, as you proposed. The original cleanup and behavioral-test findings are closed.

Your explanation for preserveDrawingBuffer is reasonable, and the manual spot checks are useful. Its sustained rendering cost is still unmeasured; that remains a performance validation limitation, not a measured regression.

Source/CI-log review only; no local tests, benchmark, or UI checks were run.

@howdeploy
howdeploy merged commit b322234 into howdeploy:main Oct 1, 2026
3 checks passed
@Yt4aZaveta
Yt4aZaveta deleted the codex/fix-webgl-grid-refit branch October 3, 2026 06:54
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.

2 participants