From eca463252f36f22243176073b4bc2c19503fda26 Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 19 Jun 2026 04:27:02 +0200 Subject: [PATCH] fix(tui): prevented full paints from interrupting resize fast path - Prevent interleaved ordinary renders from forcing a full redraw during active viewport resizing. - Remove redundant state tracking in favor of a direct check against forced-render flags, ensuring live updates like spinners remain within the optimized drag path. - Add regression test to verify that mid-drag renders do not trigger unintended transcript repaints or alternate screen exits. --- packages/tui/src/tui.ts | 31 +++++--- .../tui/test/resize-viewport-defer.test.ts | 77 ++++++++++++++++++- 2 files changed, 92 insertions(+), 16 deletions(-) diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index 137c2dcd0..23b22cde7 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -1005,11 +1005,6 @@ export class TUI extends Container { // fast path (`#renderResizeViewport`) instead of an authoritative full // paint, and no commit/window/diff state is advanced. #resizeViewportActive = false; - // Set only by the resize callback's cheap-paint request. A concurrent - // caller-forced render (tool finalization, reset, image reconciliation) must - // not be downgraded to the throwaway viewport path just because a resize - // settle window is active. - #resizeViewportPaintPending = false; // Quiet-window timer that ends the drag: its callback clears the flag and // drives the one authoritative full paint. Reset on every resize event so it // only fires once the drag stops. Cancelled on stop(). @@ -1792,7 +1787,6 @@ export class TUI extends Container { // Any non-component-scoped request makes the pending frame a full one. this.#pendingRenderComponentsOnly = false; if (force) { - this.#resizeViewportPaintPending = false; // Forced repaints landing inside the multiplexer resize debounce // (e.g. `#finishSixelProbe`, image-budget eviction, a programmatic // `requestRender(true)`) would paint into a still-reflowing pane @@ -2471,16 +2465,30 @@ export class TUI extends Container { // Strictly state-isolated: it never consumes #resizeEventPending nor // advances any commit/window/diff field, so the authoritative full paint // the settle timer queues reconciles as if these throwaway frames never - // ran. A visible overlay composites over the transcript and needs the - // whole window, so fall through to the normal forced paint when one is up - // (overlay resizes are not on the drag-cost hot path). + // ran. Two render sources reach here mid-drag and BOTH must stay on this + // path: + // - the resize callback's own cheap paint after each SIGWINCH; + // - an ordinary (non-forced) render from a live block that keeps + // animating through the drag — a spinner tick, a streamed token, a + // cursor blink — firing requestRender(false)/requestComponentRender. + // #resizeEventPending is still set (the fast path never consumed it), + // so without this branch the ordinary render falls through to the + // geometry-rebuild full paint below, which LEAVES the borrowed + // alternate screen to repaint the whole transcript on the normal + // screen — then the next SIGWINCH re-enters the alt screen and paints + // only the tail, so the block flashes in for one frame and vanishes. + // A forced render (tool finalization, reset, image reconciliation) must + // still preempt: it set #forceViewportRepaintOnNextRender via + // #prepareForcedRender and owns the next authoritative paint, so it falls + // through. A visible overlay composites over the transcript and needs the + // whole window, so it also falls through (overlay resizes are not on the + // drag-cost hot path). if ( - this.#resizeViewportPaintPending && this.#resizeViewportActive && + !this.#forceViewportRepaintOnNextRender && this.#hasEverRendered && this.#getTopmostVisibleOverlay() === undefined ) { - this.#resizeViewportPaintPending = false; this.#componentRenderTargets.clear(); this.#renderResizeViewport(width, height); return; @@ -3145,7 +3153,6 @@ export class TUI extends Container { #requestResizeViewportPaint(): void { if (this.#stopped) return; - this.#resizeViewportPaintPending = true; this.#renderRequested = false; this.#lastRenderAt = this.#renderScheduler.now(); this.#doRender(); diff --git a/packages/tui/test/resize-viewport-defer.test.ts b/packages/tui/test/resize-viewport-defer.test.ts index c3ef58b83..a0ceac04d 100644 --- a/packages/tui/test/resize-viewport-defer.test.ts +++ b/packages/tui/test/resize-viewport-defer.test.ts @@ -57,7 +57,7 @@ async function withEnvPatch(patch: Record, run: ( class DeferScheduler implements RenderScheduler { #time = 0; #immediates: (() => void)[] = []; - #renders = new Map void>(); + #renders = new Map void; delay: number }>(); #nextId = 0; now(): number { @@ -69,9 +69,9 @@ class DeferScheduler implements RenderScheduler { this.#immediates.push(callback); } - scheduleRender(callback: () => void, _delayMs: number): RenderTimer { + scheduleRender(callback: () => void, delayMs: number): RenderTimer { const id = this.#nextId++; - this.#renders.set(id, callback); + this.#renders.set(id, { run: callback, delay: delayMs }); return { cancel: () => { this.#renders.delete(id); @@ -94,6 +94,28 @@ class DeferScheduler implements RenderScheduler { await term.flush(); } + // Fire immediates plus any throttled render whose delay is below `maxDelayMs`, + // leaving longer timers (the 120ms resize settle) pending. Lets a test drive + // an interleaved ordinary render — a spinner tick / streamed token, scheduled + // at the ~33ms render throttle — mid-drag without ending the drag. + async flushOrdinaryRenders(term: VirtualTerminal, maxDelayMs = 100): Promise { + let rounds = 0; + for (;;) { + if (++rounds > 100) throw new Error("ordinary renders did not settle"); + const immediates = this.#immediates; + this.#immediates = []; + for (const callback of immediates) callback(); + if (this.#immediates.length > 0) continue; + const due = [...this.#renders.entries()].filter(([, entry]) => entry.delay < maxDelayMs); + if (due.length === 0) break; + for (const [id, entry] of due) { + this.#renders.delete(id); + entry.run(); + } + } + await term.flush(); + } + async flushAll(term: VirtualTerminal): Promise { let rounds = 0; while (this.#immediates.length > 0 || this.#renders.size > 0) { @@ -104,7 +126,7 @@ class DeferScheduler implements RenderScheduler { if (this.#immediates.length > 0) continue; const renders = [...this.#renders.values()]; this.#renders.clear(); - for (const callback of renders) callback(); + for (const entry of renders) entry.run(); } await term.flush(); } @@ -269,6 +291,53 @@ describe("non-multiplexer resize viewport fast path", () => { }); }); + it("keeps an interleaved live-block render on the viewport fast path instead of flashing a normal-screen full paint", async () => { + await withEnvPatch(NO_MULTIPLEXER_ENV, async () => { + const term = new VirtualTerminal(40, 10, 1000); + const { tui, scheduler } = makeTui(term); + try { + tui.start(); + await scheduler.flushImmediates(term); + + // One drag SIGWINCH enters the fast path and borrows the alt screen. + term.resize(60, 10); + await scheduler.flushImmediates(term); + expect(tui.resizeViewportActive).toBe(true); + + const baselineFull = tui.fullRedraws; + const baselinePaints = tui.resizeViewportPaints; + const writes = captureWrites(term); + + // A live block keeps animating mid-drag: a spinner tick / streamed + // token fires an ordinary (non-forced) render before the 120ms settle + // elapses. It must stay on the viewport fast path. Without the guard it + // falls through to the geometry-rebuild full paint, which leaves the + // borrowed alternate screen (ALT_SCREEN_EXIT) and erases native + // scrollback (ED3) to repaint the whole transcript on the normal screen + // for one frame — the flash — before the next SIGWINCH hides it again. + tui.requestRender(); + await scheduler.flushOrdinaryRenders(term); + + // Still mid-drag, still on the alternate screen: a viewport-only paint, + // no authoritative full redraw, no scrollback erase, no alt-screen exit. + expect(tui.resizeViewportActive).toBe(true); + expect(tui.resizeViewportPaints).toBeGreaterThan(baselinePaints); + expect(tui.fullRedraws).toBe(baselineFull); + expect(writes.join("")).not.toContain(ALT_SCREEN_EXIT); + expect(eraseScrollbackCount(writes)).toBe(0); + + // The settle still fires exactly one authoritative full paint once the + // drag goes quiet — the interleaved render did not consume or corrupt + // the deferred geometry rebuild. + await scheduler.flushAll(term); + expect(tui.resizeViewportActive).toBe(false); + expect(tui.fullRedraws).toBe(baselineFull + 1); + } finally { + tui.stop(); + } + }); + }); + it("does not leave a pending settle paint after stop()", async () => { await withEnvPatch(NO_MULTIPLEXER_ENV, async () => { const term = new VirtualTerminal(40, 10, 1000);