Skip to content

fix(terminal): smooth pixel-level scrolling for TUI apps via patched xterm viewport - #7450

Open
hmrserver wants to merge 4 commits into
stablyai:mainfrom
hmrserver:hmrserver/smooth-scroll-for-tui-default
Open

fix(terminal): smooth pixel-level scrolling for TUI apps via patched xterm viewport#7450
hmrserver wants to merge 4 commits into
stablyai:mainfrom
hmrserver:hmrserver/smooth-scroll-for-tui-default

Conversation

@hmrserver

@hmrserver hmrserver commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Scrolling inside TUI apps (Claude Code, vim, htop, etc.) in Orca's terminal currently jumps line by line because xterm.js snaps the viewport to whole rows on every scroll event. Rendering the terminal at fractional pixel offsets keeps those same apps smooth.

This PR brings that behavior to Orca by patching @xterm/xterm@6.1.0-beta.287 through pnpm patches, alongside the existing @xterm/addon-webgl patch:

  • Fractional pixel offset: after each row scroll, the viewport keeps the sub-row remainder (_pixelOffset = newRow * cellHeight - scrollTop) and applies it as a device-pixel-ratio-rounded translateY on the screen element instead of snapping to the row grid. The offset resets on buffer switch (normal ↔ alt).
  • Catch-up glide: when TUI output outpaces the viewport (for example, Claude Code streaming), the screen glides to the new position over ~120 ms, clamped to ±2 rows, instead of jumping.
  • WebGL renderer support: the existing @xterm/addon-webgl patch replaces the one-shot beginFrame() boolean with a monotonic clearModelGeneration counter so the GPU renderer keeps re-rendering while the offset animates.

The same logic is mirrored across src/browser/Viewport.ts and both shipped bundles (lib/xterm.js, lib/xterm.mjs). Parity greps verified the copies stay in sync.

No new user-facing settings. The earlier smoothScrollDuration setting experiment was removed from settings UI and all locale files.

Screenshots

Visual change: terminal scrolling and streaming TUI output now move at pixel granularity instead of jumping whole rows.
before: whole-row snapping:

before.mov

after: smooth pixel scroll:

after.mov

Testing

  • pnpm lint — fails with one pre-existing error unrelated to this PR (SourceControlActionRepoOverrideNote.tsx:35 switch-exhaustiveness-check; file is untouched by this branch and identical to the base). No new lint errors introduced.
  • pnpm typecheck
  • pnpm test (2359 files, 24509 tests passed)
  • pnpm build
  • Added or updated high-quality tests that would catch regressions, or explained why tests were not needed — behavior lives inside the patched xterm.js render loop (RAF + GPU transforms), which our unit test harness cannot observe; affected Orca-side suites (terminal pane options/settings, codex-accounts service, locale parity) pass. Bundle/.ts parity is enforced by grep checks; runtime behavior was verified manually against Claude Code, vim and streaming output, cross-checked frame-by-frame against reference recordings.

AI Review Report

Ran an adversarial multi-pass review (Claude + subagents) over the patch and integration. The review focused on offset math across DPR values, animation lifecycle leaks, interaction correctness during animation, renderer parity (DOM/canvas/WebGL), and drift between patched .ts source and minified bundles. Findings fixed:

  • No off-switch: glide now respects prefers-reduced-motion: reduce and snaps instantly
  • Mouse hit-testing skew during glide: capture-phase mousedown settles the animation before selection/clicks
  • Trigger/clamp constant mismatch: reconciled to a single ±2-row clamp with matching ≤2-row trigger
  • RAF leak: animation frame is cancelled via disposable on Terminal.dispose()
  • Permanent compositor layer: willChange: transform now toggles only while an offset is active

Residual: during a glide, a ≤2-row strip at the leading edge can be briefly blank until the next frame. A full fix needs overscan rendering and is out of scope.

Cross-platform: the change is renderer-only inside the xterm patch. It does not touch shortcuts, labels, file paths, shell behavior, or Electron main-process APIs. It runs the same way on macOS, Linux, and Windows, and prefers-reduced-motion is honored through matchMedia on all three. Rendering does not depend on where the PTY lives, so local, remote-server, and SSH-worktree sessions use the same client-side scrolling behavior. SSH latency only affects when output arrives.

Security Audit

  • No new dependencies; package.json only registers the pnpm patch, and the patch content is integrity-pinned through pnpm-lock.yaml
  • No input parsing, command execution, path construction, auth, secrets, or IPC changes; the patch only changes viewport transform math in vendored renderer code
  • Animation state is bounded (offset clamped to ±2 rows, single RAF in flight, cancelled on dispose), so hostile or fast PTY output cannot grow memory or queue unbounded work
  • No follow-up needed

Notes

  • The fix lives in config/patches/@xterm__xterm@6.1.0-beta.287.patch; upgrading xterm.js requires re-applying it.
  • The patch intentionally edits the shipped minified bundles as well as Viewport.ts. This follows the same convention as the existing addon-webgl patch because pnpm consumes the built lib/ output. Parity between the three copies was verified.

ELI5

Scrolling in full-screen TUI apps jumped whole lines and felt choppy. Pixel-level viewport offsets make Claude, vim, htop, and friends scroll smoothly.

…xterm viewport

- Patch @xterm/xterm 6.1.0-beta.287 (Viewport.ts + lib bundles) to carry a
  fractional pixel offset after row scrolls and apply it as a DPR-rounded
  translateY, instead of snapping to whole rows
- Add a catch-up glide when output outpaces the viewport, with
  prefers-reduced-motion off-switch, mousedown settle for hit-testing,
  RAF cleanup on dispose, and willChange toggled only while offset is active
- Pairs with the existing @xterm/addon-webgl clearModelGeneration patch
@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1666482a-fb41-473a-9c01-6edb321b553a

📥 Commits

Reviewing files that changed from the base of the PR and between 59855d0 and 2ef6b1a.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (1)
  • package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • package.json

📝 Walkthrough

Walkthrough

This change updates pnpm.patchedDependencies in package.json to apply a patch to @xterm/xterm@6.1.0-beta.287, while retaining the existing xterm-related patch mappings.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: patched xterm viewport behavior to enable smooth pixel-level scrolling for TUI apps.
Description check ✅ Passed The description follows the template and includes all required sections with concrete testing, review, security, and notes details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@hmrserver

Copy link
Copy Markdown
Contributor Author

@Jinwoo-H any updates on this PR?

@Jinwoo-H

Copy link
Copy Markdown
Contributor

Thanks for working on this — smoother terminal scrolling would be a meaningful improvement. I’d like to discuss two limitations I found before we decide what to do with the PR:

  • Translating xterm’s screen by a fractional row can expose a blank strip at the top or bottom. xterm normally renders exactly the visible rows, so there is no extra rendered row available to slide into view. Avoiding this artifact appears to require renderer-level overscan in both the DOM and WebGL paths.
  • Full-screen TUIs such as Claude handle scrolling themselves. Orca can only send standard, discrete “wheel up/down” terminal reports; the protocol does not include a pixel-distance value. Pixel mouse mode makes the pointer coordinates pixel-accurate, but wheel movement remains discrete. Claude ultimately decides how far each report moves its content.

Because of that, the current approach may improve ordinary terminal scrollback, but I don’t think it can provide true 1 px scrolling inside Claude as written.

Was your intended scope ordinary scrollback as well as application-owned TUI scrolling? And did you find a way around the missing rendered row or the discrete wheel protocol? Possible directions might be narrowing this to scrollback with proper renderer overscan, or exploring TUI-side support/visual interpolation separately.

@hmrserver

Copy link
Copy Markdown
Contributor Author

1. Blank strip — yep, xterm only paints exactly the visible rows, so when we slide the canvas there's literally nothing beyond the edge. Hmm, I was thinking - what if we just render a bit extra on both sides? One overscan row above and below (canvas at rows + 2, offset by -cellHeight), clipped with overflow: hidden on the container. Then real text slides into view instead of a gap.

2. Fullscreen TUIs - right, those won't get this, and that's intentional. Alt-screen apps own their rendering (the CLI itself controls scrolling, e.g. Claude Code's fullscreen mode), so there's nothing for the viewport to animate.
But it does kick in for Claude Code's default mode and general terminal usage - running commands, tailing logs, build output, scrolling back through history, basically any case where a fullscreen TUI isn't involved. That's where people scroll the most anyway.

@AmethystLiang AmethystLiang assigned OrcaWin and unassigned Jinwoo-H Jul 20, 2026
@AmethystLiang
AmethystLiang requested review from OrcaWin and removed request for Jinwoo-H July 20, 2026 18:41
@nwparker nwparker added the bug Something isn't working label Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants