From 08e76da3a1687a1303733406b684a9a6ab523acb Mon Sep 17 00:00:00 2001
From: James Holderness <j4_james@hotmail.com>
Date: Wed, 30 Apr 2025 00:15:38 +0100
Subject: [PATCH] Fix two image erasure bugs (#18855)

This PR fixes two cases where image content wasn't correctly erased when
overwritten.

1. When legacy console APIs fill an area of the buffer using a starting
coordinate and a length, the affected area could potentially wrap over
multiple rows, but we were only erasing the overwritten image content on
the first affected row.

2. When copying an area of the buffer with text content over another
area that contained image content, the image in the target area would
sometimes not be erased, because we ignored the `_eraseCells` return
value which indicated that the image slice needed to be removed.

## References and Relevant Issues

The original code was from the Sixel implementation in PR #17421.

## Validation Steps Performed

I've manually verified that these two cases are now working as expected.

## PR Checklist
- [x] Closes #18568
---
 src/buffer/out/ImageSlice.cpp | 31 +++++++++++++++++++++----------
 src/buffer/out/ImageSlice.hpp |  2 +-
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/src/buffer/out/ImageSlice.cpp b/src/buffer/out/ImageSlice.cpp
index 65809e58a0..3500eaf0a5 100644
--- a/src/buffer/out/ImageSlice.cpp
+++ b/src/buffer/out/ImageSlice.cpp
@@ -186,18 +186,20 @@ bool ImageSlice::_copyCells(const ImageSlice& srcSlice, const til::CoordType src
     }
 
     // The used destination before and after the written area must be erased.
-    if (dstUsedBegin < dstWriteBegin)
+    // If this results in the entire range being erased, we return true to let
+    // the caller know that the slice should be deleted.
+    if (dstUsedBegin < dstWriteBegin && _eraseCells(dstUsedBegin, dstWriteBegin))
     {
-        _eraseCells(dstUsedBegin, dstWriteBegin);
+        return true;
     }
-    if (dstUsedEnd > dstWriteEnd)
+    if (dstUsedEnd > dstWriteEnd && _eraseCells(dstWriteEnd, dstUsedEnd))
     {
-        _eraseCells(dstWriteEnd, dstUsedEnd);
+        return true;
     }
 
-    // If the beginning column is now not less than the end, that means the
-    // content has been entirely erased, so we return true to let the caller
-    // know that the slice should be deleted.
+    // At this point, if the beginning column is not less than the end, that
+    // means this was an empty slice into which nothing was copied, so we can
+    // again return true to let the caller know it should be deleted.
     return _columnBegin >= _columnEnd;
 }
 
@@ -210,10 +212,19 @@ void ImageSlice::EraseBlock(TextBuffer& buffer, const til::rect rect)
     }
 }
 
-void ImageSlice::EraseCells(TextBuffer& buffer, const til::point at, const size_t distance)
+void ImageSlice::EraseCells(TextBuffer& buffer, const til::point at, const til::CoordType distance)
 {
-    auto& row = buffer.GetMutableRowByOffset(at.y);
-    EraseCells(row, at.x, gsl::narrow_cast<til::CoordType>(at.x + distance));
+    auto x = at.x;
+    auto y = at.y;
+    auto distanceRemaining = distance;
+    while (distanceRemaining > 0)
+    {
+        auto& row = buffer.GetMutableRowByOffset(y);
+        EraseCells(row, x, x + distanceRemaining);
+        distanceRemaining -= (static_cast<til::CoordType>(row.size()) - x);
+        x = 0;
+        y++;
+    }
 }
 
 void ImageSlice::EraseCells(ROW& row, const til::CoordType columnBegin, const til::CoordType columnEnd)
diff --git a/src/buffer/out/ImageSlice.hpp b/src/buffer/out/ImageSlice.hpp
index 87244abe78..14c0b314ae 100644
--- a/src/buffer/out/ImageSlice.hpp
+++ b/src/buffer/out/ImageSlice.hpp
@@ -41,7 +41,7 @@ public:
     static void CopyRow(const ROW& srcRow, ROW& dstRow);
     static void CopyCells(const ROW& srcRow, const til::CoordType srcColumn, ROW& dstRow, const til::CoordType dstColumnBegin, const til::CoordType dstColumnEnd);
     static void EraseBlock(TextBuffer& buffer, const til::rect rect);
-    static void EraseCells(TextBuffer& buffer, const til::point at, const size_t distance);
+    static void EraseCells(TextBuffer& buffer, const til::point at, const til::CoordType distance);
     static void EraseCells(ROW& row, const til::CoordType columnBegin, const til::CoordType columnEnd);
 
 private: