From 4ef87c12e00f8fd889b4fb9f73eafce8fcb6115f Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 24 Jun 2026 14:12:38 -0700 Subject: [PATCH] Use numeric scroll restore for visibility resume Co-authored-by: Orca --- .../components/terminal-pane/pane-helpers.ts | 1 + .../terminal-pane/terminal-visibility-resume.ts | 9 ++++++--- .../use-terminal-pane-global-effects.test.ts | 6 ++++-- .../src/lib/pane-manager/pane-manager.ts | 6 +++++- .../src/lib/pane-manager/pane-scroll.test.ts | 17 +++++++++++++++++ .../src/lib/pane-manager/pane-scroll.ts | 14 ++++++++------ .../src/lib/pane-manager/pane-tree-ops.ts | 4 +++- 7 files changed, 44 insertions(+), 13 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/pane-helpers.ts b/src/renderer/src/components/terminal-pane/pane-helpers.ts index 12be949dd46..f7934471822 100644 --- a/src/renderer/src/components/terminal-pane/pane-helpers.ts +++ b/src/renderer/src/components/terminal-pane/pane-helpers.ts @@ -3,6 +3,7 @@ import type { PaneManager } from '@/lib/pane-manager/pane-manager' type FitPanesOptions = { debugSource?: string syncScrollbar?: boolean + useMarkers?: boolean } export function fitPanes(manager: PaneManager, options?: FitPanesOptions): void { diff --git a/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts b/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts index ea67cca52dd..7cc4150be9e 100644 --- a/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts +++ b/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts @@ -156,12 +156,14 @@ function resumeTerminalVisibilityHeavy(manager: PaneManager, isActive: boolean): if (isActive) { fitAndFocusPanes(manager, { debugSource: 'visibility-resume-fit', - syncScrollbar: false + syncScrollbar: false, + useMarkers: false }) } else { fitPanes(manager, { debugSource: 'visibility-resume-fit', - syncScrollbar: false + syncScrollbar: false, + useMarkers: false }) } } @@ -182,7 +184,8 @@ function restoreTerminalViewportPositions( }) restoreScrollStateAfterLayout(pane.terminal, position, { debugSource: 'visibility-restore', - syncScrollbar: false + syncScrollbar: false, + useMarkers: false }) } } diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts index 9d30f7e6643..e6995005e53 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts @@ -531,7 +531,8 @@ describe('useTerminalPaneGlobalEffects', () => { expect(manager.resumeRendering).toHaveBeenCalledTimes(1) expect(mocks.fitAndFocusPanes).toHaveBeenCalledWith(manager, { debugSource: 'visibility-resume-fit', - syncScrollbar: false + syncScrollbar: false, + useMarkers: false }) expect(manager.resetWebglTextureAtlases).toHaveBeenCalledTimes(1) }) @@ -623,7 +624,8 @@ describe('useTerminalPaneGlobalEffects', () => { expect(manager.suspendRendering).toHaveBeenCalledTimes(1) expect(mocks.restoreScrollStateAfterLayout).toHaveBeenLastCalledWith(terminalA, preHideState, { debugSource: 'visibility-restore', - syncScrollbar: false + syncScrollbar: false, + useMarkers: false }) }) diff --git a/src/renderer/src/lib/pane-manager/pane-manager.ts b/src/renderer/src/lib/pane-manager/pane-manager.ts index 76e382c363a..9f2451943c1 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager.ts @@ -161,7 +161,11 @@ export class PaneManager { return Array.from(this.panes.values()).map(toPublicPane) } - fitAllPanes(options?: { debugSource?: string; syncScrollbar?: boolean }): void { + fitAllPanes(options?: { + debugSource?: string + syncScrollbar?: boolean + useMarkers?: boolean + }): void { fitAllPanesInternal(this.panes, options) } diff --git a/src/renderer/src/lib/pane-manager/pane-scroll.test.ts b/src/renderer/src/lib/pane-manager/pane-scroll.test.ts index 963576281cc..f06fd8ff6e9 100644 --- a/src/renderer/src/lib/pane-manager/pane-scroll.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-scroll.test.ts @@ -210,6 +210,23 @@ describe('scroll state', () => { expect(marker.dispose).toHaveBeenCalledTimes(1) }) + it('can disable marker restore for visibility resume', () => { + const terminal = createTerminal({ viewportY: 10, baseY: 300 }) + const marker = createMarker(71) + const state: ScrollState = { + bufferType: 'normal', + wasAtBottom: false, + viewportY: 150, + baseY: 154, + firstVisibleLineMarker: marker + } + + restoreScrollStateAfterLayout(terminal, state, { useMarkers: false }) + + expect(terminal.scrollToLine).toHaveBeenCalledWith(150) + expect(terminal.buffer.active.viewportY).toBe(150) + }) + it('reapplies a layout restore after xterm settles asynchronously', () => { vi.useFakeTimers() const rafCallbacks: FrameRequestCallback[] = [] diff --git a/src/renderer/src/lib/pane-manager/pane-scroll.ts b/src/renderer/src/lib/pane-manager/pane-scroll.ts index db6258310c4..3aaf00668c9 100644 --- a/src/renderer/src/lib/pane-manager/pane-scroll.ts +++ b/src/renderer/src/lib/pane-manager/pane-scroll.ts @@ -22,6 +22,7 @@ const deferredScrollRestores = new WeakMap< type RestoreScrollStateOptions = { debugSource?: string syncScrollbar?: boolean + useMarkers?: boolean } export function recordTerminalOutput(terminal: Terminal): void { @@ -74,7 +75,7 @@ export function captureScrollState(terminal: Terminal): ScrollState { export function restoreScrollState(terminal: Terminal, state: ScrollState): void { cancelDeferredScrollRestore(terminal) - restoreScrollStateNow(terminal, state, { syncScrollbar: true }) + restoreScrollStateNow(terminal, state, { syncScrollbar: true, useMarkers: true }) releaseScrollStateMarker(state) } @@ -85,6 +86,7 @@ export function restoreScrollStateAfterLayout( ): void { cancelDeferredScrollRestore(terminal) const syncScrollbar = options.syncScrollbar ?? true + const useMarkers = options.useMarkers ?? true const debugSource = options.debugSource logTerminalScrollRestore('restore-scheduled', { source: debugSource, @@ -92,7 +94,7 @@ export function restoreScrollStateAfterLayout( syncScrollbar, viewport: terminalViewportForDebug(terminal) }) - restoreScrollStateNow(terminal, state, { debugSource, syncScrollbar }) + restoreScrollStateNow(terminal, state, { debugSource, syncScrollbar, useMarkers }) if (typeof requestAnimationFrame !== 'function') { releaseScrollStateMarker(state) return @@ -107,7 +109,7 @@ export function restoreScrollStateAfterLayout( } const restore = (): void => { if (!pending.cancelled) { - restoreScrollStateNow(terminal, state, { debugSource, syncScrollbar }) + restoreScrollStateNow(terminal, state, { debugSource, syncScrollbar, useMarkers }) } } const cancelPendingRafs = (): void => { @@ -129,7 +131,7 @@ export function restoreScrollStateAfterLayout( }) const timeoutId = setTimeout(() => { if (!pending.cancelled) { - restoreScrollStateNow(terminal, state, { debugSource, syncScrollbar }) + restoreScrollStateNow(terminal, state, { debugSource, syncScrollbar, useMarkers }) } // Why: background tabs can throttle rAF past the timeout. Once the // authoritative timeout restore has run, stale frame callbacks must not @@ -146,7 +148,7 @@ export function restoreScrollStateAfterLayout( function restoreScrollStateNow( terminal: Terminal, state: ScrollState, - options: RestoreScrollStateOptions & { syncScrollbar: boolean } + options: RestoreScrollStateOptions & { syncScrollbar: boolean; useMarkers: boolean } ): void { if (!terminal.element) { logTerminalScrollRestore('restore-attempt', { @@ -197,7 +199,7 @@ function restoreScrollStateNow( : -1 // Why: xterm markers can drift while TUIs rewrite/reflow the same scrollback // base. Prefer the exact numeric viewport unless the buffer base changed. - const shouldUseMarkerLine = markerLine >= 0 && buf.baseY !== state.baseY + const shouldUseMarkerLine = options.useMarkers && markerLine >= 0 && buf.baseY !== state.baseY const targetLine = Math.min(shouldUseMarkerLine ? markerLine : state.viewportY, buf.baseY) // Why: deferred rAF/timeout restores re-invoke this function after xterm // reflow settles; keep the original viewport and marker alive so later diff --git a/src/renderer/src/lib/pane-manager/pane-tree-ops.ts b/src/renderer/src/lib/pane-manager/pane-tree-ops.ts index 80f1ad8b261..c82c1093943 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-ops.ts +++ b/src/renderer/src/lib/pane-manager/pane-tree-ops.ts @@ -30,6 +30,7 @@ type TreeOpsCallbacks = { type SafeFitOptions = { debugSource?: string syncScrollbar?: boolean + useMarkers?: boolean } const MIN_PANE_FIT_WIDTH_PX = 48 @@ -110,7 +111,8 @@ export function safeFit(pane: ManagedPane, options: SafeFitOptions = {}): void { try { restoreScrollStateAfterLayout(pane.terminal, scrollState, { debugSource: options.debugSource ?? 'safeFit', - syncScrollbar: options.syncScrollbar + syncScrollbar: options.syncScrollbar, + useMarkers: options.useMarkers }) } catch { // Why: xterm can temporarily expose a terminal whose renderer has not