From 475d95eba2c47e37027488381bf3fd9ac232d2fb Mon Sep 17 00:00:00 2001 From: Sergey Fedorov Date: Tue, 21 Jul 2026 21:28:32 +0000 Subject: [PATCH 09/13] fix(vendor): scroll to fit a kitty image taller than the space below the cursor renderKittyImage() hardcoded autoScroll=false in its call to the shared renderImage(), unlike sixel and GIP which already pass a real scroll flag (autoScrollAtBottomMargin / GipRenderParams::autoScroll). renderImage() only ever draws min(gridSize.lines, linesAvailable) rows, where linesAvailable is measured from the CURSOR position, not the whole page -- with autoScroll off, the remainder-loop that scrolls to place the rest never runs. Placing a kitty image right after a shell prompt near the bottom of the screen is the ordinary case, so a multi-row image routinely had its lower rows silently and permanently dropped, looking like it was "cut to 1 line". This is a general engine bug, not specific to this port: the shared renderImage() remainder loop already handles both the sixel band-cursor convention and the GIP (and now kitty) plain "cursor goes below the last rendered row" convention correctly, keyed only on whether imageSize is nonzero -- kitty passes ImageSize{} like GIP, so it takes the same already-exercised path. updateCursor was already wired independently from command.doNotMoveCursor, so this is a single-flag fix. Regression test added: cursor on the last row of a 4-line page, image two rows tall, both rows must end up visible via scrolling. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01LzTmVgP2ruMz987VJ78k77 (cherry picked from commit fdf868a191ba1f8f3250ebe9ed49e43071b61542) --- src/vtbackend/KittyGraphics_test.cpp | 35 ++++++++++++++++++++++++++++ src/vtbackend/Screen.cpp | 9 ++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/vtbackend/KittyGraphics_test.cpp b/src/vtbackend/KittyGraphics_test.cpp index cc0c5ff5..95ac6738 100644 --- a/src/vtbackend/KittyGraphics_test.cpp +++ b/src/vtbackend/KittyGraphics_test.cpp @@ -450,6 +450,41 @@ TEST_CASE("KittyGraphics.an_oversized_but_positive_cell_count_is_clamped_to_the_ CHECK(screen.at(LineOffset(3), ColumnOffset(7)).imageFragment()); } +TEST_CASE("KittyGraphics.an_image_taller_than_the_space_below_the_cursor_scrolls_to_fit", "[kitty]") +{ + // Regression test: renderKittyImage() used to pass autoScroll=false to renderImage(), unlike + // sixel/GIP. renderImage() only ever draws min(gridSize.lines, linesAvailable) rows, where + // linesAvailable is the space BELOW THE CURSOR, not the whole page -- so with autoScroll off, + // an image placed near the bottom of the screen (the common case, right after a shell prompt) + // had its lower rows silently dropped instead of scrolled into view. A 2-row image placed on + // the last row of a 4-line page leaves only 1 row available; it must still end up fully + // visible by scrolling, exactly as sixel/GIP already do in the same situation. + auto mock = MockTerm { PageSize { LineCount(4), ColumnCount(8) }, LineCount(20) }; + auto const& screen = mock.terminal.primaryScreen(); + mock.terminal.setCellPixelSize(ImageSize { Width(2), Height(2) }); + + mock.writeToScreen("\033[4;1H"sv); // CUP to the last row (1-based). + + // A 2x4 RGBA image: two cells tall at this cell size, one wider than the single row left below + // the cursor. + auto pixels = std::string {}; + for (int i = 0; i < 8; ++i) + pixels += "\xFF\x00\x00\xFF"sv; // opaque red + auto const encoded = crispy::base64::encode(pixels); + + mock.writeToScreen(std::format("\033_Ga=T,f=32,s=2,v=4,i=1;{}\033\\", encoded)); + + // Both rows of the image must be visible somewhere on the (now-scrolled) page -- none of it may + // have been silently dropped. + auto rowsWithFragment = 0; + for (auto const line: std::views::iota(-20, 4)) + { + if (screen.at(LineOffset(line), ColumnOffset(0)).imageFragment()) + ++rowsWithFragment; + } + CHECK(rowsWithFragment == 2); +} + TEST_CASE("KittyGraphics.an_APC_body_past_the_cap_is_dropped_not_dispatched_truncated", "[kitty]") { // An APC body is bounded, as OSC is, because it is attacker-controlled. What the bound must NOT diff --git a/src/vtbackend/Screen.cpp b/src/vtbackend/Screen.cpp index 57eb557a..823a515a 100644 --- a/src/vtbackend/Screen.cpp +++ b/src/vtbackend/Screen.cpp @@ -5627,7 +5627,14 @@ void Screen::renderKittyImage(kitty_graphics::Command const& command, ImageSize {}, ImageAlignment::TopStart, ImageResize::ResizeToFit, - /*autoScroll*/ false, + // Without this, an image taller than the space remaining below the cursor (the + // common case: placed right after a shell prompt near the bottom of the screen) + // had its lower rows silently dropped -- renderImage() only ever draws + // min(gridSize.lines, linesAvailable) rows and, with autoScroll off, never runs + // the remainder loop that scrolls to place the rest. Sixel/GIP already pass true + // here (see autoScrollAtBottomMargin above and GipRenderParams::autoScroll) for + // the same reason; a real kitty terminal also scrolls to fit a placed image. + /*autoScroll*/ true, /*updateCursor*/ !command.doNotMoveCursor, command.zIndex < 0 ? ImageLayer::Below : ImageLayer::Above); }