Improve Unicode correctness of AdeptDispatch's rect fill

src/buffer/out/Row.cpp

@@ -146,8 +146,11 @@ void ROW::TransferAttributes(const til::small_rle<TextAttribute, uint16_t, 1>& a
 
 void ROW::CopyFrom(const ROW& source)
 {
-    til::CoordType begin = 0;
-    CopyTextFrom(0, til::CoordTypeMax, source, begin, til::CoordTypeMax);
+    RowCopyTextFromState state{
+        .source = source,
+        .columnLimit = til::CoordTypeMax,
+    };
+    CopyTextFrom(state);
     TransferAttributes(source.Attributes(), _columnCount);
     _lineRendition = source._lineRendition;
     _wrapForced = source._wrapForced;
@@ -451,46 +454,55 @@ catch (...)
     charsConsumed = ch - chBeg;
 }
 
-til::CoordType ROW::CopyTextFrom(til::CoordType columnBegin, til::CoordType columnLimit, const ROW& other, til::CoordType& otherBegin, til::CoordType otherLimit)
+void ROW::CopyTextFrom(RowCopyTextFromState& state)
 try
 {
-    const auto otherColBeg = other._clampedColumnInclusive(otherBegin);
-    const auto otherColLimit = other._clampedColumnInclusive(otherLimit);
+    const auto sourceColBeg = state.source._clampedColumnInclusive(state.sourceColumnBegin);
+    const auto sourceColLimit = state.source._columnCount;
     std::span<uint16_t> charOffsets;
     std::wstring_view chars;
 
-    if (otherColBeg < otherColLimit)
+    if (sourceColBeg < sourceColLimit)
     {
-        charOffsets = other._charOffsets.subspan(otherColBeg, static_cast<size_t>(otherColLimit) - otherColBeg + 1);
+        charOffsets = state.source._charOffsets.subspan(sourceColBeg, static_cast<size_t>(sourceColLimit) - sourceColBeg + 1);
         const auto charsOffset = charOffsets.front() & CharOffsetsMask;
         // We _are_ using span. But C++ decided that string_view and span aren't convertible.
         // _chars is a std::span for performance and because it refers to raw, shared memory.
 #pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1).
-        chars = { other._chars.data() + charsOffset, other._chars.size() - charsOffset };
+        chars = { state.source._chars.data() + charsOffset, state.source._chars.size() - charsOffset };
     }
 
-    WriteHelper h{ *this, columnBegin, columnLimit, chars };
-    // If we were to copy text from ourselves, we'd overwrite
-    // our _charOffsets and break Finish() which reads from it.
-    if (!h.IsValid() || this == &other)
-    {
-        assert(false); // You probably shouldn't call this function in the first place.
-        return h.colBeg;
-    }
-    // Any valid charOffsets array is at least 2 elements long (the 1st element is the start offset and the 2nd
-    // element is the length of the first glyph) and begins/ends with a non-trailer offset. We don't really
-    // need to test for the end offset, since `WriteHelper::WriteWithOffsets` already takes care of that.
-    if (charOffsets.size() < 2 || WI_IsFlagSet(charOffsets.front(), CharOffsetsTrailer))
+    WriteHelper h{ *this, state.columnBegin, state.columnLimit, chars };
+
+    const auto destinationInvalid = !h.IsValid();
+    const auto sourceInvalid =
+        // If we were to copy text from ourselves, we'd overwrite
+        // our _charOffsets and break Finish() which reads from it.
+        this == &state.source ||
+        // Any valid charOffsets array is at least 2 elements long (the 1st element is the start offset and the 2nd
+        // element is the length of the first glyph) and begins/ends with a non-trailer offset. We don't really
+        // need to test for the end offset, since `WriteHelper::WriteWithOffsets` already takes care of that.
+        charOffsets.size() < 2 || WI_IsFlagSet(charOffsets.front(), CharOffsetsTrailer);
+
+    if (destinationInvalid || sourceInvalid)
     {
-        assert(false);
-        otherBegin = other.size();
-        return h.colBeg;
+        state.columnEnd = h.colBeg;
+        state.columnBeginDirty = h.colBeg;
+        state.columnEndDirty = h.colBeg;
+        // When the source is invalid we'll mark it as if we had it fully
+        // consumed so that the caller hopefully doesn't use it again.
+        state.sourceColumnEnd = sourceInvalid ? sourceColLimit : sourceColBeg;
+        return;
     }
+
     h.CopyTextFrom(charOffsets);
     h.Finish();
 
-    otherBegin += h.colEnd - h.colBeg;
-    return h.colEndDirty;
+    // state.columnEnd is computed identical to ROW::ReplaceText. Check it out for more information.
+    state.columnEnd = h.charsConsumed == chars.size() ? h.colEnd : h.colLimit;
+    state.columnBeginDirty = h.colBegDirty;
+    state.columnEndDirty = h.colEndDirty;
+    state.sourceColumnEnd = sourceColBeg + h.colEnd - h.colBeg;
 }
 catch (...)
 {

src/buffer/out/Row.hpp

@@ -26,6 +26,7 @@ Revision History:
 #include "OutputCell.hpp"
 #include "OutputCellIterator.hpp"
 
+class ROW;
 class TextBuffer;
 
 enum class DelimiterClass
@@ -57,6 +58,28 @@ struct RowWriteState
     til::CoordType columnEndDirty = 0; // OUT
 };
 
+struct RowCopyTextFromState
+{
+    // The row to copy text from.
+    const ROW& source; // IN
+    // The column at which to start writing.
+    til::CoordType columnBegin = 0; // IN
+    // The first column which should not be written to anymore.
+    til::CoordType columnLimit = 0; // IN
+    // The column at which to start reading from source.
+    til::CoordType sourceColumnBegin = 0; // IN
+
+    til::CoordType columnEnd = 0; // OUT
+    // The first column that got modified by this write operation. In case that the first glyph we write overwrites
+    // the trailing half of a wide glyph, leadingSpaces will be 1 and this value will be 1 less than colBeg.
+    til::CoordType columnBeginDirty = 0; // OUT
+    // This is 1 past the last column that was modified and will be 1 past columnEnd if we overwrote
+    // the leading half of a wide glyph and had to fill the trailing half with whitespace.
+    til::CoordType columnEndDirty = 0; // OUT
+    // This is 1 past the last column that was read from.
+    til::CoordType sourceColumnEnd = 0; // OUT
+};
+
 class ROW final
 {
 public:
@@ -109,7 +132,7 @@ class ROW final
     void ReplaceAttributes(til::CoordType beginIndex, til::CoordType endIndex, const TextAttribute& newAttr);
     void ReplaceCharacters(til::CoordType columnBegin, til::CoordType width, const std::wstring_view& chars);
     void ReplaceText(RowWriteState& state);
-    til::CoordType CopyTextFrom(til::CoordType columnBegin, til::CoordType columnLimit, const ROW& other, til::CoordType& otherBegin, til::CoordType otherLimit);
+    void CopyTextFrom(RowCopyTextFromState& state);
 
     til::small_rle<TextAttribute, uint16_t, 1>& Attributes() noexcept;
     const til::small_rle<TextAttribute, uint16_t, 1>& Attributes() const noexcept;

src/buffer/out/textBuffer.cpp

@@ -196,7 +196,15 @@ ROW& TextBuffer::GetRowByOffset(const til::CoordType index)
 // Returns a row filled with whitespace and the current attributes, for you to freely use.
 ROW& TextBuffer::GetScratchpadRow()
 {
-    return _getRowByOffsetDirect(0);
+    return GetScratchpadRow(_currentAttributes);
+}
+
+// Returns a row filled with whitespace and the given attributes, for you to freely use.
+ROW& TextBuffer::GetScratchpadRow(const TextAttribute& attributes)
+{
+    auto& r = _getRowByOffsetDirect(0);
+    r.Reset(attributes);
+    return r;
 }
 
 #pragma warning(pop)
@@ -429,10 +437,8 @@ void TextBuffer::ConsumeGrapheme(std::wstring_view& chars) noexcept
     chars = til::utf16_pop(chars);
 }
 
-// This function is intended for writing regular "lines" of text and only the `state.text` and`state.columnBegin`
-// fields are being used, whereas `state.columnLimit` is automatically overwritten by the line width of the given row.
-// This allows this function to automatically set the wrap-forced field of the row, which is also the return value.
-// The return value indicates to the caller whether the cursor should be moved to the next line.
+// This function is intended for writing regular "lines" of text as it'll set the wrap flag on the given row.
+// You can continue calling the function on the same row as long as state.columnEnd < state.columnLimit.
 void TextBuffer::WriteLine(til::CoordType row, bool wrapAtEOL, const TextAttribute& attributes, RowWriteState& state)
 {
     auto& r = GetRowByOffset(row);
@@ -448,6 +454,53 @@ void TextBuffer::WriteLine(til::CoordType row, bool wrapAtEOL, const TextAttribu
     TriggerRedraw(Viewport::FromExclusive({ state.columnBeginDirty, row, state.columnEndDirty, row + 1 }));
 }
 
+// Fills an area of the buffer with a given fill character(s) and attributes.
+void TextBuffer::FillRect(const til::rect& rect, const std::wstring_view& fill, const TextAttribute& attributes)
+{
+    if (!rect || fill.empty())
+    {
+        return;
+    }
+
+    auto& scratchpad = GetScratchpadRow(attributes);
+
+    // The scratchpad row gets reset to whitespace by default, so there's no need to initialize it again.
+    if (fill != L" ")
+    {
+        RowWriteState state{
+            .columnLimit = rect.right,
+            .columnEnd = rect.left,
+        };
+
+        // Fill the scratchpad row with consecutive copies of "fill" up to the amount we need.
+        while (state.columnEnd < rect.right)
+        {
+            state.columnBegin = state.columnEnd;
+            state.text = fill;
+            scratchpad.ReplaceText(state);
+        }
+    }
+
+    // Fill the given rows with copies of the scratchpad row. That's a little
+    // slower when filling just a single row, but will be much faster for >1 rows.
+    {
+        RowCopyTextFromState state{
+            .source = scratchpad,
+            .columnBegin = rect.left,
+            .columnLimit = rect.right,
+            .sourceColumnBegin = rect.left,
+        };
+
+        for (auto y = rect.top; y < rect.bottom; ++y)
+        {
+            auto& r = GetRowByOffset(y);
+            r.CopyTextFrom(state);
+            r.ReplaceAttributes(rect.left, rect.right, attributes);
+            TriggerRedraw(Viewport::FromExclusive({ state.columnBeginDirty, y, state.columnEndDirty, y + 1 }));
+        }
+    }
+}
+
 // Routine Description:
 // - Writes cells to the output buffer. Writes at the cursor.
 // Arguments:

src/buffer/out/textBuffer.hpp

@@ -85,6 +85,7 @@ class TextBuffer final
 
     // row manipulation
     ROW& GetScratchpadRow();
+    ROW& GetScratchpadRow(const TextAttribute& attributes);
     const ROW& GetRowByOffset(til::CoordType index) const;
     ROW& GetRowByOffset(til::CoordType index);
 
@@ -98,6 +99,7 @@ class TextBuffer final
     // Text insertion functions
     static void ConsumeGrapheme(std::wstring_view& chars) noexcept;
     void WriteLine(til::CoordType row, bool wrapAtEOL, const TextAttribute& attributes, RowWriteState& state);
+    void FillRect(const til::rect& rect, const std::wstring_view& fill, const TextAttribute& attributes);
 
     OutputCellIterator Write(const OutputCellIterator givenIt);