If status line is active during sixel image render, take into account

the height of status line as well to draw the whole image.
contour-terminal · Feb 25, 2023 · 8db7840 · 8db7840
1 parent a94b47a
commit 8db7840
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 20 deletions.
diff --git a/metainfo.xml b/metainfo.xml
@@ -113,6 +113,7 @@
           <li>Fixes mouse selection to only be initiated if actually meant to, i.e. in alt screen mode only if bypass-modifier was pressed (#1017).</li>
           <li>Fixes mouse selection within scrolloff setting to not cause the viewport to jump anymore (#1019).</li>
           <li>Fixes vi-like motion `t{space}` (#1037).</li>
+          <li>Fixes rendering sixel image when status line is active (#1050).</li>
           <li>Adds normal mode motion `[[`, `]]`, `[]`, `][` mimmicking exactly what vim does.</li>
           <li>Adds normal mode motion `[m` and `]m` to jump line marks up/down.</li>
           <li>Adds normal mode motion `mm` to toggle the line mark at the current active cursor position.</li>

diff --git a/src/vtbackend/Screen.cpp b/src/vtbackend/Screen.cpp
@@ -1852,7 +1852,8 @@ void Screen<Cell>::renderImage(shared_ptr<Image const> image,
     // TODO: make use of imageOffset
     (void) imageOffset;
 
-    auto const linesAvailable = _settings.pageSize.lines - topLeft.line.as<LineCount>();
+    auto const linesAvailable =
+        _settings.pageSize.lines - topLeft.line.as<LineCount>() - _terminal.statusLineHeight();
     auto const linesToBeRendered = min(gridSize.lines, linesAvailable);
     auto const columnsAvailable = *_settings.pageSize.columns - *topLeft.column;
     auto const columnsToBeRendered = ColumnCount(min(columnsAvailable, *gridSize.columns));
@@ -1901,7 +1902,8 @@ void Screen<Cell>::renderImage(shared_ptr<Image const> image,
                 auto const offset =
                     CellLocation { boxed_cast<LineOffset>(linesToBeRendered) + lineOffset, columnOffset };
                 Cell& cell =
-                    at(boxed_cast<LineOffset>(_settings.pageSize.lines) - 1, topLeft.column + columnOffset);
+                    at(boxed_cast<LineOffset>(_settings.pageSize.lines - _terminal.statusLineHeight()) - 1,
+                       topLeft.column + columnOffset);
                 cell.setImageFragment(rasterizedImage, offset);
                 cell.setHyperlink(_cursor.hyperlink);
             };

diff --git a/src/vtbackend/Screen_test.cpp b/src/vtbackend/Screen_test.cpp
@@ -67,6 +67,21 @@ namespace // {{{
 //     }
 // };
 
+// Chessboard image with each square of size 10x10 pixels
+std::string const chessBoard =
+    R"=(P0;0;0q"1;1;100;100#0;2;0;0;0#1;2;100;100;100#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~-#0!10N!10o!10N!10o!10N!10o!10N!10o!10N!10o$#1!10o!10N!10o!10N!10o!10N!10o!10N!10o!10N-!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~-!10{!10B!10{!10B!10{!10B!10{!10B!10{!10B$#1!10B!10{!10B!10{!10B!10{!10B!10{!10B!10{-#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~-!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~-!10o!10N!10o!10N!10o!10N!10o!10N!10o!10N$#1!10N!10o!10N!10o!10N!10o!10N!10o!10N!10o-#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~-#0!10B!10{!10B!10{!10B!10{!10B!10{!10B!10{$#1!10{!10B!10{!10B!10{!10B!10{!10B!10{!10B-!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~-!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~-#0!10N!10o!10N!10o!10N!10o!10N!10o!10N!10o$#1!10o!10N!10o!10N!10o!10N!10o!10N!10o!10N-!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~-!10{!10B!10{!10B!10{!10B!10{!10B!10{!10B$#1!10B!10{!10B!10{!10B!10{!10B!10{!10B!10{-#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~-!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~#1!10~#0!10~-#1!10N#0!10N#1!10N#0!10N#1!10N#0!10N#1!10N#0!10N#1!10N#0!10N-\)=";
+
+Image::Data const black10x10 = [] {
+    Image::Data ret(100 * 4, 0);
+    for (size_t i = 3; i < ret.size(); i += 4)
+    {
+        ret[i] = 255;
+    }
+    return ret;
+}();
+
+Image::Data const white10x10(100 * 4, 255);
+
 struct TextRenderBuilder
 {
     std::string text;
@@ -3493,26 +3508,29 @@ TEST_CASE("Screen.tcap.string", "[screen, tcap]")
 
 TEST_CASE("Sixel.simple", "[screen]")
 {
-    auto const pageSize = PageSize { LineCount(10), ColumnCount(10) };
-    auto mock = MockTerm { pageSize, LineCount(10) };
+    auto const pageSize = PageSize { LineCount(11), ColumnCount(11) };
+    auto mock = MockTerm { pageSize, LineCount(11) };
     mock.terminal.setCellPixelSize(ImageSize { Width(10), Height(10) });
 
-    auto const sixelData = crispy::readFileAsString("./test/images/squirrel-50.sixel");
-
-    mock.writeToScreen(sixelData);
+    mock.writeToScreen(chessBoard);
 
     CHECK(mock.terminal.primaryScreen().cursor().position.column.value == ColumnOffset(0).value);
-    CHECK(mock.terminal.primaryScreen().cursor().position.line.value == LineOffset(5).value);
+    CHECK(mock.terminal.primaryScreen().cursor().position.line.value == LineOffset(10).value);
 
     for (auto line = LineOffset(0); line < boxed_cast<LineOffset>(pageSize.lines); ++line)
     {
         for (auto column = ColumnOffset(0); column < boxed_cast<ColumnOffset>(pageSize.columns); ++column)
         {
             auto const& cell = mock.terminal.primaryScreen().at(line, column);
-            if (line <= LineOffset(4) && column <= ColumnOffset(7))
+            if (line <= LineOffset(9) && column <= ColumnOffset(9))
             {
                 auto fragment = cell.imageFragment();
                 REQUIRE(fragment);
+                if ((column.value + line.value) % 2)
+                    REQUIRE_THAT(white10x10, Catch::Matchers::Equals(fragment->data()));
+                else
+                    REQUIRE_THAT(black10x10, Catch::Matchers::Equals(fragment->data()));
+
                 CHECK(fragment->offset().line == line);
                 CHECK(fragment->offset().column == column);
                 CHECK(!fragment->data().empty());
@@ -3523,24 +3541,20 @@ TEST_CASE("Sixel.simple", "[screen]")
             }
         }
     }
-
-    // Um, we could actually test more precise here by validating the grid cell contents.
 }
 
 TEST_CASE("Sixel.AutoScroll-1", "[screen]")
 {
-    // Create a 10x3x5 grid and render a 7x5 image causing one a line-scroll by one.
-    auto const pageSize = PageSize { LineCount(4), ColumnCount(10) };
-    auto mock = MockTerm { pageSize, LineCount(5) };
+    // Create a 11x9x10 grid and render a 10x10 image causing a line-scroll by one.
+    auto const pageSize = PageSize { LineCount(9), ColumnCount(10) };
+    auto mock = MockTerm { pageSize, LineCount(11) };
     mock.terminal.setCellPixelSize(ImageSize { Width(10), Height(10) });
     mock.terminal.setMode(DECMode::NoSixelScrolling, false);
 
-    auto const sixelData = crispy::readFileAsString("./test/images/squirrel-50.sixel");
-
-    mock.writeToScreen(sixelData);
+    mock.writeToScreen(chessBoard);
 
     CHECK(mock.terminal.primaryScreen().cursor().position.column == ColumnOffset(0));
-    CHECK(mock.terminal.primaryScreen().cursor().position.line == LineOffset(3));
+    CHECK(mock.terminal.primaryScreen().cursor().position.line == LineOffset(8));
 
     for (auto line = LineOffset(-1); line < boxed_cast<LineOffset>(pageSize.lines); ++line)
     {
@@ -3549,10 +3563,14 @@ TEST_CASE("Sixel.AutoScroll-1", "[screen]")
         {
             INFO(fmt::format("column {}", column));
             auto const& cell = mock.terminal.primaryScreen().at(line, column);
-            if (line <= LineOffset(4) && column <= ColumnOffset(7))
+            if (line <= LineOffset(9) && column <= ColumnOffset(9))
             {
                 auto fragment = cell.imageFragment();
                 REQUIRE(fragment);
+                if ((column.value + line.value) % 2)
+                    REQUIRE_THAT(black10x10, Catch::Matchers::Equals(fragment->data()));
+                else
+                    REQUIRE_THAT(white10x10, Catch::Matchers::Equals(fragment->data()));
                 CHECK(fragment->offset().line == line + 1);
                 CHECK(fragment->offset().column == column);
                 CHECK(!fragment->data().empty());
@@ -3563,8 +3581,45 @@ TEST_CASE("Sixel.AutoScroll-1", "[screen]")
             }
         }
     }
+}
+
+TEST_CASE("Sixel.status_line", "[screen]")
+{
+    // Test for #1050
+    auto const pageSize = PageSize { LineCount(5), ColumnCount(11) };
+    auto mock = MockTerm { pageSize, LineCount(12) };
+    mock.terminal.setCellPixelSize(ImageSize { Width(10), Height(10) });
+    mock.terminal.setStatusDisplay(StatusDisplayType::Indicator);
+
+    mock.writeToScreen(chessBoard);
+
+    CHECK(mock.terminal.primaryScreen().cursor().position.column.value == ColumnOffset(0).value);
+    CHECK(mock.terminal.primaryScreen().cursor().position.line.value == LineOffset(3).value);
+
+    for (auto line = LineOffset(-6); line < boxed_cast<LineOffset>(pageSize.lines) - 1; ++line)
+    {
+        for (auto column = ColumnOffset(0); column < boxed_cast<ColumnOffset>(pageSize.columns); ++column)
+        {
+            auto const& cell = mock.terminal.primaryScreen().at(line, column);
+            if (line <= LineOffset(9) && column <= ColumnOffset(9))
+            {
+                auto fragment = cell.imageFragment();
+                REQUIRE(fragment);
+                if ((column.value + line.value) % 2)
+                    REQUIRE_THAT(white10x10, Catch::Matchers::Equals(fragment->data()));
+                else
+                    REQUIRE_THAT(black10x10, Catch::Matchers::Equals(fragment->data()));
 
-    // Um, we could actually test more precise here by validating the grid cell contents.
+                CHECK(fragment->offset().line == line + 6);
+                CHECK(fragment->offset().column == column);
+                CHECK(!fragment->data().empty());
+            }
+            else
+            {
+                CHECK(cell.empty());
+            }
+        }
+    }
 }
 
 TEST_CASE("DECSTR", "[screen]")

diff --git a/src/vtbackend/Terminal.h b/src/vtbackend/Terminal.h
@@ -226,6 +226,19 @@ class Terminal
 
     [[nodiscard]] PageSize totalPageSize() const noexcept { return _settings.pageSize; }
 
+    // Returns status line height
+    [[nodiscard]] LineCount statusLineHeight() const noexcept
+    {
+        using enum StatusDisplayType;
+        switch (_state.statusDisplayType)
+        {
+            case None: return LineCount(0);
+            case Indicator: return _indicatorStatusScreen.pageSize().lines;
+            case HostWritable: return _hostWritableStatusLineScreen.pageSize().lines;
+        }
+        crispy::unreachable();
+    }
+
     /// Resizes the terminal screen to the given amount of grid cells with their pixel dimensions.
     /// Important! In case a status line is currently visible, the status line count is being
     /// accumulated into the screen size, too.