Fix DBCS attribute corruption during reflow (#12853)

855e136 contains a regression which breaks buffer reflow if wide surrogate
characters are present. This happens because we made use of the
`TextBufferCellIterator` whose increment operator skips 2 cells for wide
characters. This created a "misalignment" in the reflow logic which was written
for cell-wise iteration. This commit fixes the issue, by reverting back to the
previous algorithm without iterators.

Closes #12837
Closes MSFT-38904421

## Validation Steps Performed
* Run ``pwsh -noprofile -command echo "`u{D83D}`u{DE43}"``
* Resizing conhost preserves all contents ✅
* Resizing Windows Terminal doesn't crash it ✅
* Added a test covering this issue ✅

(cherry picked from commit 10b9044)
Service-Card-Id: 80340091
Service-Version: Inbox

src/buffer/out/OutputCellIterator.cpp

@@ -97,14 +97,14 @@ OutputCellIterator::OutputCellIterator(const std::wstring_view utf16Text) :
 // Arguments:
 // - utf16Text - UTF-16 text range
 // - attribute - Color to apply over the entire range
-OutputCellIterator::OutputCellIterator(const std::wstring_view utf16Text, const TextAttribute attribute) :
+OutputCellIterator::OutputCellIterator(const std::wstring_view utf16Text, const TextAttribute& attribute, const size_t fillLimit) :
     _mode(Mode::Loose),
     _currentView(s_GenerateView(utf16Text, attribute)),
     _run(utf16Text),
     _attr(attribute),
     _distance(0),
     _pos(0),
-    _fillLimit(0)
+    _fillLimit(fillLimit)
 {
 }
 

src/buffer/out/OutputCellIterator.hpp

@@ -38,7 +38,7 @@ class OutputCellIterator final
     OutputCellIterator(const wchar_t& wch, const TextAttribute& attr, const size_t fillLimit = 0) noexcept;
     OutputCellIterator(const CHAR_INFO& charInfo, const size_t fillLimit = 0) noexcept;
     OutputCellIterator(const std::wstring_view utf16Text);
-    OutputCellIterator(const std::wstring_view utf16Text, const TextAttribute attribute);
+    OutputCellIterator(const std::wstring_view utf16Text, const TextAttribute& attribute, const size_t fillLimit = 0);
     OutputCellIterator(const gsl::span<const WORD> legacyAttributes) noexcept;
     OutputCellIterator(const gsl::span<const CHAR_INFO> charInfos) noexcept;
     OutputCellIterator(const gsl::span<const OutputCell> cells);

src/buffer/out/textBuffer.cpp

@@ -2230,8 +2230,6 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
         // the "right" boundary, which is one past the final valid
         // character)
         short iOldCol = 0;
-        auto chars{ row.GetCharRow().cbegin() };
-        auto attrs{ row.GetAttrRow().begin() };
         const auto copyRight = iRight;
         for (; iOldCol < copyRight; iOldCol++)
         {
@@ -2244,9 +2242,9 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
             try
             {
                 // TODO: MSFT: 19446208 - this should just use an iterator and the inserter...
-                const auto glyph = chars->Char();
-                const auto dbcsAttr = chars->DbcsAttr();
-                const auto textAttr = *attrs;
+                const auto glyph = row.GetCharRow().GlyphAt(iOldCol);
+                const auto dbcsAttr = row.GetCharRow().DbcsAttrAt(iOldCol);
+                const auto textAttr = row.GetAttrRow().GetAttrByColumn(iOldCol);
 
                 if (!newBuffer.InsertCharacter(glyph, dbcsAttr, textAttr))
                 {
@@ -2255,9 +2253,6 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
                 }
             }
             CATCH_RETURN();
-
-            ++chars;
-            ++attrs;
         }
 
         // GH#32: Copy the attributes from the rest of the row into this new buffer.
@@ -2288,21 +2283,18 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
         // for inserting an attr would be past the right of the new buffer.
         for (short copyAttrCol = iOldCol;
              copyAttrCol < cOldColsTotal && newAttrColumn < newWidth;
-             copyAttrCol++)
+             copyAttrCol++, newAttrColumn++)
         {
             try
             {
                 // TODO: MSFT: 19446208 - this should just use an iterator and the inserter...
-                const auto textAttr = *attrs;
+                const auto textAttr = row.GetAttrRow().GetAttrByColumn(copyAttrCol);
                 if (!newRow.GetAttrRow().SetAttrToEnd(newAttrColumn, textAttr))
                 {
                     break;
                 }
             }
             CATCH_LOG(); // Not worth dying over.
-
-            ++newAttrColumn;
-            ++attrs;
         }
 
         // If we found the old row that the caller was interested in, set the

src/host/ut_host/ScreenBufferTests.cpp

@@ -6308,6 +6308,7 @@ void ScreenBufferTests::TestReflowEndOfLineColor()
     stateMachine.ProcessString(L"\x1b[44m"); // Blue BG
     stateMachine.ProcessString(L" CCC \n"); // " abc " (with spaces on either side)
     stateMachine.ProcessString(L"\x1b[43m"); // yellow BG
+    stateMachine.ProcessString(L"\U0001F643\n"); // GH#12837: Support for surrogate characters during reflow
     stateMachine.ProcessString(L"\x1b[K"); // Erase line
     stateMachine.ProcessString(L"\x1b[2;6H"); // move the cursor to the end of the BBBBB's
 
@@ -6329,7 +6330,11 @@ void ScreenBufferTests::TestReflowEndOfLineColor()
         TestUtils::VerifyLineContains(iter2, L' ', defaultAttrs, static_cast<size_t>(width - 5));
 
         auto iter3 = tb.GetCellLineDataAt({ 0, 3 });
-        TestUtils::VerifyLineContains(iter3, L' ', yellow, static_cast<size_t>(width));
+        TestUtils::VerifyLineContains(iter3, L"\U0001F643", yellow, 2u);
+        TestUtils::VerifyLineContains(iter3, L' ', defaultAttrs, static_cast<size_t>(width - 2));
+
+        auto iter4 = tb.GetCellLineDataAt({ 0, 4 });
+        TestUtils::VerifyLineContains(iter4, L' ', yellow, static_cast<size_t>(width));
     };
 
     Log::Comment(L"========== Checking the buffer state (before) ==========");