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
This commit is contained in:
James Holderness 2025-04-30 00:15:38 +01:00 committed by GitHub
parent 0568173aba
commit 08e76da3a1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 22 additions and 11 deletions

View File

@ -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)

View File

@ -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: