Skip to content

Commit 08e76da

Browse files
authored
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
1 parent 0568173 commit 08e76da

File tree

2 files changed

+22
-11
lines changed

2 files changed

+22
-11
lines changed

src/buffer/out/ImageSlice.cpp

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -186,18 +186,20 @@ bool ImageSlice::_copyCells(const ImageSlice& srcSlice, const til::CoordType src
186186
}
187187

188188
// The used destination before and after the written area must be erased.
189-
if (dstUsedBegin < dstWriteBegin)
189+
// If this results in the entire range being erased, we return true to let
190+
// the caller know that the slice should be deleted.
191+
if (dstUsedBegin < dstWriteBegin && _eraseCells(dstUsedBegin, dstWriteBegin))
190192
{
191-
_eraseCells(dstUsedBegin, dstWriteBegin);
193+
return true;
192194
}
193-
if (dstUsedEnd > dstWriteEnd)
195+
if (dstUsedEnd > dstWriteEnd && _eraseCells(dstWriteEnd, dstUsedEnd))
194196
{
195-
_eraseCells(dstWriteEnd, dstUsedEnd);
197+
return true;
196198
}
197199

198-
// If the beginning column is now not less than the end, that means the
199-
// content has been entirely erased, so we return true to let the caller
200-
// know that the slice should be deleted.
200+
// At this point, if the beginning column is not less than the end, that
201+
// means this was an empty slice into which nothing was copied, so we can
202+
// again return true to let the caller know it should be deleted.
201203
return _columnBegin >= _columnEnd;
202204
}
203205

@@ -210,10 +212,19 @@ void ImageSlice::EraseBlock(TextBuffer& buffer, const til::rect rect)
210212
}
211213
}
212214

213-
void ImageSlice::EraseCells(TextBuffer& buffer, const til::point at, const size_t distance)
215+
void ImageSlice::EraseCells(TextBuffer& buffer, const til::point at, const til::CoordType distance)
214216
{
215-
auto& row = buffer.GetMutableRowByOffset(at.y);
216-
EraseCells(row, at.x, gsl::narrow_cast<til::CoordType>(at.x + distance));
217+
auto x = at.x;
218+
auto y = at.y;
219+
auto distanceRemaining = distance;
220+
while (distanceRemaining > 0)
221+
{
222+
auto& row = buffer.GetMutableRowByOffset(y);
223+
EraseCells(row, x, x + distanceRemaining);
224+
distanceRemaining -= (static_cast<til::CoordType>(row.size()) - x);
225+
x = 0;
226+
y++;
227+
}
217228
}
218229

219230
void ImageSlice::EraseCells(ROW& row, const til::CoordType columnBegin, const til::CoordType columnEnd)

src/buffer/out/ImageSlice.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class ImageSlice
4141
static void CopyRow(const ROW& srcRow, ROW& dstRow);
4242
static void CopyCells(const ROW& srcRow, const til::CoordType srcColumn, ROW& dstRow, const til::CoordType dstColumnBegin, const til::CoordType dstColumnEnd);
4343
static void EraseBlock(TextBuffer& buffer, const til::rect rect);
44-
static void EraseCells(TextBuffer& buffer, const til::point at, const size_t distance);
44+
static void EraseCells(TextBuffer& buffer, const til::point at, const til::CoordType distance);
4545
static void EraseCells(ROW& row, const til::CoordType columnBegin, const til::CoordType columnEnd);
4646

4747
private:

0 commit comments

Comments
 (0)