From 392bae6b67d92c82608e69906283bcebf5f385ae Mon Sep 17 00:00:00 2001 From: Vegard Stikbakke Date: Tue, 23 Jun 2026 10:36:47 +0200 Subject: [PATCH] fix(tui): avoid tall dialog redraw loops closes #5990 --- packages/tui/CHANGELOG.md | 4 ++ packages/tui/src/tui.ts | 58 ++++++++++++++++++++++++- packages/tui/test/tui-render.test.ts | 64 ++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 1 deletion(-) diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index a3cf70ef..9230cd04 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -6,6 +6,10 @@ - Added `Ctrl+J` as a default newline keybinding alongside `Shift+Enter`. +### Fixed + +- Fixed full redraw loops when offscreen lines change above a tall visible viewport, avoiding flicker for oversized dialogs ([#5990](https://github.com/earendil-works/pi/issues/5990)). + ## [0.79.10] - 2026-06-22 ## [0.79.9] - 2026-06-20 diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index a7054888..f6e81e01 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -313,6 +313,7 @@ export class TUI extends Container { private clearOnShrink = process.env.PI_CLEAR_ON_SHRINK === "1"; // Clear empty rows when content shrinks (default: off) private maxLinesRendered = 0; // Track terminal's working area (max lines ever rendered) private previousViewportTop = 0; // Track previous viewport top for resize-aware cursor moves + private skippedOffscreenChangeRange: { first: number; last: number } | undefined; private fullRedrawCount = 0; private stopped = false; private pendingOsc11BackgroundReplies = 0; @@ -718,6 +719,7 @@ export class TUI extends Container { this.hardwareCursorRow = 0; this.maxLinesRendered = 0; this.previousViewportTop = 0; + this.skippedOffscreenChangeRange = undefined; if (this.renderTimer) { clearTimeout(this.renderTimer); this.renderTimer = undefined; @@ -1172,6 +1174,30 @@ export class TUI extends Container { return this.deleteKittyImages(ids); } + private changedRangeContainsKittyImages(firstChanged: number, lastChanged: number, newLines: string[]): boolean { + for (let i = firstChanged; i <= lastChanged; i++) { + if (extractKittyImageIds(this.previousLines[i] ?? "").length > 0) return true; + if (extractKittyImageIds(newLines[i] ?? "").length > 0) return true; + } + return false; + } + + private markSkippedOffscreenChange(first: number, last: number): void { + if (!this.skippedOffscreenChangeRange) { + this.skippedOffscreenChangeRange = { first, last }; + return; + } + this.skippedOffscreenChangeRange = { + first: Math.min(this.skippedOffscreenChangeRange.first, first), + last: Math.max(this.skippedOffscreenChangeRange.last, last), + }; + } + + private skippedOffscreenChangesIntersect(first: number, last: number): boolean { + const range = this.skippedOffscreenChangeRange; + return range !== undefined && range.first <= last && range.last >= first; + } + /** Splice overlay content into a base line at a specific column. Single-pass optimized. */ private compositeLineAt( baseLine: string, @@ -1322,6 +1348,7 @@ export class TUI extends Container { this.previousKittyImageIds = this.collectKittyImageIds(newLines); this.previousWidth = width; this.previousHeight = height; + this.skippedOffscreenChangeRange = undefined; }; const debugRedraw = process.env.PI_DEBUG_REDRAW === "1"; @@ -1349,7 +1376,18 @@ export class TUI extends Container { // Height changes normally need a full re-render to keep the visible viewport aligned, // but Termux changes height when the software keyboard shows or hides. // In that environment, a full redraw causes the entire history to replay on every toggle. - if (heightChanged && !isTermuxSession()) { + // If the new Termux viewport would reveal rows skipped by the offscreen optimization, + // redraw once so stale scrollback does not become visible. + if (heightChanged && isTermuxSession()) { + const viewportBottom = prevViewportTop + height - 1; + if (this.skippedOffscreenChangesIntersect(prevViewportTop, viewportBottom)) { + logRedraw( + `terminal height changed revealing skipped offscreen changes (${this.previousHeight} -> ${height})`, + ); + fullRender(true); + return; + } + } else if (heightChanged) { logRedraw(`terminal height changed (${this.previousHeight} -> ${height})`); fullRender(true); return; @@ -1401,6 +1439,24 @@ export class TUI extends Container { return; } + // If all changed lines are above the visible viewport and the buffer length did not change, + // the on-screen content is already correct. Avoid clearing/replaying the viewport for + // offscreen animations such as tall dialogs with an updating header/status line. + if ( + lastChanged < prevViewportTop && + newLines.length === this.previousLines.length && + !this.changedRangeContainsKittyImages(firstChanged, lastChanged, newLines) + ) { + this.markSkippedOffscreenChange(firstChanged, lastChanged); + this.positionHardwareCursor(cursorPos, newLines.length); + this.previousLines = newLines; + this.previousKittyImageIds = this.collectKittyImageIds(newLines); + this.previousWidth = width; + this.previousHeight = height; + this.previousViewportTop = prevViewportTop; + return; + } + // All changes are in deleted lines (nothing to render, just clear) if (firstChanged >= newLines.length) { if (this.previousLines.length > newLines.length) { diff --git a/packages/tui/test/tui-render.test.ts b/packages/tui/test/tui-render.test.ts index ab038ac7..f20dffd6 100644 --- a/packages/tui/test/tui-render.test.ts +++ b/packages/tui/test/tui-render.test.ts @@ -378,6 +378,44 @@ describe("TUI resize handling", () => { }); }); + it("full re-renders in Termux when height changes reveal skipped offscreen changes", async () => { + await withEnv({ TERMUX_VERSION: "1" }, async () => { + const terminal = new VirtualTerminal(20, 5); + const tui = new TUI(terminal); + const component = new TestComponent(); + tui.addChild(component); + + component.lines = Array.from({ length: 11 }, (_, i) => `Line ${i}`); + tui.start(); + await terminal.waitForRender(); + + component.lines = Array.from({ length: 11 }, (_, i) => (i === 5 ? "Changed 5" : `Line ${i}`)); + tui.requestRender(); + await terminal.waitForRender(); + + const redrawsAfterSkippedChange = tui.fullRedraws; + assert.deepStrictEqual(terminal.getViewport(), ["Line 6", "Line 7", "Line 8", "Line 9", "Line 10"]); + + terminal.resize(20, 6); + await terminal.waitForRender(); + + assert.ok( + tui.fullRedraws > redrawsAfterSkippedChange, + "Height change should redraw before skipped offscreen content becomes visible", + ); + assert.deepStrictEqual(terminal.getViewport(), [ + "Changed 5", + "Line 6", + "Line 7", + "Line 8", + "Line 9", + "Line 10", + ]); + + tui.stop(); + }); + }); + it("triggers full re-render when terminal width changes", async () => { const terminal = new VirtualTerminal(40, 10); const tui = new TUI(terminal); @@ -657,6 +695,32 @@ describe("TUI differential rendering", () => { tui.stop(); }); + it("does not full redraw for offscreen changes above the viewport", async () => { + const terminal = new VirtualTerminal(20, 5); + const tui = new TUI(terminal); + const component = new TestComponent(); + tui.addChild(component); + + const stableVisibleLines = Array.from({ length: 8 }, (_, i) => `Dialog ${i}`); + component.lines = ["Ticker 0", "Intro", "", ...stableVisibleLines]; + tui.start(); + await terminal.waitForRender(); + + const initialRedraws = tui.fullRedraws; + const initialViewport = terminal.getViewport(); + + for (let i = 1; i <= 3; i++) { + component.lines = [`Ticker ${i}`, "Intro", "", ...stableVisibleLines]; + tui.requestRender(); + await terminal.waitForRender(); + } + + assert.strictEqual(tui.fullRedraws, initialRedraws, "Offscreen changes should not force full redraws"); + assert.deepStrictEqual(terminal.getViewport(), initialViewport, "Visible viewport should remain stable"); + + tui.stop(); + }); + it("full re-renders when deleted lines move the viewport upward", async () => { const terminal = new VirtualTerminal(20, 5); const tui = new TUI(terminal);