From aa4e5b581933c9ec24e7dd3dd921b3ce0c316d18 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 3 Aug 2026 23:13:38 -0700 Subject: [PATCH] fix(terminal): rate-cap WebGL glyph-atlas recovery on streaming output A TUI redrawing just slower than the 200ms quiet window trips the debounce trailing edge on every parse, so the streaming path wiped the shared glyph atlas and repainted every pane ~200x/min (field bundle F0BMLFAFWF7: 442 wipes in 6.2 min). Bound the streaming path with a token bucket: 3s minimum interval, 10-wipe burst, refilling one wipe per 6s. Over-budget settles re-present from the live buffers instead. Paste and tab reveal keep their immediate burst. A fixed count over a rolling 60s window was rejected: it spends the whole allowance in the first 30s of a sustained stream and then starves for 33s, and the over-budget present never rebuilds the glyph atlas, so vim-style rewrites stayed garbled for half a minute. Continuous refill keeps the same sustained ceiling with a worst-case wipe gap of ~6s. Co-authored-by: Orca --- .../terminal-webgl-atlas-recovery.test.ts | 147 +++++++++++++++++- .../terminal-webgl-atlas-recovery.ts | 78 +++++++++- .../lib/pane-manager/pane-manager-registry.ts | 23 +++ 3 files changed, 245 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/terminal-webgl-atlas-recovery.test.ts b/src/renderer/src/components/terminal-pane/terminal-webgl-atlas-recovery.test.ts index 93c696719f9..5b34b4cf3a3 100644 --- a/src/renderer/src/components/terminal-pane/terminal-webgl-atlas-recovery.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-webgl-atlas-recovery.test.ts @@ -4,6 +4,7 @@ import { unregisterLivePaneManager } from '@/lib/pane-manager/pane-manager-registry' import { + resetTerminalWebglAtlasRecoveryBudgetForTesting, scheduleImagePasteWebglAtlasRecovery, scheduleTabRevealWebglAtlasRecovery, scheduleTerminalWebglAtlasRecovery, @@ -16,10 +17,12 @@ describe('terminal WebGL atlas recovery', () => { function registerManager(): { resetWebglTextureAtlases: Mock<() => void> refreshAllPanes: Mock<() => void> + scheduleRevealPresent: Mock<() => void> } { const manager = { resetWebglTextureAtlases: vi.fn<() => void>(), - refreshAllPanes: vi.fn<() => void>() + refreshAllPanes: vi.fn<() => void>(), + scheduleRevealPresent: vi.fn<() => void>() } registerLivePaneManager(manager) registeredManagers.push(manager) @@ -36,6 +39,7 @@ describe('terminal WebGL atlas recovery', () => { vi.clearAllTimers() vi.useRealTimers() vi.unstubAllGlobals() + resetTerminalWebglAtlasRecoveryBudgetForTesting() }) it('clears atlases and refreshes panes through the post-paste redraw window', () => { @@ -272,6 +276,147 @@ describe('terminal WebGL atlas recovery', () => { } }) + it('caps shared-atlas wipes under a sustained alt-screen redraw cadence', () => { + // Field regression (F0BMLFAFWF7 on 1.4.167): 442 full shared-glyph-atlas + // wipes in 6.2 minutes. A TUI redrawing just slower than the quiet window + // clears the debounce on every parse, so the trailing edge fires each time + // — the debounce bounds nothing at this cadence. + vi.useFakeTimers() + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + ) + const manager = registerManager() + + for (let elapsed = 0; elapsed < 60_000; elapsed += 300) { + scheduleTerminalWebglAtlasRecovery() + vi.advanceTimersByTime(300) + } + + expect(manager.resetWebglTextureAtlases.mock.calls.length).toBeLessThanOrEqual(30) + }) + + it('keeps the wipe gap bounded under a sustained redraw cadence, not burst-then-starve', () => { + // Review regression: a fixed count over a rolling 60s window spends its whole + // allowance in the first 30s and then starves for 33s. The over-budget path + // only re-presents the buffers — it never rebuilds the glyph atlas — so a + // vim-style rewrite stayed garbled for half a minute. The refill must be + // continuous so the worst-case gap degrades to the refill interval. + vi.useFakeTimers() + vi.setSystemTime(0) + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + ) + const wipeTimes: number[] = [] + const manager = { + resetWebglTextureAtlases: vi.fn(() => wipeTimes.push(Date.now())), + refreshAllPanes: vi.fn(), + scheduleRevealPresent: vi.fn() + } + registerLivePaneManager(manager) + registeredManagers.push(manager) + + // Five minutes of the field cadence (F0BMLFAFWF7 ran ~1 redraw/s for hours). + for (let elapsed = 0; elapsed < 300_000; elapsed += 300) { + scheduleTerminalWebglAtlasRecovery() + vi.advanceTimersByTime(300) + } + + const gaps = wipeTimes.slice(1).map((at, index) => at - wipeTimes[index]!) + expect(Math.max(...gaps)).toBeLessThanOrEqual(6_500) + // Still a large cut from the unbounded ~200 wipes/min the debounce allowed. + expect(wipeTimes.length).toBeLessThanOrEqual(75) + }) + + it('presents from the live buffers when a settle is over the wipe budget', () => { + // De-escalation: a suppressed settle must still recover stale pixels, just + // without wiping the atlas shared by every same-config terminal. + vi.useFakeTimers() + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + ) + const manager = registerManager() + + scheduleTerminalWebglAtlasRecovery() + vi.advanceTimersByTime(TERMINAL_OUTPUT_RECOVERY_QUIET_MS) + expect(manager.resetWebglTextureAtlases).toHaveBeenCalledTimes(1) + expect(manager.scheduleRevealPresent).not.toHaveBeenCalled() + + // Second settle lands inside the minimum interval → present, no wipe. + scheduleTerminalWebglAtlasRecovery() + vi.advanceTimersByTime(TERMINAL_OUTPUT_RECOVERY_QUIET_MS) + expect(manager.resetWebglTextureAtlases).toHaveBeenCalledTimes(1) + expect(manager.scheduleRevealPresent).toHaveBeenCalledTimes(1) + }) + + it('recovers the wipe budget when the wall clock steps backwards', () => { + // Sleep/resume, NTP correction and SSH host-clock skew can move Date.now() + // backwards; the bucket must not wedge until the clock catches up. + vi.useFakeTimers() + vi.setSystemTime(600_000) + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + ) + const manager = registerManager() + + scheduleTerminalWebglAtlasRecovery() + vi.advanceTimersByTime(TERMINAL_OUTPUT_RECOVERY_QUIET_MS) + expect(manager.resetWebglTextureAtlases).toHaveBeenCalledTimes(1) + + vi.setSystemTime(60_000) + scheduleTerminalWebglAtlasRecovery() + vi.advanceTimersByTime(TERMINAL_OUTPUT_RECOVERY_QUIET_MS) + expect(manager.resetWebglTextureAtlases).toHaveBeenCalledTimes(2) + }) + + it('keeps paste and reveal recovery immediate after the streaming budget is spent', () => { + // Reveal/paste are one-shot user-visible events; the streaming rate cap must + // never defer or downgrade them. + vi.useFakeTimers() + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + ) + const manager = registerManager() + + for (let elapsed = 0; elapsed < 60_000; elapsed += 300) { + scheduleTerminalWebglAtlasRecovery() + vi.advanceTimersByTime(300) + } + manager.resetWebglTextureAtlases.mockClear() + manager.refreshAllPanes.mockClear() + + scheduleTabRevealWebglAtlasRecovery() + expect(manager.resetWebglTextureAtlases).toHaveBeenCalledTimes(1) + vi.advanceTimersByTime(500) + expect(manager.resetWebglTextureAtlases).toHaveBeenCalledTimes(3) + expect(manager.refreshAllPanes).toHaveBeenCalledTimes(3) + + manager.resetWebglTextureAtlases.mockClear() + scheduleImagePasteWebglAtlasRecovery() + expect(manager.resetWebglTextureAtlases).toHaveBeenCalledTimes(1) + vi.advanceTimersByTime(500) + expect(manager.resetWebglTextureAtlases).toHaveBeenCalledTimes(3) + }) + it('coalesces the terminal-output callers onto one shared timer while paste stays immediate', () => { // Cross-caller single-timer invariant: the terminal-output re-arm sites // (foreground and hidden-output PTY writes) funnel through diff --git a/src/renderer/src/components/terminal-pane/terminal-webgl-atlas-recovery.ts b/src/renderer/src/components/terminal-pane/terminal-webgl-atlas-recovery.ts index 1912e0c0e06..4028799a7c2 100644 --- a/src/renderer/src/components/terminal-pane/terminal-webgl-atlas-recovery.ts +++ b/src/renderer/src/components/terminal-pane/terminal-webgl-atlas-recovery.ts @@ -1,4 +1,7 @@ -import { resetAndRefreshAllTerminalWebglAtlases } from '@/lib/pane-manager/pane-manager-registry' +import { + presentAllTerminalPanesWithoutAtlasClear, + resetAndRefreshAllTerminalWebglAtlases +} from '@/lib/pane-manager/pane-manager-registry' const ATLAS_RECOVERY_DELAYS_MS = [120, 500] @@ -7,7 +10,25 @@ const ATLAS_RECOVERY_DELAYS_MS = [120, 500] // (STA-1365). Wait for output to go quiet so recovery runs once, on settle. export const TERMINAL_OUTPUT_RECOVERY_QUIET_MS = 200 +// Why: the debounce bounds nothing against a TUI redrawing just slower than the +// quiet window — every parse then trips the trailing edge. Field bundle +// F0BMLFAFWF7 recorded 442 shared-atlas wipes in 6.2 min (~1/s, sustained for +// hours). Bound how often the streaming path may escalate to a wipe; over-budget +// settles present from the live buffers instead. +const TERMINAL_OUTPUT_RECOVERY_MIN_INTERVAL_MS = 3_000 +// Why: a token bucket, not a fixed-window count. A count over a rolling 60s +// window burns its whole allowance in the first 30s of a sustained stream and +// then starves for 33s, and the only recovery left in that hole is a buffer +// re-present, which never rebuilds the glyph atlas — so vim-style rewrites stay +// garbled for half a minute. Refilling continuously keeps the same sustained +// ceiling while degrading the worst-case wipe gap to the refill interval. +const TERMINAL_OUTPUT_RECOVERY_BURST_WIPES = 10 +const TERMINAL_OUTPUT_RECOVERY_REFILL_INTERVAL_MS = 6_000 + let terminalOutputRecoveryDebounceTimer: ReturnType | null = null +let terminalOutputRecoveryWipeTokens = TERMINAL_OUTPUT_RECOVERY_BURST_WIPES +let terminalOutputRecoveryTokensRefilledAt: number | null = null +let terminalOutputRecoveryLastWipeAt: number | null = null function scheduleNextFrame(callback: () => void): void { if (typeof globalThis.requestAnimationFrame === 'function') { @@ -27,6 +48,14 @@ function resetAtlasesAndRefreshPanes(): void { } } +function presentPanesWithoutAtlasClear(): void { + try { + presentAllTerminalPanesWithoutAtlasClear() + } catch { + /* ignore - terminal pane may have unmounted after scheduling recovery */ + } +} + function scheduleAtlasRecoveryBurst(): void { scheduleNextFrame(() => resetAtlasesAndRefreshPanes()) for (const delayMs of ATLAS_RECOVERY_DELAYS_MS) { @@ -56,6 +85,51 @@ export function scheduleTerminalWebglAtlasRecovery(): void { } terminalOutputRecoveryDebounceTimer = globalThis.setTimeout(() => { terminalOutputRecoveryDebounceTimer = null - resetAtlasesAndRefreshPanes() + if (consumeTerminalOutputRecoveryWipeBudget()) { + resetAtlasesAndRefreshPanes() + return + } + presentPanesWithoutAtlasClear() }, TERMINAL_OUTPUT_RECOVERY_QUIET_MS) } + +// Test seam: the wipe budget is module-global, so a suite of scheduler tests +// would otherwise inherit the previous test's spent budget. +export function resetTerminalWebglAtlasRecoveryBudgetForTesting(): void { + terminalOutputRecoveryWipeTokens = TERMINAL_OUTPUT_RECOVERY_BURST_WIPES + terminalOutputRecoveryTokensRefilledAt = null + terminalOutputRecoveryLastWipeAt = null +} + +function refillTerminalOutputRecoveryWipeTokens(now: number): void { + const elapsedMs = now - (terminalOutputRecoveryTokensRefilledAt ?? now) + terminalOutputRecoveryTokensRefilledAt = now + // Why: wall-clock can step backwards (NTP, system sleep, VM resume); treat that + // as a fresh bucket so recovery can't wedge until the clock catches up. + if (elapsedMs < 0) { + terminalOutputRecoveryWipeTokens = TERMINAL_OUTPUT_RECOVERY_BURST_WIPES + terminalOutputRecoveryLastWipeAt = null + return + } + terminalOutputRecoveryWipeTokens = Math.min( + TERMINAL_OUTPUT_RECOVERY_BURST_WIPES, + terminalOutputRecoveryWipeTokens + elapsedMs / TERMINAL_OUTPUT_RECOVERY_REFILL_INTERVAL_MS + ) +} + +function consumeTerminalOutputRecoveryWipeBudget(): boolean { + const now = Date.now() + refillTerminalOutputRecoveryWipeTokens(now) + if ( + terminalOutputRecoveryLastWipeAt != null && + now - terminalOutputRecoveryLastWipeAt < TERMINAL_OUTPUT_RECOVERY_MIN_INTERVAL_MS + ) { + return false + } + if (terminalOutputRecoveryWipeTokens < 1) { + return false + } + terminalOutputRecoveryWipeTokens -= 1 + terminalOutputRecoveryLastWipeAt = now + return true +} diff --git a/src/renderer/src/lib/pane-manager/pane-manager-registry.ts b/src/renderer/src/lib/pane-manager/pane-manager-registry.ts index d864ce0de37..afe3b3b7949 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-registry.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-registry.ts @@ -10,6 +10,7 @@ type RegisteredPaneManager = { getPanes?: (limit?: number) => { id: number; terminal: unknown }[] getPaneCount?: () => number isVisibleForAtlasRecovery?: () => boolean + scheduleRevealPresent?: () => void } const liveManagers = new Set() @@ -77,6 +78,28 @@ export function resetAndRefreshAllTerminalWebglAtlases(): void { } } +/** + * Re-presents every live pane from its xterm buffer without touching the shared + * glyph atlas. + * + * Why: the de-escalated recovery step. A wipe rebuilds every same-config + * terminal's render model; a present only asks the compositor for a fresh frame, + * so a busy TUI can keep recovering stale pixels without the global churn. + */ +export function presentAllTerminalPanesWithoutAtlasClear(): void { + for (const manager of liveManagers) { + if (manager.isVisibleForAtlasRecovery?.() === false) { + continue + } + try { + manager.scheduleRevealPresent?.() + } catch { + // Why: best-effort during teardown; one disposed manager must not block + // sibling terminals from re-presenting. + } + } +} + /** * Per-pane WebGL renderer state across all live managers, for the one-paste * freeze report. Lets a post-wake garble report show, per pane, whether it