From 1c8cb1781785f3baf66f3ba1edf8eb239fe35ba2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Hou=C5=A1ka?= Date: Fri, 23 Oct 2026 21:51:17 +0200 Subject: [PATCH] fix(tui): bound image line search by tallest registered image Placeholder rows below a Kitty image line can contain box padding, borders, or side-by-side text. Stopping the backward search at the first non-blank row dropped those images once their first row scrolled above the viewport. Search back at most the tallest registered image height instead; the existing hiddenRows check keeps results correct. --- packages/tui/src/layout.ts | 28 +++++++++++++----------- packages/tui/src/terminal-image.ts | 9 ++++++++ packages/tui/test/tui-alt-screen.test.ts | 22 +++++++++++++++++++ 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/packages/tui/src/layout.ts b/packages/tui/src/layout.ts index 5e089078c..a64c262c1 100644 --- a/packages/tui/src/layout.ts +++ b/packages/tui/src/layout.ts @@ -1,7 +1,7 @@ import type { ScrollView } from "./components/scroll-view.ts"; import { allocateStackSizes, visibleStackEntries } from "./components/stack.ts"; import { getLayoutNode } from "./layout-node.ts"; -import { cropKittyImageLine, getKittyImageMetadata, isImageLine } from "./terminal-image.ts"; +import { cropKittyImageLine, getKittyImageMetadata, getMaxKittyImageRows, isImageLine } from "./terminal-image.ts"; import { type Component, CURSOR_MARKER, compositeTuiLine } from "./tui.ts"; import { extractAnsiCode, @@ -357,21 +357,23 @@ function paintBox(box: LayoutBox, screen: string[], totalWidth: number): void { for (const child of box.children) paintBox(child, screen, totalWidth); if (box.scrollView && box.scrollContentLines && box.scrollView.scrollTop > 0 && box.rect.height > 0) { - for (let imageRow = box.scrollView.scrollTop - 1; imageRow >= 0; imageRow--) { + // Find the nearest image line above the viewport. Placeholder rows below an + // image line may contain padding, borders, or side-by-side text, so the search + // is bounded by the tallest registered image instead of by row content. + const scrollTop = box.scrollView.scrollTop; + const minRow = Math.max(0, scrollTop - getMaxKittyImageRows()); + for (let imageRow = scrollTop - 1; imageRow >= minRow; imageRow--) { const imageLine = box.scrollContentLines[imageRow] ?? ""; + if (!isImageLine(imageLine)) continue; const metadata = getKittyImageMetadata(imageLine); - if (metadata) { - const hiddenRows = box.scrollView.scrollTop - imageRow; - if (hiddenRows < metadata.rows) { - const visibleRows = Math.min(box.rect.height, metadata.rows - hiddenRows); - const cropped = cropKittyImageLine(imageLine, hiddenRows, visibleRows); - if (box.rect.x === 0 && box.rect.width >= totalWidth) screen[box.rect.y] = cropped; - } - break; + if (!metadata) continue; + const hiddenRows = scrollTop - imageRow; + if (hiddenRows < metadata.rows) { + const visibleRows = Math.min(box.rect.height, metadata.rows - hiddenRows); + const cropped = cropKittyImageLine(imageLine, hiddenRows, visibleRows); + if (box.rect.x === 0 && box.rect.width >= totalWidth) screen[box.rect.y] = cropped; } - // Image placeholder rows inside padded or colored boxes contain only - // spaces and SGR codes; keep searching past them for the image line. - if (imageLine.replace(/\x1b\[[0-9;:]*m/g, "").trim() !== "") break; + break; } } diff --git a/packages/tui/src/terminal-image.ts b/packages/tui/src/terminal-image.ts index cf1ca29ae..201a40603 100644 --- a/packages/tui/src/terminal-image.ts +++ b/packages/tui/src/terminal-image.ts @@ -349,9 +349,13 @@ export interface KittyImagePlacement { const kittyImageMetadata = new Map(); let kittyTransmissionGeneration = 0; +// Tallest registered image. Bounds backward searches for image lines above a +// viewport: no image line further up than this can reach into it. +let maxKittyImageRows = 0; export function registerKittyImageMetadata(metadata: KittyImageMetadata): void { kittyTransmissionGeneration += 1; + maxKittyImageRows = Math.max(maxKittyImageRows, metadata.rows); kittyImageMetadata.delete(metadata.imageId); kittyImageMetadata.set(metadata.imageId, { ...metadata, transmissionGeneration: kittyTransmissionGeneration }); if (kittyImageMetadata.size > 1000) { @@ -360,6 +364,11 @@ export function registerKittyImageMetadata(metadata: KittyImageMetadata): void { } } +/** Return the row height of the tallest Kitty image registered so far. */ +export function getMaxKittyImageRows(): number { + return maxKittyImageRows; +} + function getRegisteredKittyImageMetadataFromControls(controls: string): RegisteredKittyImageMetadata | undefined { const imageId = /(?:^|,)i=(\d+)(?:,|$)/.exec(controls)?.[1]; return imageId === undefined ? undefined : kittyImageMetadata.get(Number.parseInt(imageId, 10)); diff --git a/packages/tui/test/tui-alt-screen.test.ts b/packages/tui/test/tui-alt-screen.test.ts index 412d662d0..41402d5c7 100644 --- a/packages/tui/test/tui-alt-screen.test.ts +++ b/packages/tui/test/tui-alt-screen.test.ts @@ -1099,6 +1099,28 @@ describe("TuiAltScreen", () => { tui.stop(); }); + it("crops a Kitty image whose placeholder rows contain text when its first line is above the viewport", async () => { + // Regression test for #11271: borders or side-by-side text on placeholder rows stopped the search. + const terminal = new RecordingTerminal(20, 3); + const tui = new TuiAltScreen(terminal); + const imageId = 11272; + const imageLine = encodeKitty("AAAA", { columns: 2, rows: 3, imageId, moveCursor: false }); + registerKittyImageMetadata({ imageId, columns: 2, rows: 3, widthPx: 100, heightPx: 100 }); + tui.addChild({ render: () => ["before", imageLine, " | side", " | side"], invalidate: () => {} }); + tui.addChild({ render: () => ["after", "end"], invalidate: () => {} }); + tui.start(); + await terminal.waitForRender(); + + assert.strictEqual(tui.viewportTop, 3); + assert.ok( + terminal.events.some( + (event) => event.type === "write" && event.data.includes("i=11272") && event.data.includes("y=66,h=34,r=1"), + ), + ); + + tui.stop(); + }); + it("redraws WezTerm Kitty images after writes to covered rows", async () => { // Regression test for #10319: a scrollbar update below an unchanged image anchor erased its cells. const weztermPane = process.env.WEZTERM_PANE;