From 02ea41045b7432cf6fa4df905ab30cd48102559e Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:11:25 -0700 Subject: [PATCH] Revert "fix(renderer): repair canvas backing synchronously on reveal fit" This reverts commit 60bfa6bed04586b0b1513b295c48b024b8b869ac. --- .../terminal-visibility-resume.test.ts | 13 +- .../terminal-visibility-resume.ts | 7 +- .../lib/pane-manager/pane-reveal-fit.test.ts | 45 +-- .../src/lib/pane-manager/pane-reveal-fit.ts | 25 +- .../lib/pane-manager/pane-webgl-renderer.ts | 8 +- .../terminal-canvas-backing-repair.test.ts | 368 ------------------ .../terminal-canvas-backing-repair.ts | 166 -------- .../terminal-canvas-dpr-repair.test.ts | 150 +++++++ .../terminal-canvas-dpr-repair.ts | 70 ++++ 9 files changed, 240 insertions(+), 612 deletions(-) delete mode 100644 src/renderer/src/lib/pane-manager/terminal-canvas-backing-repair.test.ts delete mode 100644 src/renderer/src/lib/pane-manager/terminal-canvas-backing-repair.ts create mode 100644 src/renderer/src/lib/pane-manager/terminal-canvas-dpr-repair.test.ts create mode 100644 src/renderer/src/lib/pane-manager/terminal-canvas-dpr-repair.ts diff --git a/src/renderer/src/components/terminal-pane/terminal-visibility-resume.test.ts b/src/renderer/src/components/terminal-pane/terminal-visibility-resume.test.ts index a9b28e13ca8..3b198054861 100644 --- a/src/renderer/src/components/terminal-pane/terminal-visibility-resume.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-visibility-resume.test.ts @@ -30,10 +30,9 @@ vi.mock('@/lib/pane-manager/pane-fit', () => ({ flushDeferredPaneMetricOptionsIfMeasurable: (pane: unknown) => flushDeferredPaneMetricOptionsIfMeasurable(pane) })) -const repairPaneWebglCanvasBackingMismatch = vi.fn((_pane: unknown) => false) -vi.mock('@/lib/pane-manager/terminal-canvas-backing-repair', () => ({ - repairPaneWebglCanvasBackingMismatch: (pane: unknown) => - repairPaneWebglCanvasBackingMismatch(pane) +const repairPaneWebglCanvasDprMismatch = vi.fn((_pane: unknown) => false) +vi.mock('@/lib/pane-manager/terminal-canvas-dpr-repair', () => ({ + repairPaneWebglCanvasDprMismatch: (pane: unknown) => repairPaneWebglCanvasDprMismatch(pane) })) const resetTerminalLinkifierHoverState = vi.fn() const isTerminalLinkifierHoverActive = vi.fn((_terminal: unknown) => false) @@ -164,9 +163,9 @@ describe('resumeTerminalVisibility reveal repaint', () => { resumeTerminalVisibility(resumeArgs(manager, true)) - expect(repairPaneWebglCanvasBackingMismatch).toHaveBeenCalledTimes(2) - expect(repairPaneWebglCanvasBackingMismatch).toHaveBeenNthCalledWith(1, first) - expect(repairPaneWebglCanvasBackingMismatch).toHaveBeenNthCalledWith(2, second) + expect(repairPaneWebglCanvasDprMismatch).toHaveBeenCalledTimes(2) + expect(repairPaneWebglCanvasDprMismatch).toHaveBeenNthCalledWith(1, first) + expect(repairPaneWebglCanvasDprMismatch).toHaveBeenNthCalledWith(2, second) }) it('flushes hidden-era metric options on reveal and refits the light path', () => { 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 d84fd71092d..ee86edeba18 100644 --- a/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts +++ b/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts @@ -16,7 +16,7 @@ import { import { focusActivePane } from './pane-helpers' import { scheduleTabRevealWebglAtlasRecovery } from './terminal-webgl-atlas-recovery' import { flushDeferredPaneMetricOptionsIfMeasurable } from '@/lib/pane-manager/pane-fit' -import { repairPaneWebglCanvasBackingMismatch } from '@/lib/pane-manager/terminal-canvas-backing-repair' +import { repairPaneWebglCanvasDprMismatch } from '@/lib/pane-manager/terminal-canvas-dpr-repair' const VISIBLE_RESUME_FLUSH_CHARS = 256 * 1024 const WINDOW_WAKE_FLUSH_CHARS = 64 * 1024 @@ -80,9 +80,8 @@ export function resumeTerminalVisibility({ flushedDeferredMetrics = true } // Why here: the light path neither recreates WebGL nor fits, so a dpr - // or layout change that landed while this tab was hidden has no other - // repair point. - repairPaneWebglCanvasBackingMismatch(pane) + // change that landed while this tab was hidden has no other repair point. + repairPaneWebglCanvasDprMismatch(pane) } // Why: intra-worktree tab switches only toggle the overlay. Keeping // synchronous drain and atlas rebuilds off this path avoids racing the diff --git a/src/renderer/src/lib/pane-manager/pane-reveal-fit.test.ts b/src/renderer/src/lib/pane-manager/pane-reveal-fit.test.ts index f77285a9bff..c3ac10f44b4 100644 --- a/src/renderer/src/lib/pane-manager/pane-reveal-fit.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-reveal-fit.test.ts @@ -10,8 +10,7 @@ const mocks = vi.hoisted(() => ({ requestStablePaneFit: vi.fn(), clearPaneFitContinuationRetry: vi.fn(), resumePendingFitScrollRestoreAfterFit: vi.fn(), - flushDeferredPaneMetricOptionsIfMeasurable: vi.fn(() => false), - repairPaneWebglCanvasBackingMismatch: vi.fn(() => false) + flushDeferredPaneMetricOptionsIfMeasurable: vi.fn(() => false) })) vi.mock('./pane-fit', () => ({ @@ -30,9 +29,6 @@ vi.mock('./pane-fit-continuation-retry', () => ({ vi.mock('./pane-scroll', () => ({ resumePendingFitScrollRestoreAfterFit: mocks.resumePendingFitScrollRestoreAfterFit })) -vi.mock('./terminal-canvas-backing-repair', () => ({ - repairPaneWebglCanvasBackingMismatch: mocks.repairPaneWebglCanvasBackingMismatch -})) type RevealTestPane = ManagedPane & { lastFitClientSize?: { width: number; height: number } } @@ -161,45 +157,6 @@ describe('fitRevealedPane routing', () => { expect(mocks.flushPendingSafeFitContinuations).not.toHaveBeenCalled() }) - it('repairs the canvas backing store synchronously, after the grid fit', () => { - // The blurry reveal: a hidden-time layout change refits the grid here while - // xterm still holds the pre-hide backing. Deferring the repair to rAF is what - // stretches the rescaled-frame window, so it must land in this same task. - const raf = vi.fn() - vi.stubGlobal('requestAnimationFrame', raf) - const pane = createPane({ - lastFitClientSize: { width: 800, height: 600 }, - currentSize: { width: 640, height: 480 }, - terminal: { cols: 80, rows: 24 }, - proposed: { cols: 64, rows: 20 } - }) - - fitRevealedPane(pane) - - expect(mocks.repairPaneWebglCanvasBackingMismatch).toHaveBeenCalledWith(pane) - expect(mocks.repairPaneWebglCanvasBackingMismatch.mock.invocationCallOrder[0]!).toBeGreaterThan( - mocks.safeFit.mock.invocationCallOrder[0]! - ) - expect(raf).not.toHaveBeenCalled() - vi.unstubAllGlobals() - }) - - it('repairs the canvas backing store on the branches that skip the fit', () => { - // A dpr change behind display:none moves neither pixels nor grid, so this - // branch is the only reveal-time repair point it has. - const pane = createPane({ - lastFitClientSize: { width: 800, height: 600 }, - currentSize: { width: 800, height: 600 }, - terminal: { cols: 80, rows: 24 }, - proposed: { cols: 80, rows: 24 } - }) - - fitRevealedPane(pane) - - expect(mocks.safeFit).not.toHaveBeenCalled() - expect(mocks.repairPaneWebglCanvasBackingMismatch).toHaveBeenCalledWith(pane) - }) - it('does not release continuations when an unchanged pane is unmeasurable', () => { mocks.canMeasurePaneForFit.mockReturnValue(false) const pane = createPane({ diff --git a/src/renderer/src/lib/pane-manager/pane-reveal-fit.ts b/src/renderer/src/lib/pane-manager/pane-reveal-fit.ts index c6e969e00eb..f0125433764 100644 --- a/src/renderer/src/lib/pane-manager/pane-reveal-fit.ts +++ b/src/renderer/src/lib/pane-manager/pane-reveal-fit.ts @@ -9,7 +9,6 @@ import { import { requestStablePaneFit } from './pane-fit-resize-observer' import { clearPaneFitContinuationRetry } from './pane-fit-continuation-retry' import { resumePendingFitScrollRestoreAfterFit } from './pane-scroll' -import { repairPaneWebglCanvasBackingMismatch } from './terminal-canvas-backing-repair' // Why: a real resize changes the element's pixels; a metric-only wobble does not. // No baseline / unmeasurable counts as changed so a first reveal still fits. @@ -51,30 +50,18 @@ function releaseMeasurableFitContinuations(pane: ManagedPane): void { clearPaneFitContinuationRetry(pane) } -// Reveal fit (minimize→restore, worktree foreground, window wake): settle the -// grid, then the canvas surface that has to agree with it. -export function fitRevealedPane(pane: ManagedPane): void { - fitRevealedPaneGrid(pane) - // Why synchronously, in this same task: the grid above re-measures a layout - // change that landed while the pane was hidden, but xterm defers the matching - // canvas resize while its render service is still paused, so the compositor - // rescales the pre-hide surface into the new box (blurry, outlined text). - // rAF is the very queue that stretches that window, so the backing store has - // to be corrected before this task yields. No-ops when it already agrees. - repairPaneWebglCanvasBackingMismatch(pane) -} - -// resumeRendering re-attaches WebGL, whose cell metrics briefly differ from the -// DOM renderer's, so a raw fit can propose a one-column-off grid, reflow xterm, -// then snap back — and xterm's wrap→unwrap is not a perfect inverse, so a -// diff-painting inline TUI (grok, Codex) is left corrupted. So: +// Reveal fit (minimize→restore, worktree foreground, window wake). resumeRendering +// re-attaches WebGL, whose cell metrics briefly differ from the DOM renderer's, so +// a raw fit can propose a one-column-off grid, reflow xterm, then snap back — and +// xterm's wrap→unwrap is not a perfect inverse, so a diff-painting inline TUI +// (grok, Codex) is left corrupted. So: // - pixels changed while hidden → real resize: fit now (also keeps xterm ahead of // the async {fit:false} PTY size reassert so it can't forward a stale grid); // - pixels unchanged but grid diverged (a direct terminal.resize from snapshot / // SSH-reattach, or a DPI change) → repair on a steady grid, so a sustained // mismatch refits but a transient metric wobble does not reflow; // - grid already correct → leave it alone. -function fitRevealedPaneGrid(pane: ManagedPane): void { +export function fitRevealedPane(pane: ManagedPane): void { // Why first: the checks below can both say "nothing to do" and return without // fitting, stranding metric options parked while the pane was unmeasurable. const flushed = flushDeferredPaneMetricOptionsIfMeasurable(pane) diff --git a/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts b/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts index 2087fdb3edf..f162052cebe 100644 --- a/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts +++ b/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts @@ -9,7 +9,7 @@ import { } from './terminal-webgl-auto-policy' import { safeFit, safeFitAndThen } from './pane-fit' import { setPaneFitWebglAttachHook } from './pane-fit-webgl-attach-signal' -import { repairPaneWebglCanvasBackingMismatch } from './terminal-canvas-backing-repair' +import { repairPaneWebglCanvasDprMismatch } from './terminal-canvas-dpr-repair' export const ENABLE_WEBGL_RENDERER = true let suggestedRendererType: 'dom' | undefined @@ -206,9 +206,9 @@ export function attachWebglAfterFitIfMissing(pane: ManagedPaneInternal): void { setPaneFitWebglAttachHook((pane) => { attachWebglAfterFitIfMissing(pane) // Why here too: a fit proves the pane has a live box, which is the earliest - // safe moment to catch a canvas whose backing store still reflects the grid - // or devicePixelRatio from before a hidden-time layout/display change. - repairPaneWebglCanvasBackingMismatch(pane) + // safe moment to catch a canvas whose backing store still reflects a + // devicePixelRatio from before a hidden-time display change. + repairPaneWebglCanvasDprMismatch(pane) }) export function attachWebgl(pane: ManagedPaneInternal): void { diff --git a/src/renderer/src/lib/pane-manager/terminal-canvas-backing-repair.test.ts b/src/renderer/src/lib/pane-manager/terminal-canvas-backing-repair.test.ts deleted file mode 100644 index fec7a6c8e0e..00000000000 --- a/src/renderer/src/lib/pane-manager/terminal-canvas-backing-repair.test.ts +++ /dev/null @@ -1,368 +0,0 @@ -import { describe, expect, it, vi } from 'vitest' -import type { ManagedPane } from './pane-manager-types' -import { repairPaneWebglCanvasBackingMismatch } from './terminal-canvas-backing-repair' - -type PaneArgs = { - backingWidth: number - dpr: number - backingHeight?: number - /** Renderer device cell metrics; the grid predicate is cols/rows x these. */ - cellWidth?: number - cellHeight?: number - cols?: number - rows?: number - /** Stale `dimensions.device.canvas`, used only when cell metrics are absent. */ - deviceCanvasWidth?: number - deviceCanvasHeight?: number - /** Inline css box the renderer last wrote; defaults to backing / dpr. */ - cssWidth?: number - cssHeight?: number - connected?: boolean - hasRenderer?: boolean - paneBox?: { width: number; height: number } | null - paused?: boolean - charSizeValid?: boolean -} - -function makePane(args: PaneArgs): { - pane: ManagedPane - handleDevicePixelRatioChange: ReturnType - handleResize: ReturnType - refresh: ReturnType - refreshRows: ReturnType -} { - const handleDevicePixelRatioChange = vi.fn() - const handleResize = vi.fn() - const refresh = vi.fn() - const refreshRows = vi.fn() - const backingHeight = args.backingHeight ?? 1200 - const cols = args.cols ?? 120 - const rows = args.rows ?? 40 - const canvas = { - width: args.backingWidth, - height: backingHeight, - isConnected: args.connected ?? true, - ownerDocument: { defaultView: { devicePixelRatio: args.dpr } }, - style: { - width: `${args.cssWidth ?? Math.round(args.backingWidth / args.dpr)}px`, - height: `${args.cssHeight ?? Math.round(backingHeight / args.dpr)}px` - }, - getBoundingClientRect: () => { - throw new Error('repair detection must not force layout on the canvas') - } - } - const hasCellMetrics = args.cellWidth != null && args.cellHeight != null - const renderer = - (args.hasRenderer ?? true) - ? { - _canvas: canvas, - dimensions: { - device: { - cell: hasCellMetrics ? { width: args.cellWidth, height: args.cellHeight } : undefined, - canvas: { - width: args.deviceCanvasWidth ?? 0, - height: args.deviceCanvasHeight ?? 0 - } - } - }, - handleDevicePixelRatioChange, - handleResize - } - : undefined - const box = args.paneBox === undefined ? { width: 1080, height: 600 } : args.paneBox - const container = box - ? { getBoundingClientRect: () => ({ width: box.width, height: box.height }) } - : {} - const pane = { - id: 1, - container, - terminal: { - cols, - rows, - refresh, - _core: { - _charSizeService: { hasValidSize: args.charSizeValid ?? true }, - _renderService: { - _renderer: { value: renderer }, - _isPaused: args.paused ?? false, - refreshRows - } - } - } - } as unknown as ManagedPane - return { pane, handleDevicePixelRatioChange, handleResize, refresh, refreshRows } -} - -describe('repairPaneWebglCanvasBackingMismatch', () => { - it('repairs a stale dpr-2 backing composited on a dpr-1 display', () => { - // The originally reproduced field bug: a hidden-time display change leaves a - // 2160px backing behind a 1080px css box at dpr 1 (half-size/smeared text). - const { pane, handleDevicePixelRatioChange, handleResize, refresh } = makePane({ - backingWidth: 2160, - backingHeight: 1200, - cssWidth: 1080, - cssHeight: 600, - dpr: 1, - cellWidth: 9, - cellHeight: 15 - }) - - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(true) - expect(handleDevicePixelRatioChange).toHaveBeenCalledTimes(1) - expect(handleResize).toHaveBeenCalledWith(120, 40) - expect(refresh).toHaveBeenCalledWith(0, 39) - // Dpr refresh must precede the resize that rebuilds the backing store. - expect(handleDevicePixelRatioChange.mock.invocationCallOrder[0]!).toBeLessThan( - handleResize.mock.invocationCallOrder[0]! - ) - }) - - it('repairs the opposite direction (dpr-1 backing upscaled on retina)', () => { - const { pane, handleResize } = makePane({ - backingWidth: 1080, - backingHeight: 600, - cssWidth: 1080, - cssHeight: 600, - dpr: 2, - cellWidth: 18, - cellHeight: 30 - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(true) - expect(handleResize).toHaveBeenCalledTimes(1) - }) - - it('repairs a backing left behind by a hidden-time layout change, with no dpr change', () => { - // The reveal bug: a sidebar collapse behind display:none refits the grid to - // 92 cols while xterm's paused render service still holds the 159-col - // canvas, so the compositor rescales the old surface into the new box. - const { pane, handleResize, refresh } = makePane({ - backingWidth: 1272, - backingHeight: 1040, - cssWidth: 1272, - cssHeight: 1040, - dpr: 1, - cols: 92, - rows: 65, - cellWidth: 8, - cellHeight: 16 - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(true) - expect(handleResize).toHaveBeenCalledWith(92, 65) - expect(refresh).toHaveBeenCalledWith(0, 64) - }) - - it('drives the repaint through a paused render service instead of a swallowed refresh', () => { - // Reveal usually runs before xterm's IntersectionObserver catches up; a - // plain refresh would be dropped and leave the resized canvas blank. - const { pane, refresh, refreshRows } = makePane({ - backingWidth: 1272, - backingHeight: 1040, - dpr: 1, - cols: 92, - rows: 65, - cellWidth: 8, - cellHeight: 16, - paused: true - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(true) - expect(refreshRows).toHaveBeenCalledWith(0, 64, true) - expect(refresh).not.toHaveBeenCalled() - }) - - it('is a no-op when the backing matches the current grid and css box', () => { - const { pane, handleResize, refresh } = makePane({ - backingWidth: 2160, - backingHeight: 1200, - dpr: 2, - cols: 120, - rows: 40, - cellWidth: 18, - cellHeight: 30 - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(false) - expect(handleResize).not.toHaveBeenCalled() - expect(refresh).not.toHaveBeenCalled() - }) - - it('tolerates sub-pixel rounding without churning', () => { - // The css round trip can land one device pixel away from the grid product. - const { pane, handleResize } = makePane({ - backingWidth: 2161, - backingHeight: 1200, - dpr: 2, - cols: 120, - rows: 40, - cellWidth: 18, - cellHeight: 30 - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(false) - expect(handleResize).not.toHaveBeenCalled() - }) - - it('tolerates the larger device-pixel round trip on high-dpr displays', () => { - const { pane, handleResize } = makePane({ - backingWidth: 2162, - backingHeight: 1200, - dpr: 4, - cols: 120, - rows: 40, - cellWidth: 18, - cellHeight: 30 - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(false) - expect(handleResize).not.toHaveBeenCalled() - }) - - it('tolerates the device-pixel-content-box write-back at fractional dpr', () => { - // xterm's own observer parks the compositor's rounding of the css box in - // canvas.width while `dimensions.device.canvas` keeps the exact cell - // multiple, so a healthy pane at 150 % scaling sits ~2 device px off the - // grid product. Firing here would refit and repaint on every fit forever. - const { pane, handleResize, refreshRows } = makePane({ - backingWidth: 2162, - backingHeight: 1200, - cssWidth: 1441, - cssHeight: 800, - dpr: 1.5, - cols: 120, - rows: 40, - cellWidth: 18, - cellHeight: 30 - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(false) - expect(handleResize).not.toHaveBeenCalled() - expect(refreshRows).not.toHaveBeenCalled() - }) - - it('still repairs a real drift at fractional dpr', () => { - // Same 150 % display, but the backing is a whole stale grid behind — orders - // of magnitude past the write-back tolerance above. - const { pane, handleResize } = makePane({ - backingWidth: 1272, - backingHeight: 1200, - cssWidth: 848, - cssHeight: 800, - dpr: 1.5, - cols: 120, - rows: 40, - cellWidth: 18, - cellHeight: 30 - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(true) - expect(handleResize).toHaveBeenCalledWith(120, 40) - }) - - it('repairs when only the canvas height has stale backing', () => { - const { pane, handleResize } = makePane({ - backingWidth: 2160, - backingHeight: 600, - dpr: 2, - cols: 120, - rows: 40, - cellWidth: 18, - cellHeight: 30 - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(true) - expect(handleResize).toHaveBeenCalledTimes(1) - }) - - it('falls back to the renderer device canvas when cell metrics are unavailable', () => { - const { pane, handleResize } = makePane({ - backingWidth: 2160, - backingHeight: 1200, - cssWidth: 1080, - cssHeight: 600, - dpr: 1, - deviceCanvasWidth: 1080, - deviceCanvasHeight: 600 - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(true) - expect(handleResize).toHaveBeenCalledTimes(1) - }) - - it('skips a hidden or unmeasurable pane so its parked backing is left alone', () => { - const zeroBox = makePane({ - backingWidth: 1272, - backingHeight: 1040, - dpr: 1, - cols: 92, - rows: 65, - cellWidth: 8, - cellHeight: 16, - paneBox: { width: 0, height: 0 } - }) - expect(repairPaneWebglCanvasBackingMismatch(zeroBox.pane)).toBe(false) - expect(zeroBox.handleResize).not.toHaveBeenCalled() - - const unmeasurable = makePane({ - backingWidth: 1272, - backingHeight: 1040, - dpr: 1, - cols: 92, - rows: 65, - cellWidth: 8, - cellHeight: 16, - paneBox: null - }) - expect(repairPaneWebglCanvasBackingMismatch(unmeasurable.pane)).toBe(false) - expect(unmeasurable.handleResize).not.toHaveBeenCalled() - }) - - it('skips a renderer that cannot recompute its dimensions yet', () => { - // With no measured char size xterm's `_updateDimensions` bails, so the - // resize would re-apply the same stale numbers — an unfixable mismatch that - // would otherwise churn a full resize plus repaint on every fit. - const { pane, handleResize, refreshRows } = makePane({ - backingWidth: 1272, - backingHeight: 1040, - dpr: 1, - cols: 92, - rows: 65, - cellWidth: 8, - cellHeight: 16, - charSizeValid: false - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(false) - expect(handleResize).not.toHaveBeenCalled() - expect(refreshRows).not.toHaveBeenCalled() - }) - - it('skips detached, dimensionless, and renderer-less panes', () => { - const detached = makePane({ - backingWidth: 2160, - cssWidth: 1080, - dpr: 1, - cellWidth: 9, - cellHeight: 15, - connected: false - }) - expect(repairPaneWebglCanvasBackingMismatch(detached.pane)).toBe(false) - - const noExpectation = makePane({ backingWidth: 2160, dpr: 1, cssWidth: 0, cssHeight: 0 }) - expect(repairPaneWebglCanvasBackingMismatch(noExpectation.pane)).toBe(false) - - const noRenderer = makePane({ - backingWidth: 2160, - cssWidth: 1080, - dpr: 1, - hasRenderer: false - }) - expect(repairPaneWebglCanvasBackingMismatch(noRenderer.pane)).toBe(false) - }) - - it('reports failure without throwing when the repair path throws mid-teardown', () => { - const { pane, handleResize } = makePane({ - backingWidth: 2160, - backingHeight: 1200, - cssWidth: 1080, - cssHeight: 600, - dpr: 1, - cellWidth: 9, - cellHeight: 15 - }) - handleResize.mockImplementation(() => { - throw new Error('disposed') - }) - expect(repairPaneWebglCanvasBackingMismatch(pane)).toBe(false) - }) -}) diff --git a/src/renderer/src/lib/pane-manager/terminal-canvas-backing-repair.ts b/src/renderer/src/lib/pane-manager/terminal-canvas-backing-repair.ts deleted file mode 100644 index f76db4f2703..00000000000 --- a/src/renderer/src/lib/pane-manager/terminal-canvas-backing-repair.ts +++ /dev/null @@ -1,166 +0,0 @@ -import type { ManagedPane } from './pane-manager-types' -import { recordTerminalWebglDiagnostic } from '../../../../shared/terminal-webgl-diagnostics' -import { readFitClientSize } from './pane-fit-client-size' -import { forceRepaintThroughRenderPause } from './terminal-render-pause-release' - -/** - * Why: xterm's WebGL canvas can be left holding a backing store that no longer - * matches the box it is composited into, and the browser silently rescales the - * stale bitmap to fit. Two proven ways in: - * - * - devicePixelRatio changed while the pane was hidden (window moved between - * retina/non-retina displays, then the worktree revealed) — the addon's own - * device-pixel observer misses changes that land while the element has no - * box. Proven live: backing 2160 px for a 1080 css box at dpr 1. - * - the grid was re-fitted while `RenderService._isPaused` was still true (a - * layout change made behind `display:none`, then reveal). RenderService - * defers the renderer's `handleResize` onto an idle task there, so the - * freshly measured cols/rows land on the buffer while the canvas keeps its - * pre-hide backing and `.xterm-screen` box — the same text at the wrong - * pitch with thin gray stems, for up to ~1.3 s under load. - * - * Both are one defect: the backing store disagrees with what the current grid - * and dpr require. The repair is xterm's own resize path, driven straight on - * the renderer so a paused render service cannot defer it again. - */ -type XtermRendererInternals = { - _canvas?: HTMLCanvasElement - _gl?: { canvas?: HTMLCanvasElement } - dimensions?: { - device?: { - canvas?: { width?: number; height?: number } - cell?: { width?: number; height?: number } - } - } - handleDevicePixelRatioChange?: () => void - handleResize?: (cols: number, rows: number) => void -} - -type BackingSize = { width: number; height: number } - -type XtermCoreInternals = { - _renderService?: { _renderer?: { value?: XtermRendererInternals } } - _charSizeService?: { hasValidSize?: boolean } -} - -function getPaneXtermCore(pane: ManagedPane): XtermCoreInternals | undefined { - return (pane.terminal as unknown as { _core?: XtermCoreInternals })._core -} - -// Why: `_updateDimensions` bails before writing anything when the char size is -// not measured, so `handleResize` would re-apply the same stale numbers — the -// mismatch is real but unfixable, and retrying it on every fit would churn a -// full resize plus a forced repaint per divider-drag frame. Leave it to the -// fit that follows a successful measure. -function canRendererRecomputeDimensions(core: XtermCoreInternals | undefined): boolean { - const hasValidSize = core?._charSizeService?.hasValidSize - return hasValidSize !== false -} - -// Why recompute rather than trust `device.canvas`: xterm derives that field from -// cols x cell when it last ran `_updateDimensions`, so it goes stale alongside -// the canvas whenever a resize is deferred behind a paused render service. Cell -// metrics only move on font/dpr changes, which the css-box predicate covers. -function expectedBackingForGrid( - renderer: XtermRendererInternals, - pane: ManagedPane -): BackingSize | null { - const device = renderer.dimensions?.device - const cellWidth = device?.cell?.width ?? 0 - const cellHeight = device?.cell?.height ?? 0 - const { cols, rows } = pane.terminal - if (cellWidth > 0 && cellHeight > 0 && cols > 0 && rows > 0) { - return { width: cols * cellWidth, height: rows * cellHeight } - } - const canvasWidth = device?.canvas?.width ?? 0 - const canvasHeight = device?.canvas?.height ?? 0 - return canvasWidth > 0 && canvasHeight > 0 ? { width: canvasWidth, height: canvasHeight } : null -} - -// Why the inline style and not getBoundingClientRect: the renderer writes the -// css size and the backing size in the same pass, so the style is that css box -// verbatim — and this predicate runs on every successful fit, where forcing -// layout would be a real cost. -function expectedBackingForCssBox(canvas: HTMLCanvasElement, dpr: number): BackingSize | null { - const cssWidth = Number.parseFloat(canvas.style.width) - const cssHeight = Number.parseFloat(canvas.style.height) - if (!(cssWidth > 0) || !(cssHeight > 0)) { - return null - } - return { width: Math.round(cssWidth * dpr), height: Math.round(cssHeight * dpr) } -} - -function backingMatches(canvas: HTMLCanvasElement, expected: BackingSize, dpr: number): boolean { - // xterm rounds its css canvas size before converting back to device pixels - // (drift <= dpr/2), and the addon's `device-pixel-content-box` observer then - // writes the compositor's own rounding of that box straight into canvas.width - // while deliberately leaving `dimensions.device.canvas` on the exact cell - // multiple (+/- 1 more). At fractional dpr — Windows 125/150/175 %, scaled - // Retina modes — that steady-state gap exceeds ceil(dpr/2), so the extra pixel - // is what keeps a healthy pane from refitting on every fit. The real defect - // drifts by hundreds of device pixels, so this stays far from masking it. - const tolerance = Math.max(1, Math.ceil(dpr / 2) + 1) - return ( - Math.abs(canvas.width - expected.width) <= tolerance && - Math.abs(canvas.height - expected.height) <= tolerance - ) -} - -export function repairPaneWebglCanvasBackingMismatch(pane: ManagedPane): boolean { - const core = getPaneXtermCore(pane) - const renderer = core?._renderService?._renderer?.value - const canvas = renderer?._canvas ?? renderer?._gl?.canvas - if (!renderer || !canvas?.isConnected || !canRendererRecomputeDimensions(core)) { - return false - } - const dpr = canvas.ownerDocument?.defaultView?.devicePixelRatio ?? 0 - if (!(dpr > 0)) { - return false - } - const grid = expectedBackingForGrid(renderer, pane) - const cssBox = expectedBackingForCssBox(canvas, dpr) - // Report whichever expectation the backing actually failed, so the breadcrumb - // names the real drift rather than a value that already agreed. - let expected: BackingSize | null = null - if (grid && !backingMatches(canvas, grid, dpr)) { - expected = grid - } else if (cssBox && !backingMatches(canvas, cssBox, dpr)) { - expected = cssBox - } - if (!expected) { - return false - } - // Why only now: a hidden pane keeps its pre-hide backing on purpose, and - // resizing it there clears the canvas with nothing able to repaint it. Kept - // behind the mismatch check so the common matching case costs no layout. - const box = readFitClientSize(pane) - if (!box || box.width <= 0 || box.height <= 0) { - return false - } - const staleBackingWidth = canvas.width - const staleBackingHeight = canvas.height - try { - // Order matters: refresh the renderer's cached dpr/dimensions first, then - // the resize path recreates the backing store and layer sizes from them. - renderer.handleDevicePixelRatioChange?.() - renderer.handleResize?.(pane.terminal.cols, pane.terminal.rows) - // Why forced: resizing a canvas clears it, and reveal usually runs while - // xterm's IntersectionObserver still reports paused — a plain refresh is - // swallowed there, trading the rescaled frame for a blank one. - if (!forceRepaintThroughRenderPause(pane.terminal)) { - pane.terminal.refresh(0, pane.terminal.rows - 1) - } - } catch { - // Pane may be mid-teardown; the next reveal/fit retries the check. - return false - } - recordTerminalWebglDiagnostic('webgl-canvas-backing-repair', { - paneId: pane.id, - staleBackingWidth, - staleBackingHeight, - expectedBackingWidth: expected.width, - expectedBackingHeight: expected.height, - devicePixelRatio: dpr - }) - return true -} diff --git a/src/renderer/src/lib/pane-manager/terminal-canvas-dpr-repair.test.ts b/src/renderer/src/lib/pane-manager/terminal-canvas-dpr-repair.test.ts new file mode 100644 index 00000000000..188b8441104 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-canvas-dpr-repair.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, it, vi } from 'vitest' +import type { ManagedPane } from './pane-manager-types' +import { repairPaneWebglCanvasDprMismatch } from './terminal-canvas-dpr-repair' + +function makePane(args: { + backingWidth: number + expectedWidth: number + dpr: number + backingHeight?: number + expectedHeight?: number + connected?: boolean + hasRenderer?: boolean +}): { + pane: ManagedPane + handleDevicePixelRatioChange: ReturnType + handleResize: ReturnType + refresh: ReturnType +} { + const handleDevicePixelRatioChange = vi.fn() + const handleResize = vi.fn() + const refresh = vi.fn() + const canvas = { + width: args.backingWidth, + height: args.backingHeight ?? 1200, + isConnected: args.connected ?? true, + ownerDocument: { defaultView: { devicePixelRatio: args.dpr } }, + getBoundingClientRect: () => { + throw new Error('repair detection must not force layout') + } + } + const renderer = + (args.hasRenderer ?? true) + ? { + _canvas: canvas, + dimensions: { + device: { + canvas: { width: args.expectedWidth, height: args.expectedHeight ?? 1200 } + } + }, + handleDevicePixelRatioChange, + handleResize + } + : undefined + const pane = { + id: 1, + terminal: { + cols: 120, + rows: 40, + refresh, + _core: { _renderService: { _renderer: { value: renderer } } } + } + } as unknown as ManagedPane + return { pane, handleDevicePixelRatioChange, handleResize, refresh } +} + +describe('repairPaneWebglCanvasDprMismatch', () => { + it('repairs a stale dpr-2 backing composited on a dpr-1 display', () => { + // The reproduced field bug: hidden-time display change leaves a 2160px + // backing behind a 1080px css box at dpr 1 (half-size/smeared text). + const { pane, handleDevicePixelRatioChange, handleResize, refresh } = makePane({ + backingWidth: 2160, + expectedWidth: 1080, + dpr: 1 + }) + + expect(repairPaneWebglCanvasDprMismatch(pane)).toBe(true) + expect(handleDevicePixelRatioChange).toHaveBeenCalledTimes(1) + expect(handleResize).toHaveBeenCalledWith(120, 40) + expect(refresh).toHaveBeenCalledWith(0, 39) + // Dpr refresh must precede the resize that rebuilds the backing store. + expect(handleDevicePixelRatioChange.mock.invocationCallOrder[0]!).toBeLessThan( + handleResize.mock.invocationCallOrder[0]! + ) + }) + + it('repairs the opposite direction (dpr-1 backing upscaled on retina)', () => { + const { pane, handleResize } = makePane({ backingWidth: 1080, expectedWidth: 2160, dpr: 2 }) + expect(repairPaneWebglCanvasDprMismatch(pane)).toBe(true) + expect(handleResize).toHaveBeenCalledTimes(1) + }) + + it('is a no-op when backing matches the renderer device dimensions', () => { + const { pane, handleResize, refresh } = makePane({ + backingWidth: 2160, + expectedWidth: 2160, + dpr: 2 + }) + expect(repairPaneWebglCanvasDprMismatch(pane)).toBe(false) + expect(handleResize).not.toHaveBeenCalled() + expect(refresh).not.toHaveBeenCalled() + }) + + it('tolerates sub-pixel rounding without churning', () => { + // ResizeObserver can round one device pixel away from renderer dimensions. + const { pane, handleResize } = makePane({ backingWidth: 2160, expectedWidth: 2161, dpr: 2 }) + expect(repairPaneWebglCanvasDprMismatch(pane)).toBe(false) + expect(handleResize).not.toHaveBeenCalled() + }) + + it('tolerates the larger device-pixel round trip on high-dpr displays', () => { + const { pane, handleResize } = makePane({ backingWidth: 2160, expectedWidth: 2162, dpr: 4 }) + expect(repairPaneWebglCanvasDprMismatch(pane)).toBe(false) + expect(handleResize).not.toHaveBeenCalled() + }) + + it('repairs when only the canvas height has stale backing', () => { + const { pane, handleResize } = makePane({ + backingWidth: 2160, + expectedWidth: 2160, + dpr: 2, + backingHeight: 600, + expectedHeight: 1200 + }) + expect(repairPaneWebglCanvasDprMismatch(pane)).toBe(true) + expect(handleResize).toHaveBeenCalledTimes(1) + }) + + it('skips detached, zero-dimension, and renderer-less panes', () => { + const detached = makePane({ + backingWidth: 2160, + expectedWidth: 1080, + dpr: 1, + connected: false + }) + expect(repairPaneWebglCanvasDprMismatch(detached.pane)).toBe(false) + + const zeroDimension = makePane({ backingWidth: 2160, expectedWidth: 0, dpr: 1 }) + expect(repairPaneWebglCanvasDprMismatch(zeroDimension.pane)).toBe(false) + + const noRenderer = makePane({ + backingWidth: 2160, + expectedWidth: 1080, + dpr: 1, + hasRenderer: false + }) + expect(repairPaneWebglCanvasDprMismatch(noRenderer.pane)).toBe(false) + }) + + it('reports failure without throwing when the repair path throws mid-teardown', () => { + const { pane, handleResize } = makePane({ + backingWidth: 2160, + expectedWidth: 1080, + dpr: 1 + }) + handleResize.mockImplementation(() => { + throw new Error('disposed') + }) + expect(repairPaneWebglCanvasDprMismatch(pane)).toBe(false) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/terminal-canvas-dpr-repair.ts b/src/renderer/src/lib/pane-manager/terminal-canvas-dpr-repair.ts new file mode 100644 index 00000000000..72bde40b30b --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-canvas-dpr-repair.ts @@ -0,0 +1,70 @@ +import type { ManagedPane } from './pane-manager-types' +import { recordTerminalWebglDiagnostic } from '../../../../shared/terminal-webgl-diagnostics' + +/** + * Why: when devicePixelRatio changes while a pane is hidden (window moved + * between retina/non-retina displays, then the worktree revealed), xterm's + * WebGL renderer re-measures cell dimensions but its canvas keeps the old + * backing-store size — the addon's own device-pixel observer misses changes + * that land while the element has no box. The browser then composites the + * stale-scale bitmap into the css box: half/double-size or smeared text until + * a real resize. Proven live: backing 2160 px for a 1080 css box at dpr 1. + * The repair is xterm's own resize path, which recomputes device dimensions + * at the current dpr and resizes the canvas backing store. + */ +type XtermRendererInternals = { + _canvas?: HTMLCanvasElement + _gl?: { canvas?: HTMLCanvasElement } + dimensions?: { + device?: { canvas?: { width?: number; height?: number } } + } + handleDevicePixelRatioChange?: () => void + handleResize?: (cols: number, rows: number) => void +} + +export function repairPaneWebglCanvasDprMismatch(pane: ManagedPane): boolean { + const renderer = ( + pane.terminal as unknown as { + _core?: { _renderService?: { _renderer?: { value?: XtermRendererInternals } } } + } + )._core?._renderService?._renderer?.value + const canvas = renderer?._canvas ?? renderer?._gl?.canvas + if (!renderer || !canvas?.isConnected) { + return false + } + const view = canvas.ownerDocument?.defaultView + const expected = renderer.dimensions?.device?.canvas + const expectedWidth = expected?.width ?? 0 + const expectedHeight = expected?.height ?? 0 + if (!view || expectedWidth <= 0 || expectedHeight <= 0) { + return false + } + const staleBackingWidth = canvas.width + const staleBackingHeight = canvas.height + // xterm rounds its CSS canvas size before ResizeObserver converts it back to + // device pixels; allow that round trip without forcing layout on every fit. + const roundingTolerance = Math.max(1, Math.ceil(view.devicePixelRatio / 2)) + if ( + Math.abs(staleBackingWidth - expectedWidth) <= roundingTolerance && + Math.abs(staleBackingHeight - expectedHeight) <= roundingTolerance + ) { + return false + } + try { + // Order matters: refresh the renderer's cached dpr/dimensions first, then + // the resize path recreates the backing store and layer sizes from them. + renderer.handleDevicePixelRatioChange?.() + renderer.handleResize?.(pane.terminal.cols, pane.terminal.rows) + pane.terminal.refresh(0, pane.terminal.rows - 1) + } catch { + // Pane may be mid-teardown; the next reveal/fit retries the check. + return false + } + recordTerminalWebglDiagnostic('webgl-canvas-dpr-repair', { + paneId: pane.id, + staleBackingWidth, + expectedBackingWidth: expectedWidth, + devicePixelRatio: view.devicePixelRatio + }) + return true +}