diff --git a/.oxlintrc.json b/.oxlintrc.json index 4cdb4f2c0..0f679f17d 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -150,10 +150,5 @@ } } ], - "ignorePatterns": [ - "**/node_modules", - "**/dist", - "**/out", - "tests/e2e/.cross-version-checkouts" - ] + "ignorePatterns": ["**/node_modules", "**/dist", "**/out", "tests/e2e/.cross-version-checkouts"] } diff --git a/config/scripts/relay-watcher-fault-harness.mjs b/config/scripts/relay-watcher-fault-harness.mjs index 12e36f724..b27a1f945 100644 --- a/config/scripts/relay-watcher-fault-harness.mjs +++ b/config/scripts/relay-watcher-fault-harness.mjs @@ -109,9 +109,7 @@ function waitForStdoutSentinel(proc, protocol, stderr) { settled = true proc.stdout.off('data', onData) rejectPromise( - new Error( - `process exited before sentinel (code=${code}, signal=${signal})\n${stderr()}` - ) + new Error(`process exited before sentinel (code=${code}, signal=${signal})\n${stderr()}`) ) } proc.stdout.on('data', onData) @@ -162,11 +160,7 @@ function createRelayClient(entryPath, args, env, protocol) { }) const waitForMessage = (startIndex, predicate, label) => - pollUntil( - () => messages.slice(startIndex).find(predicate), - label, - streams.stderr - ) + pollUntil(() => messages.slice(startIndex).find(predicate), label, streams.stderr) const request = async (method, params = {}) => { const id = nextSequence++ @@ -317,8 +311,10 @@ async function main() { ) await relay.request('fs.watch', { rootPath: watchRoot }) - const firstWatcherPid = await waitForWatcherPid(pidFile, undefined, () => - `${daemonStreams.stderr()}\n${relay.stderr()}` + const firstWatcherPid = await waitForWatcherPid( + pidFile, + undefined, + () => `${daemonStreams.stderr()}\n${relay.stderr()}` ) const beforePath = join(watchRoot, 'before.txt') startIndex = relay.messageCount() @@ -330,8 +326,10 @@ async function main() { const faultSignal = process.platform === 'win32' ? 'SIGTERM' : 'SIGSEGV' startIndex = relay.messageCount() process.kill(firstWatcherPid, faultSignal) - const replacementWatcherPid = await waitForWatcherPid(pidFile, firstWatcherPid, () => - `${daemonStreams.stderr()}\n${relay.stderr()}` + const replacementWatcherPid = await waitForWatcherPid( + pidFile, + firstWatcherPid, + () => `${daemonStreams.stderr()}\n${relay.stderr()}` ) await relay.waitForNotification(startIndex, 'fs.changed', (params) => Array.isArray(params.events) diff --git a/src/renderer/src/components/github/GitHubMarkdownComposerEditorPane.tsx b/src/renderer/src/components/github/GitHubMarkdownComposerEditorPane.tsx index 6914f9b3a..792799376 100644 --- a/src/renderer/src/components/github/GitHubMarkdownComposerEditorPane.tsx +++ b/src/renderer/src/components/github/GitHubMarkdownComposerEditorPane.tsx @@ -13,7 +13,10 @@ export function GitHubMarkdownComposerEditorPane({ const scrollContainerRef = useRef(null) return ( -
+
{ }) describe('applyTerminalAppearance theme assignment', () => { // xterm rebuilds the palette on any new theme-object identity (wiping OSC color mutations), so the assignment must be value-gated. - function makePane(id: number): ManagedPane { - return { id, terminal: { options: {}, cols: 80, rows: 24 } } as unknown as ManagedPane + // Measurable by default: metric options (fontSize/fontFamily/…) only land on + // panes that can measure; unmeasurable panes defer them until fit/reveal. + function makePane(id: number, overrides?: { measurable?: boolean }): ManagedPane { + const measurable = overrides?.measurable ?? true + return { + id, + terminal: { options: {}, cols: 80, rows: 24 }, + container: { + dataset: {}, + getBoundingClientRect: () => ({ width: measurable ? 800 : 0, height: measurable ? 600 : 0 }) + }, + fitAddon: { + proposeDimensions: () => (measurable ? { cols: 80, rows: 24 } : undefined) + } + } as unknown as ManagedPane } function makeManager(panes: ManagedPane[]): PaneManager { return { - getPanes: () => panes, + // Mirrors the real getPanes(), which allocates a fresh toPublicPane() + // wrapper per call over a shared terminal — per-pane state must survive that. + getPanes: () => panes.map((pane) => ({ ...pane })), setPaneLigaturesEnabled: vi.fn(), setPaneStyleOptions: vi.fn() } as unknown as PaneManager @@ -265,6 +281,75 @@ describe('applyTerminalAppearance theme assignment', () => { // The value-gate must not rewrite an unchanged ratio — each write clears xterm's contrast cache. expect(writes).toBe(writesAfterFirst) }) + + it('defers metric options on an unmeasurable pane and lands them on the next fit', () => { + // A metric write makes xterm clear, resize and full-refresh; on a pane with + // no usable box that repaint is wasted and the cols/rows re-fit that must + // follow it cannot run. The write waits for a measurable pane. + let measurable = false + const pane = { + id: 1, + terminal: { options: {}, cols: 80, rows: 24 }, + container: { + dataset: {}, + getBoundingClientRect: () => ({ width: measurable ? 800 : 0, height: measurable ? 600 : 0 }) + }, + fitAddon: { + fit: vi.fn(), + proposeDimensions: () => (measurable ? { cols: 80, rows: 24 } : undefined) + } + } as unknown as ManagedPane + const settings = getDefaultSettings('/tmp') + + apply(pane, { ...settings, terminalFontSize: 19 }) + + expect(pane.terminal.options.fontSize).toBeUndefined() + expect(pane.terminal.options.fontFamily).toBeUndefined() + // Non-metric options are safe while hidden and must not be deferred with them. + expect(pane.terminal.options.cursorStyle).toBeDefined() + + measurable = true + safeFit(pane) + + expect(pane.terminal.options.fontSize).toBe(19) + expect(pane.terminal.options.fontFamily).toContain('monospace') + }) + + it('applies only the latest deferred metric options after repeated hidden changes', () => { + let measurable = false + const writes: number[] = [] + const options: Record = {} + Object.defineProperty(options, 'fontSize', { + configurable: true, + enumerable: true, + get: () => writes.at(-1), + set: (value: number) => { + writes.push(value) + } + }) + const pane = { + id: 1, + terminal: { options, cols: 80, rows: 24 }, + container: { + dataset: {}, + getBoundingClientRect: () => ({ width: measurable ? 800 : 0, height: measurable ? 600 : 0 }) + }, + fitAddon: { + fit: vi.fn(), + proposeDimensions: () => (measurable ? { cols: 80, rows: 24 } : undefined) + } + } as unknown as ManagedPane + const settings = getDefaultSettings('/tmp') + + apply(pane, { ...settings, terminalFontSize: 15 }) + apply(pane, { ...settings, terminalFontSize: 21 }) + + measurable = true + safeFit(pane) + + // Latest wins, exactly one write: intermediate hidden values never touch xterm. + expect(writes).toEqual([21]) + }) }) describe('publishTerminalViewAttributesAtAppStart', () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-appearance.ts b/src/renderer/src/components/terminal-pane/terminal-appearance.ts index c1ff2c24f..f9dbd9a1a 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.ts @@ -10,6 +10,11 @@ import { } from '@/lib/terminal-theme' import { buildFontFamily } from './layout-serialization' import { safeFit, safeFitAndThen } from '@/lib/pane-manager/pane-tree-ops' +import { canApplyPaneMetricOptions } from '@/lib/pane-manager/pane-fit' +import { + applyOrDeferPaneMetricOptions, + paneMetricOptionsAlreadySettled +} from '@/lib/pane-manager/pane-metric-options-deferral' import { normalizeTerminalFastScrollSensitivity, normalizeTerminalScrollSensitivity, @@ -174,10 +179,21 @@ export function applyTerminalAppearance( pane.terminal.options.cursorInactiveStyle = resolveTerminalCursorInactiveStyle(cursorStyle) pane.terminal.options.cursorBlink = settings.terminalCursorBlink const paneSize = paneFontSizes.get(pane.id) - pane.terminal.options.fontSize = paneSize ?? settings.terminalFontSize - pane.terminal.options.fontFamily = buildFontFamily(settings.terminalFontFamily) - pane.terminal.options.fontWeight = terminalFontWeights.fontWeight - pane.terminal.options.fontWeightBold = terminalFontWeights.fontWeightBold + const metricOptions = { + fontSize: paneSize ?? settings.terminalFontSize, + fontFamily: buildFontFamily(settings.terminalFontFamily), + fontWeight: terminalFontWeights.fontWeight, + fontWeightBold: terminalFontWeights.fontWeightBold, + lineHeight: normalizeTerminalLineHeight(settings.terminalLineHeight) + } + // Why value-gated: any settings write re-runs this over every mounted pane, and + // canApplyPaneMetricOptions forces style+layout; an unchanged no-op deferral + // would also arm a pointless refit on the next reveal. + // Why deferred: a metric write makes xterm clear/resize/full-refresh, which is + // wasted on a pane with no usable box and whose follow-up cols/rows fit can't run. + if (!paneMetricOptionsAlreadySettled(pane, metricOptions)) { + applyOrDeferPaneMetricOptions(pane, metricOptions, canApplyPaneMetricOptions(pane)) + } pane.terminal.options.scrollSensitivity = normalizeTerminalScrollSensitivity( settings.terminalScrollSensitivity ) @@ -186,7 +202,6 @@ export function applyTerminalAppearance( ) // Why only 'true': 'left'/'right' are handled in the keydown policy, which needs Option composable at the xterm level. pane.terminal.options.macOptionIsMeta = effectiveMacOptionAsAlt === 'true' - pane.terminal.options.lineHeight = normalizeTerminalLineHeight(settings.terminalLineHeight) // Why unconditional: the helper no-ops when addon state already matches, so this keeps new panes and live toggles in sync. manager.setPaneLigaturesEnabled(pane.id, ligaturesEnabled) const transport = paneTransports.get(pane.id) 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 e2d4aac29..c6b82f247 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 @@ -27,6 +27,11 @@ vi.mock('./terminal-webgl-atlas-recovery', () => ({ // the terminal-output debounce (which a background stream could otherwise defer). scheduleTabRevealWebglAtlasRecovery: () => scheduleTabRevealWebglAtlasRecovery() })) +const flushDeferredPaneMetricOptionsIfMeasurable = vi.fn((_pane: unknown) => false) +vi.mock('@/lib/pane-manager/pane-fit', () => ({ + flushDeferredPaneMetricOptionsIfMeasurable: (pane: unknown) => + flushDeferredPaneMetricOptionsIfMeasurable(pane) +})) const resetTerminalLinkifierHoverState = vi.fn() const isTerminalLinkifierHoverActive = vi.fn((_terminal: unknown) => false) vi.mock('@/lib/pane-manager/terminal-linkifier-hover-reset', () => ({ @@ -131,6 +136,17 @@ describe('resumeTerminalVisibility reveal repaint', () => { expect(manager.fitAllPanes).not.toHaveBeenCalled() }) + it('leaves heavy metric flushing to the reveal fit after rendering resumes', () => { + const manager = createManager() + manager.getPanes.mockReturnValue([{ terminal: {} }]) + + resumeTerminalVisibility(resumeArgs(manager, false)) + + expect(manager.resumeRendering).toHaveBeenCalledTimes(1) + expect(manager.fitAllRevealedPanes).toHaveBeenCalledTimes(1) + expect(flushDeferredPaneMetricOptionsIfMeasurable).not.toHaveBeenCalled() + }) + it('does not fit on a light tab reveal', () => { const manager = createManager() resumeTerminalVisibility(resumeArgs(manager, true)) @@ -139,6 +155,19 @@ describe('resumeTerminalVisibility reveal repaint', () => { expect(manager.fitAllPanes).not.toHaveBeenCalled() }) + it('flushes hidden-era metric options on reveal and refits the light path', () => { + // A font change while hidden must land and refit on reveal, or cols/rows + // stay pinned to the old metrics. + const manager = createManager() + manager.getPanes.mockReturnValue([{ terminal: {} }]) + flushDeferredPaneMetricOptionsIfMeasurable.mockReturnValueOnce(true) + + resumeTerminalVisibility(resumeArgs(manager, true)) + + expect(flushDeferredPaneMetricOptionsIfMeasurable).toHaveBeenCalledTimes(1) + expect(manager.fitAllRevealedPanes).toHaveBeenCalledTimes(1) + }) + it('fits window wake recovery through the stable path, not the sync fit', () => { const manager = createManager() recoverVisibleTerminalWindowWake({ 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 c9cf2a892..01d3627d2 100644 --- a/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts +++ b/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts @@ -15,6 +15,7 @@ import { } from '@/lib/pane-manager/terminal-linkifier-hover-reset' import { focusActivePane } from './pane-helpers' import { scheduleTabRevealWebglAtlasRecovery } from './terminal-webgl-atlas-recovery' +import { flushDeferredPaneMetricOptionsIfMeasurable } from '@/lib/pane-manager/pane-fit' const VISIBLE_RESUME_FLUSH_CHARS = 256 * 1024 const WINDOW_WAKE_FLUSH_CHARS = 64 * 1024 @@ -72,6 +73,12 @@ export function resumeTerminalVisibility({ captureViewportPositions(!wasVisible) withSuppressedScrollTracking(() => { if (shouldUseLightTabResume) { + let flushedDeferredMetrics = false + for (const pane of manager.getPanes()) { + if (flushDeferredPaneMetricOptionsIfMeasurable(pane)) { + flushedDeferredMetrics = true + } + } // Why: intra-worktree tab switches only toggle the overlay. Keeping // synchronous drain and atlas rebuilds off this path avoids racing the // overlay's delayed geometry fit. Still request hidden-output recovery: @@ -81,10 +88,17 @@ export function resumeTerminalVisibility({ // — a background agent streaming in another pane must not defer this tab's // atlas rebuild. scheduleTabRevealWebglAtlasRecovery() + if (flushedDeferredMetrics) { + // Why: the light path normally skips fitting, but flushed metrics changed + // cell size — refit so cols/rows match before the overlay settles. + manager.fitAllRevealedPanes() + } if (isActive) { focusActivePane(manager) } } else { + // fitAllRevealedPanes flushes after WebGL reattaches, avoiding a redundant + // full refresh in the suspended DOM renderer while preserving first paint. resumeTerminalVisibilityHeavy(manager, isActive) } enforceTerminalViewportIntents(manager) diff --git a/src/renderer/src/components/terminal-pane/useTerminalFontZoom.ts b/src/renderer/src/components/terminal-pane/useTerminalFontZoom.ts index 1d2bd6c22..d53cda926 100644 --- a/src/renderer/src/components/terminal-pane/useTerminalFontZoom.ts +++ b/src/renderer/src/components/terminal-pane/useTerminalFontZoom.ts @@ -2,6 +2,7 @@ import { useEffect } from 'react' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import { dispatchZoomLevelChanged } from '@/lib/zoom-events' import { safeFit } from '@/lib/pane-manager/pane-tree-ops' +import { overridePendingPaneMetricOptions } from '@/lib/pane-manager/pane-metric-options-deferral' import { getPaneOwnedActiveHelperTextarea } from './regular-terminal-focus-ownership' type FontZoomDeps = { @@ -57,6 +58,10 @@ export function useTerminalFontZoom({ } pane.terminal.options.fontSize = nextSize + // Why: safeFit flushes parked metric options, which would otherwise + // overwrite this zoom with the font size captured while the pane was + // unmeasurable. Fold the new size in; other parked keys still apply. + overridePendingPaneMetricOptions(pane, { fontSize: nextSize }) safeFit(pane) const percent = Math.round((nextSize / globalSize) * 100) diff --git a/src/renderer/src/lib/pane-manager/pane-fit-measurability.ts b/src/renderer/src/lib/pane-manager/pane-fit-measurability.ts new file mode 100644 index 000000000..571863744 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-fit-measurability.ts @@ -0,0 +1,63 @@ +import type { ManagedPane } from './pane-manager-types' +import { isManagedPaneDisplayNone } from './pane-display-visibility' +import { + flushDeferredPaneMetricOptions, + hasDeferredPaneMetricOptions +} from './pane-metric-options-deferral' + +const MIN_PANE_FIT_WIDTH_PX = 48 +const MIN_PANE_FIT_HEIGHT_PX = 24 +const MIN_PANE_FIT_COLS = 8 +const MIN_PANE_FIT_ROWS = 4 + +export function getProposedPaneDimensions( + pane: ManagedPane +): { cols: number; rows: number } | null { + try { + return pane.fitAddon.proposeDimensions() ?? null + } catch { + return null + } +} + +function hasPaneFitPixelBox(pane: ManagedPane): boolean { + const measure = pane.container?.getBoundingClientRect + if (typeof measure !== 'function') { + return true + } + const rect = measure.call(pane.container) + return rect.width >= MIN_PANE_FIT_WIDTH_PX && rect.height >= MIN_PANE_FIT_HEIGHT_PX +} + +export function canMeasurePaneForFit(pane: ManagedPane): boolean { + if (!hasPaneFitPixelBox(pane)) { + return false + } + const dims = getProposedPaneDimensions(pane) + if (!dims) { + return false + } + // Why: worktree switches can briefly measure a near-zero overlay before + // fallback positioning lands. Fitting there pins the PTY at ~2 cols. + return dims.cols >= MIN_PANE_FIT_COLS && dims.rows >= MIN_PANE_FIT_ROWS +} + +/** Why only the pixel box, not the fit floor: a pane held at the 50px divider + * clamp proposes ~5 cols, so the fit floor would reject it forever — it never + * hides and its box never changes, so nothing would flush and it would render + * a stale font until widened. A hidden pane or the transient worktree-switch + * overlay is near-zero, so the pixel floor still defers those. The cols/rows + * floor stays where it belongs: on the fit. */ +export function canApplyPaneMetricOptions(pane: ManagedPane): boolean { + return !isManagedPaneDisplayNone(pane) && hasPaneFitPixelBox(pane) +} + +/** Why the pending check comes first: it is an O(1) WeakMap lookup, while the + * measurability probe forces style+layout. This runs per pane on every reveal, + * so the common no-deferral case must cost zero DOM reads. */ +export function flushDeferredPaneMetricOptionsIfMeasurable(pane: ManagedPane): boolean { + if (!hasDeferredPaneMetricOptions(pane) || !canApplyPaneMetricOptions(pane)) { + return false + } + return flushDeferredPaneMetricOptions(pane) +} diff --git a/src/renderer/src/lib/pane-manager/pane-fit.test.ts b/src/renderer/src/lib/pane-manager/pane-fit.test.ts index e7975eec1..1e0a9cf92 100644 --- a/src/renderer/src/lib/pane-manager/pane-fit.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-fit.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder' import type { ManagedPane, ManagedPaneInternal, ScrollState } from './pane-manager-types' import { safeFit, safeFitAndThen } from './pane-fit' +import { applyOrDeferPaneMetricOptions } from './pane-metric-options-deferral' import { paneFitClientSizeChanged } from './pane-reveal-fit' vi.mock('@/lib/crash-breadcrumb-recorder', () => ({ @@ -321,3 +322,42 @@ describe('paneFitClientSizeChanged (reveal fit gate)', () => { expect(paneFitClientSizeChanged(pane)).toBe(true) }) }) + +describe('deferred metric flush inside safeFit', () => { + function createMetricPane(): ManagedPane & { fitAddon: { fit: ReturnType } } { + const terminal = { cols: 80, rows: 24, options: {} as Record } + // Grid shrinks once the parked large font lands — the case the min-dimension + // gate exists to reject, but which it can only see after the flush. + const proposeDimensions = (): { cols: number; rows: number } => + Number(terminal.options.fontSize ?? 10) >= 24 ? { cols: 5, rows: 2 } : { cols: 40, rows: 20 } + return { + id: 11, + terminal, + container: { + dataset: {}, + getBoundingClientRect: () => ({ width: 340, height: 240 }) + }, + fitAddon: { fit: vi.fn(), proposeDimensions: vi.fn(proposeDimensions) } + } as unknown as ManagedPane & { fitAddon: { fit: ReturnType } } + } + + it('does not fit when the flushed font drops the pane under the minimum grid', () => { + const pane = createMetricPane() + applyOrDeferPaneMetricOptions(pane, { fontSize: 24 }, false) + + expect(safeFit(pane)).toBe(false) + // The parked value still lands so the pane is not stuck on stale metrics. + expect(pane.terminal.options.fontSize).toBe(24) + // But the PTY must not be pinned to the 5x2 grid the floor rejects. + expect(pane.fitAddon.fit).not.toHaveBeenCalled() + }) + + it('still fits when the flushed font keeps the pane above the minimum grid', () => { + const pane = createMetricPane() + applyOrDeferPaneMetricOptions(pane, { fontSize: 12 }, false) + + expect(safeFit(pane)).toBe(true) + expect(pane.terminal.options.fontSize).toBe(12) + expect(pane.fitAddon.fit).toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-fit.ts b/src/renderer/src/lib/pane-manager/pane-fit.ts index dcb638c0a..0b7d3bd8e 100644 --- a/src/renderer/src/lib/pane-manager/pane-fit.ts +++ b/src/renderer/src/lib/pane-manager/pane-fit.ts @@ -21,13 +21,16 @@ import { deferTerminalGeometryMutationDuringRebuild, isTerminalScrollIntentRebuildInFlight } from './terminal-scroll-intent-rebuild' +import { flushDeferredPaneMetricOptions } from './pane-metric-options-deferral' +import { canMeasurePaneForFit, getProposedPaneDimensions } from './pane-fit-measurability' import { notifyPaneFitSucceeded } from './pane-fit-webgl-attach-signal' import { recordPaneFitClientSize } from './pane-fit-client-size' -const MIN_PANE_FIT_WIDTH_PX = 48 -const MIN_PANE_FIT_HEIGHT_PX = 24 -const MIN_PANE_FIT_COLS = 8 -const MIN_PANE_FIT_ROWS = 4 +export { + canApplyPaneMetricOptions, + canMeasurePaneForFit, + flushDeferredPaneMetricOptionsIfMeasurable +} from './pane-fit-measurability' export type SafeFitContinuationHandle = { completion: Promise @@ -45,33 +48,8 @@ const pendingSafeFitContinuations = new WeakMap< Map >() -function getProposedDimensions(pane: ManagedPane): { cols: number; rows: number } | null { - try { - return pane.fitAddon.proposeDimensions() ?? null - } catch { - return null - } -} - export { readFitClientSize } from './pane-fit-client-size' -export function canMeasurePaneForFit(pane: ManagedPane): boolean { - const measure = pane.container?.getBoundingClientRect - if (typeof measure === 'function') { - const rect = measure.call(pane.container) - if (rect.width < MIN_PANE_FIT_WIDTH_PX || rect.height < MIN_PANE_FIT_HEIGHT_PX) { - return false - } - } - const dims = getProposedDimensions(pane) - if (!dims) { - return false - } - // Why: worktree switches can briefly measure a near-zero overlay before - // fallback positioning lands. Fitting there pins the PTY at ~2 cols. - return dims.cols >= MIN_PANE_FIT_COLS && dims.rows >= MIN_PANE_FIT_ROWS -} - function canPreserveScrollIntentForFit(pane: ManagedPane): boolean { // Why: split reparent has its own delayed restore; restoring here can fight that timer. return !( @@ -86,6 +64,14 @@ function performSafeFit(pane: ManagedPane): boolean { if (!canMeasurePaneForFit(pane)) { return false } + // Why here: metric options deferred while the pane was unmeasurable must land + // before this fit reads dimensions, or the fit pins cols/rows to stale metrics. + // Why re-check: the gate above measured with the old cell size — a large font + // jump on a narrow pane can drop it under the floor, and fit() would then pin + // the PTY at the tiny grid that floor exists to prevent. + if (flushDeferredPaneMetricOptions(pane) && !canMeasurePaneForFit(pane)) { + return false + } let scrollIntent = null as ReturnType let pinnedScrollState: ScrollState | null = null let shouldRestoreScroll = false @@ -113,7 +99,7 @@ function performSafeFit(pane: ManagedPane): boolean { return true } - const dims = getProposedDimensions(pane) + const dims = getProposedPaneDimensions(pane) if (dims && dims.cols === pane.terminal.cols && dims.rows === pane.terminal.rows) { // Why: divider drags often stay within one cell; avoid needless clear/refresh churn. resumePendingFitScrollRestoreAfterFit(pane.terminal) diff --git a/src/renderer/src/lib/pane-manager/pane-metric-options-deferral.test.ts b/src/renderer/src/lib/pane-manager/pane-metric-options-deferral.test.ts new file mode 100644 index 000000000..e07ae670d --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-metric-options-deferral.test.ts @@ -0,0 +1,167 @@ +import { describe, expect, it } from 'vitest' +import type { ManagedPane, ManagedPaneInternal } from './pane-manager-types' +import { toPublicPane } from './pane-public-view' +import { canApplyPaneMetricOptions, canMeasurePaneForFit } from './pane-fit-measurability' +import { + applyOrDeferPaneMetricOptions, + flushDeferredPaneMetricOptions, + hasDeferredPaneMetricOptions, + overridePendingPaneMetricOptions, + paneMetricOptionsAlreadySettled +} from './pane-metric-options-deferral' + +function makePane(): ManagedPane { + return { id: 1, terminal: { options: {} } } as unknown as ManagedPane +} + +// Mirrors PaneManager.getPanes(), which returns a fresh toPublicPane() wrapper +// per call — so deferral state must not be keyed on the pane object identity. +function makeInternalPane(): ManagedPaneInternal { + return { id: 1, terminal: { options: {} } } as unknown as ManagedPaneInternal +} + +describe('pane-metric-options-deferral', () => { + it('writes metric options directly when the pane is measurable', () => { + const pane = makePane() + + const result = applyOrDeferPaneMetricOptions(pane, { fontSize: 16, fontFamily: 'X' }, true) + + expect(result).toBe('applied') + expect(pane.terminal.options.fontSize).toBe(16) + expect(pane.terminal.options.fontFamily).toBe('X') + expect(hasDeferredPaneMetricOptions(pane)).toBe(false) + }) + + it('defers writes on an unmeasurable pane until flushed', () => { + const pane = makePane() + + const result = applyOrDeferPaneMetricOptions(pane, { fontSize: 16 }, false) + + expect(result).toBe('deferred') + expect(pane.terminal.options.fontSize).toBeUndefined() + expect(hasDeferredPaneMetricOptions(pane)).toBe(true) + + expect(flushDeferredPaneMetricOptions(pane)).toBe(true) + expect(pane.terminal.options.fontSize).toBe(16) + expect(hasDeferredPaneMetricOptions(pane)).toBe(false) + }) + + it('keeps only the latest deferral and clears it when a measurable apply lands', () => { + const pane = makePane() + + applyOrDeferPaneMetricOptions(pane, { fontSize: 15 }, false) + applyOrDeferPaneMetricOptions(pane, { fontSize: 21 }, false) + // A later measurable apply supersedes the pending deferral entirely: flushing + // afterwards must not resurrect the hidden-era value. + applyOrDeferPaneMetricOptions(pane, { fontSize: 18 }, true) + + expect(pane.terminal.options.fontSize).toBe(18) + expect(flushDeferredPaneMetricOptions(pane)).toBe(false) + expect(pane.terminal.options.fontSize).toBe(18) + }) + + it('writes only the provided keys', () => { + const pane = makePane() + pane.terminal.options.lineHeight = 1.4 + + applyOrDeferPaneMetricOptions(pane, { fontSize: 12 }, true) + + expect(pane.terminal.options.lineHeight).toBe(1.4) + expect(pane.terminal.options.fontWeight).toBeUndefined() + }) + + it('flush is a no-op without a pending deferral', () => { + const pane = makePane() + expect(flushDeferredPaneMetricOptions(pane)).toBe(false) + }) + + it('flushes through a different pane view than the one that deferred', () => { + const internal = makeInternalPane() + const deferView = toPublicPane(internal) + const flushView = toPublicPane(internal) + // Every getPanes() call allocates a new wrapper; the two views are the same + // pane but never the same object. + expect(deferView).not.toBe(flushView) + + applyOrDeferPaneMetricOptions(deferView, { fontSize: 16 }, false) + + expect(hasDeferredPaneMetricOptions(flushView)).toBe(true) + expect(flushDeferredPaneMetricOptions(flushView)).toBe(true) + expect(internal.terminal.options.fontSize).toBe(16) + }) + + it('flushes through the internal pane when a public view deferred', () => { + // fitAllPanesInternal / fitAllRevealedPanes iterate the internal panes. + const internal = makeInternalPane() + + applyOrDeferPaneMetricOptions(toPublicPane(internal), { fontSize: 17 }, false) + + expect(flushDeferredPaneMetricOptions(internal)).toBe(true) + expect(internal.terminal.options.fontSize).toBe(17) + }) + + it('reports settled only when every value is live and nothing is parked', () => { + const pane = makePane() + applyOrDeferPaneMetricOptions(pane, { fontSize: 14, lineHeight: 1.2 }, true) + + expect(paneMetricOptionsAlreadySettled(pane, { fontSize: 14, lineHeight: 1.2 })).toBe(true) + expect(paneMetricOptionsAlreadySettled(pane, { fontSize: 15, lineHeight: 1.2 })).toBe(false) + }) + + it('is never settled while a deferral is parked, even if values match', () => { + // Otherwise an equal-valued apply would skip and strand the stale deferral. + const pane = makePane() + applyOrDeferPaneMetricOptions(pane, { fontSize: 20 }, false) + pane.terminal.options.fontSize = 14 + + expect(paneMetricOptionsAlreadySettled(pane, { fontSize: 14 })).toBe(false) + }) + + it('folds a direct write into a pending deferral so the flush cannot clobber it', () => { + const pane = makePane() + applyOrDeferPaneMetricOptions(pane, { fontSize: 12, lineHeight: 1.5 }, false) + + // Font zoom writes fontSize directly, then fits — which flushes. + pane.terminal.options.fontSize = 22 + overridePendingPaneMetricOptions(pane, { fontSize: 22 }) + flushDeferredPaneMetricOptions(pane) + + expect(pane.terminal.options.fontSize).toBe(22) + // The other parked key still lands. + expect(pane.terminal.options.lineHeight).toBe(1.5) + }) + + it('override is a no-op when nothing is parked', () => { + const pane = makePane() + overridePendingPaneMetricOptions(pane, { fontSize: 22 }) + + expect(hasDeferredPaneMetricOptions(pane)).toBe(false) + expect(pane.terminal.options.fontSize).toBeUndefined() + }) +}) + +describe('canApplyPaneMetricOptions gating', () => { + function makeSizedPane(rect: { width: number; height: number }, cols: number): ManagedPane { + return { + id: 2, + terminal: { options: {} }, + container: { getBoundingClientRect: () => rect }, + fitAddon: { proposeDimensions: () => ({ cols, rows: 20 }) } + } as unknown as ManagedPane + } + + it('applies to a pane clamped narrow by a divider drag', () => { + // 50px is the divider clamp: over the pixel floor but ~5 cols, under the + // fit floor. Gating on the fit floor would strand it on a stale font, + // because it never hides and its box never changes. + expect(canApplyPaneMetricOptions(makeSizedPane({ width: 50, height: 600 }, 5))).toBe(true) + }) + + it('still defers on a near-zero box (hidden pane / worktree-switch overlay)', () => { + expect(canApplyPaneMetricOptions(makeSizedPane({ width: 0, height: 0 }, 0))).toBe(false) + }) + + it('leaves the cols/rows floor on the fit itself', () => { + expect(canMeasurePaneForFit(makeSizedPane({ width: 50, height: 600 }, 5))).toBe(false) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-metric-options-deferral.ts b/src/renderer/src/lib/pane-manager/pane-metric-options-deferral.ts new file mode 100644 index 000000000..fc4b32312 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-metric-options-deferral.ts @@ -0,0 +1,113 @@ +import type { ManagedPane } from './pane-manager-types' +import { recordTerminalWebglDiagnostic } from '../../../../shared/terminal-webgl-diagnostics' + +/** The xterm options whose writes make the renderer clear, resize and full-refresh. */ +export type PaneMetricOptions = { + fontSize?: number + fontFamily?: string + fontWeight?: string | number + fontWeightBold?: string | number + lineHeight?: number +} + +type PaneTerminal = ManagedPane['terminal'] + +// Why keyed on the terminal, not the pane: getPanes() hands out a fresh +// toPublicPane() wrapper per call, so a pane-keyed entry could never be found +// again. `terminal` is carried by reference and dies with the pane. +const deferredMetricOptions = new WeakMap() + +/** + * Why deferred: writing one of these makes xterm clear the renderer, re-resize + * to the current grid and full-refresh. On a pane with no usable box that + * repaint is wasted — it lands in the DOM-renderer fallback while WebGL is + * suspended — and the cols/rows re-fit that has to follow the write cannot run. + * So the values park here and land once the pane can actually be fitted. + */ +export function applyOrDeferPaneMetricOptions( + pane: ManagedPane, + options: PaneMetricOptions, + measurable: boolean +): 'applied' | 'deferred' { + if (!measurable) { + // Latest wins: a newer settings change while hidden supersedes the pending one. + deferredMetricOptions.set(pane.terminal, options) + return 'deferred' + } + deferredMetricOptions.delete(pane.terminal) + writePaneMetricOptions(pane, options) + return 'applied' +} + +/** Applies a pending deferral. Callers must ensure the pane is measurable. */ +export function flushDeferredPaneMetricOptions(pane: ManagedPane): boolean { + const pending = deferredMetricOptions.get(pane.terminal) + if (!pending) { + return false + } + deferredMetricOptions.delete(pane.terminal) + writePaneMetricOptions(pane, pending) + recordTerminalWebglDiagnostic('metric-options-deferred-flush', { paneId: pane.id }) + return true +} + +export function hasDeferredPaneMetricOptions(pane: ManagedPane): boolean { + return deferredMetricOptions.has(pane.terminal) +} + +/** + * True when every value is already live and nothing is parked, so the caller can + * skip the apply. Why it matters: any settings write re-runs the appearance pass + * over every mounted pane, and arming a no-op deferral would make the next + * reveal refit for nothing. + */ +export function paneMetricOptionsAlreadySettled( + pane: ManagedPane, + options: PaneMetricOptions +): boolean { + if (deferredMetricOptions.has(pane.terminal)) { + return false + } + const target = pane.terminal.options + return ( + (options.fontSize === undefined || target.fontSize === options.fontSize) && + (options.fontFamily === undefined || target.fontFamily === options.fontFamily) && + (options.fontWeight === undefined || target.fontWeight === options.fontWeight) && + (options.fontWeightBold === undefined || target.fontWeightBold === options.fontWeightBold) && + (options.lineHeight === undefined || target.lineHeight === options.lineHeight) + ) +} + +/** + * Folds a directly-applied metric write into any pending deferral so a later + * flush cannot clobber it with the parked value. No-op when nothing is parked. + */ +export function overridePendingPaneMetricOptions( + pane: ManagedPane, + options: PaneMetricOptions +): void { + const pending = deferredMetricOptions.get(pane.terminal) + if (!pending) { + return + } + deferredMetricOptions.set(pane.terminal, { ...pending, ...options }) +} + +function writePaneMetricOptions(pane: ManagedPane, options: PaneMetricOptions): void { + const target = pane.terminal.options + if (options.fontSize !== undefined) { + target.fontSize = options.fontSize + } + if (options.fontFamily !== undefined) { + target.fontFamily = options.fontFamily + } + if (options.fontWeight !== undefined) { + target.fontWeight = options.fontWeight as typeof target.fontWeight + } + if (options.fontWeightBold !== undefined) { + target.fontWeightBold = options.fontWeightBold as typeof target.fontWeightBold + } + if (options.lineHeight !== undefined) { + target.lineHeight = options.lineHeight + } +} 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 caf10deb6..c3ac10f44 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 @@ -9,14 +9,16 @@ const mocks = vi.hoisted(() => ({ readFitClientSize: vi.fn<(pane: ManagedPane) => { width: number; height: number } | null>(), requestStablePaneFit: vi.fn(), clearPaneFitContinuationRetry: vi.fn(), - resumePendingFitScrollRestoreAfterFit: vi.fn() + resumePendingFitScrollRestoreAfterFit: vi.fn(), + flushDeferredPaneMetricOptionsIfMeasurable: vi.fn(() => false) })) vi.mock('./pane-fit', () => ({ safeFit: mocks.safeFit, canMeasurePaneForFit: mocks.canMeasurePaneForFit, flushPendingSafeFitContinuations: mocks.flushPendingSafeFitContinuations, - readFitClientSize: mocks.readFitClientSize + readFitClientSize: mocks.readFitClientSize, + flushDeferredPaneMetricOptionsIfMeasurable: mocks.flushDeferredPaneMetricOptionsIfMeasurable })) vi.mock('./pane-fit-resize-observer', () => ({ requestStablePaneFit: mocks.requestStablePaneFit @@ -50,6 +52,44 @@ describe('fitRevealedPane routing', () => { beforeEach(() => { vi.clearAllMocks() mocks.canMeasurePaneForFit.mockReturnValue(true) + mocks.flushDeferredPaneMetricOptionsIfMeasurable.mockReturnValue(false) + }) + + it('repairs on a steady grid when a deferred metric flush lands on a pane the size checks would skip', () => { + // Without the flush branch the checks below both say "nothing to do" and the + // parked font change never reaches the grid. It must route through the stable + // path, not a raw fit: pixels are unchanged and the grid diverged, so a + // synchronous fit would reflow on the WebGL/DOM metric wobble and corrupt a + // diff-painting inline TUI. + mocks.flushDeferredPaneMetricOptionsIfMeasurable.mockReturnValue(true) + 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.flushDeferredPaneMetricOptionsIfMeasurable).toHaveBeenCalledWith(pane) + expect(mocks.requestStablePaneFit).toHaveBeenCalledWith(pane) + expect(mocks.safeFit).not.toHaveBeenCalled() + }) + + it('still flushes parked metric options when the pane also resized while hidden', () => { + mocks.flushDeferredPaneMetricOptionsIfMeasurable.mockReturnValue(true) + 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.flushDeferredPaneMetricOptionsIfMeasurable).toHaveBeenCalledWith(pane) + // A real resize still takes the synchronous path, and the flush already landed. + expect(mocks.safeFit).toHaveBeenCalledWith(pane) }) it('fits synchronously when the fit element resized while hidden', () => { 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 a397eee93..f01254337 100644 --- a/src/renderer/src/lib/pane-manager/pane-reveal-fit.ts +++ b/src/renderer/src/lib/pane-manager/pane-reveal-fit.ts @@ -1,6 +1,7 @@ import type { ManagedPane, ManagedPaneInternal } from './pane-manager-types' import { canMeasurePaneForFit, + flushDeferredPaneMetricOptionsIfMeasurable, flushPendingSafeFitContinuations, readFitClientSize, safeFit @@ -61,11 +62,17 @@ function releaseMeasurableFitContinuations(pane: ManagedPane): void { // mismatch refits but a transient metric wobble does not reflow; // - grid already correct → leave it alone. 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) if (paneFitClientSizeChanged(pane)) { safeFit(pane) return } - if (!proposedGridMatchesTerminal(pane)) { + // Why the stable path for a flush: it leaves pixels unchanged but the grid + // diverged — the same shape as a snapshot resize, and a raw fit here would + // reflow on the transient WebGL/DOM metric wobble this function exists to avoid. + if (flushed || !proposedGridMatchesTerminal(pane)) { requestStablePaneFit(pane) return } diff --git a/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts b/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts index 5cc07ef0e..151a4b02b 100644 --- a/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts +++ b/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts @@ -65,8 +65,9 @@ async function installUnreadableOrcaYamlFault(electronApp: ElectronApplication): /** orca.yaml becomes readable again — every later check runs the production handler. */ async function healOrcaYamlRead(electronApp: ElectronApplication): Promise { await electronApp.evaluate(() => { - ;(globalThis as typeof globalThis & { __orcaE2eOrcaYamlUnreadable?: boolean }) - .__orcaE2eOrcaYamlUnreadable = false + ;( + globalThis as typeof globalThis & { __orcaE2eOrcaYamlUnreadable?: boolean } + ).__orcaE2eOrcaYamlUnreadable = false }) }