Manually copy trailing attributes on a resize

src/buffer/out/textBuffer.cpp

@@ -2197,7 +2197,8 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
     bool foundOldVisible = false;
     HRESULT hr = S_OK;
     // Loop through all the rows of the old buffer and reprint them into the new buffer
-    for (short iOldRow = 0; iOldRow < cOldRowsTotal; iOldRow++)
+    short iOldRow = 0;
+    for (; iOldRow < cOldRowsTotal; iOldRow++)
     {
         // Fetch the row and its "right" which is the last printable character.
         const ROW& row = oldBuffer.GetRowByOffset(iOldRow);
@@ -2241,7 +2242,8 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
         // Loop through every character in the current row (up to
         // the "right" boundary, which is one past the final valid
         // character)
-        for (short iOldCol = 0; iOldCol < iRight; iOldCol++)
+        short iOldCol = 0;
+        for (; iOldCol < iRight; iOldCol++)
         {
             if (iOldCol == cOldCursorPos.X && iOldRow == cOldCursorPos.Y)
             {
@@ -2265,6 +2267,48 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
             CATCH_RETURN();
         }
 
+        // GH#32: Copy the attributes from the rest of the row into this new buffer.
+        // From where we are in the old buffer, to the end of the row, copy the
+        // remaining attributes.
+        // - if the old buffer is smaller than the new buffer, then just copy
+        //   what we have. as it was. The last attr in the row will be extended
+        //   to the end of the row in the new buffer.
+        // - if the old buffer is BIGGER, than we might have wrapped onto a new
+        //   line. Use the cursor's position's Y so that we know where the new
+        //   row is, and start writing st the cursor position. Again, the attr
+        //   in the last column of the old row will be extended to the end of the
+        //   row that the text was flowed onto.
+        //   - if we the text in the old buffer didn't actually fill the whole
+        //     line in the new buffer, then we didn't wrap. That's fine. just
+        //     copy attributes from the old row till the end of the new row, and
+        //     move on.
+        const auto newRowY = newCursor.GetPosition().Y;
+        auto& newRow = newBuffer.GetRowByOffset(newRowY);
+        auto newAttrColumn = newCursor.GetPosition().X;
+        const auto newWidth = newBuffer.GetLineWidth(newRowY);
+        // Stop when we get to the end of the buffer width, or the new position
+        // for inserting an attr would be past the right of the new buffer.
+        for (short copyAttrCol = iOldCol;
+             copyAttrCol < cOldColsTotal && newAttrColumn < newWidth;
+             copyAttrCol++)
+        {
+            try
+            {
+                // TODO: MSFT: 19446208 - this should just use an iterator and the inserter...
+                const auto textAttr = row.GetAttrRow().GetAttrByColumn(copyAttrCol);
+                if (!newRow.GetAttrRow().SetAttrToEnd(newAttrColumn, textAttr))
+                {
+                    break;
+                }
+            }
+            CATCH_LOG(); // Not worth dying over.
+
+            newAttrColumn++;
+        }
+        // FOR DISCUSSION: Maybe also try to leave a
+        // default-attributes-to-the-end attr here, for the case where the line
+        // had color, then flowed onto the subsequent row.
+
         // If we found the old row that the caller was interested in, set the
         // out value of that parameter to the cursor's current Y position (the
         // new location of the _end_ of that row in the buffer).
@@ -2349,6 +2393,51 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
             }
         }
     }
+
+    // Finish copying buffer attributes to remaining rows below the last
+    // printable character. This is to fix the `color 2f` scenario, where you
+    // change the buffer colors then resize and everything below the past
+    // printable char gets reset. See GH #12567
+    auto newRowY = newCursor.GetPosition().Y + 1;
+    const auto newHeight = newBuffer.GetSize().Height();
+    const auto oldHeight = oldBuffer.GetSize().Height();
+    for (;
+         iOldRow < oldHeight && newRowY < newHeight;
+         iOldRow++)
+    {
+        const ROW& row = oldBuffer.GetRowByOffset(iOldRow);
+
+        // Loop through every character in the current row (up to
+        // the "right" boundary, which is one past the final valid
+        // character)
+
+        auto& newRow = newBuffer.GetRowByOffset(newRowY);
+        const auto newWidth = newBuffer.GetLineWidth(newRowY);
+        const auto minWidth = std::min(oldBuffer.GetLineWidth(iOldRow), newWidth);
+        uint16_t newAttrColumn = 0u;
+        // Stop when we get to the end of the buffer width, or the new position
+        // for inserting an attr would be past the right of the new buffer.
+        for (short copyAttrCol = 0;
+             copyAttrCol < minWidth;
+             copyAttrCol++)
+        {
+            try
+            {
+                // TODO: MSFT: 19446208 - this should just use an iterator and the inserter.
+                // May even just be faster to just copy the ATTR_ROW to the new row, then ATTR_ROW::Resize.
+                const auto textAttr = row.GetAttrRow().GetAttrByColumn(copyAttrCol);
+                if (!newRow.GetAttrRow().SetAttrToEnd(newAttrColumn, textAttr))
+                {
+                    break;
+                }
+            }
+            CATCH_LOG(); // Not worth dying over.
+
+            newAttrColumn++;
+        }
+        newRowY++;
+    }
+
     if (SUCCEEDED(hr))
     {
         // Finish copying remaining parameters from the old text buffer to the new one

src/host/ut_host/ScreenBufferTests.cpp

@@ -17,6 +17,8 @@
 #include "../../inc/conattrs.hpp"
 #include "../../types/inc/Viewport.hpp"
 
+#include "../../cascadia/UnitTests_TerminalCore/TestUtils.h"
+
 #include <sstream>
 
 using namespace WEX::Common;
@@ -26,6 +28,7 @@ using namespace Microsoft::Console::Types;
 using namespace Microsoft::Console::Interactivity;
 using namespace Microsoft::Console::Render;
 using namespace Microsoft::Console::VirtualTerminal;
+using namespace TerminalCoreUnitTests;
 
 class ScreenBufferTests
 {
@@ -222,6 +225,10 @@ class ScreenBufferTests
     TEST_METHOD(RetainHorizontalOffsetWhenMovingToBottom);
 
     TEST_METHOD(TestWriteConsoleVTQuirkMode);
+
+    TEST_METHOD(TestReflowEndOfLineColor);
+    TEST_METHOD(TestReflowSmallerLongLineWithColor);
+    TEST_METHOD(TestReflowBiggerLongLineWithColor);
 };
 
 void ScreenBufferTests::SingleAlternateBufferCreationTest()
@@ -6263,3 +6270,241 @@ void ScreenBufferTests::TestWriteConsoleVTQuirkMode()
         verifyLastAttribute(vtWhiteOnBlack256Attribute);
     }
 }
+
+void ScreenBufferTests::TestReflowEndOfLineColor()
+{
+    BEGIN_TEST_METHOD_PROPERTIES()
+        TEST_METHOD_PROPERTY(L"Data:dx", L"{-1, 0, 1}")
+        // TEST_METHOD_PROPERTY(L"Data:dx", L"{1}")
+        TEST_METHOD_PROPERTY(L"Data:dy", L"{-1, 0, 1}")
+        // TEST_METHOD_PROPERTY(L"Data:dy", L"{0}")
+    END_TEST_METHOD_PROPERTIES();
+
+    INIT_TEST_PROPERTY(int, dx, L"The change in width of the buffer");
+    INIT_TEST_PROPERTY(int, dy, L"The change in height of the buffer");
+
+    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& stateMachine = si.GetStateMachine();
+
+    gci.SetWrapText(true);
+
+    auto defaultAttrs = si.GetAttributes();
+    auto red = defaultAttrs;
+    red.SetIndexedBackground(TextColor::DARK_RED);
+    auto green = defaultAttrs;
+    green.SetIndexedBackground(TextColor::DARK_GREEN);
+    auto blue = defaultAttrs;
+    blue.SetIndexedBackground(TextColor::DARK_BLUE);
+    auto yellow = defaultAttrs;
+    yellow.SetIndexedBackground(TextColor::DARK_YELLOW);
+
+    Log::Comment(L"Make sure the viewport is at 0,0");
+    VERIFY_SUCCEEDED(si.SetViewportOrigin(true, COORD({ 0, 0 }), true));
+
+    Log::Comment(L"Fill buffer with some data");
+    stateMachine.ProcessString(L"\x1b[H");
+    stateMachine.ProcessString(L"\x1b[41m"); // Red BG
+    stateMachine.ProcessString(L"AAAAA"); // AAAAA
+    stateMachine.ProcessString(L"\x1b[42m"); // Green BG
+    stateMachine.ProcessString(L"\nBBBBB\n"); // BBBBB
+    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"\x1b[K"); // Erase line
+    stateMachine.ProcessString(L"\x1b[2;6H"); // move the cursor to the end of the BBBBB's
+
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& /*viewport*/, const bool /*before*/) {
+        const short width = tb.GetSize().Width();
+        Log::Comment(NoThrowString().Format(L"Buffer width: %d", width));
+
+        auto iter0 = TestUtils::VerifyLineContains(tb, { 0, 0 }, L'A', red, 5u);
+        TestUtils::VerifyLineContains(iter0, L' ', defaultAttrs, static_cast<size_t>(width - 5));
+
+        auto iter1 = tb.GetCellLineDataAt({ 0, 1 });
+        TestUtils::VerifyLineContains(iter1, L'B', green, 5u);
+        TestUtils::VerifyLineContains(iter1, L' ', defaultAttrs, static_cast<size_t>(width - 5));
+
+        auto iter2 = tb.GetCellLineDataAt({ 0, 2 });
+        TestUtils::VerifyLineContains(iter2, L' ', blue, 1u);
+        TestUtils::VerifyLineContains(iter2, L'C', blue, 3u);
+        TestUtils::VerifyLineContains(iter2, L' ', blue, 1u);
+        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));
+    };
+
+    Log::Comment(L"========== Checking the buffer state (before) ==========");
+    verifyBuffer(si.GetTextBuffer(), til::rect{ si.GetViewport().ToInclusive() }, true);
+
+    Log::Comment(L"========== resize buffer ==========");
+    const til::point delta{ dx, dy };
+    const til::point oldSize{ si.GetBufferSize().Dimensions() };
+    const til::point newSize{ oldSize + delta };
+    VERIFY_SUCCEEDED(si.ResizeWithReflow(newSize.to_win32_coord()));
+
+    Log::Comment(L"========== Checking the buffer state (after) ==========");
+    verifyBuffer(si.GetTextBuffer(), til::rect{ si.GetViewport().ToInclusive() }, false);
+}
+
+void ScreenBufferTests::TestReflowSmallerLongLineWithColor()
+{
+    Log::Comment(L"Reflow the buffer such that a long, line of text flows onto "
+                 L"the next line. Check that the trailing attributes were copied"
+                 L" to the new row.");
+
+    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& stateMachine = si.GetStateMachine();
+
+    gci.SetWrapText(true);
+
+    auto defaultAttrs = si.GetAttributes();
+    auto red = defaultAttrs;
+    red.SetIndexedBackground(TextColor::DARK_RED);
+    auto green = defaultAttrs;
+    green.SetIndexedBackground(TextColor::DARK_GREEN);
+    auto blue = defaultAttrs;
+    blue.SetIndexedBackground(TextColor::DARK_BLUE);
+    auto yellow = defaultAttrs;
+    yellow.SetIndexedBackground(TextColor::DARK_YELLOW);
+
+    Log::Comment(L"Make sure the viewport is at 0,0");
+    VERIFY_SUCCEEDED(si.SetViewportOrigin(true, COORD({ 0, 0 }), true));
+
+    Log::Comment(L"Fill buffer with some data");
+    stateMachine.ProcessString(L"\x1b[H");
+    stateMachine.ProcessString(L"\x1b[41m"); // Red BG
+    stateMachine.ProcessString(L"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); // 35 A's
+    stateMachine.ProcessString(L"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); // 35 more, 70 total
+    stateMachine.ProcessString(L"\x1b[42m"); // Green BG
+    stateMachine.ProcessString(L" BBB \n"); // " BBB " with spaces on either side).
+    // Attributes fill 70 red, 5 green, the last 5 are {whatever it was before}
+
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& /*viewport*/, const bool before) {
+        const short width = tb.GetSize().Width();
+        Log::Comment(NoThrowString().Format(L"Buffer width: %d", width));
+
+        if (before)
+        {
+            auto iter0 = TestUtils::VerifyLineContains(tb, { 0, 0 }, L'A', red, 70u);
+            TestUtils::VerifyLineContains(iter0, L' ', green, 1u);
+            TestUtils::VerifyLineContains(iter0, L'B', green, 3u);
+            TestUtils::VerifyLineContains(iter0, L' ', green, 1u);
+            TestUtils::VerifyLineContains(iter0, L' ', defaultAttrs, 5u);
+        }
+        else
+        {
+            auto iter0 = TestUtils::VerifyLineContains(tb, { 0, 0 }, L'A', red, 65u);
+
+            auto iter1 = tb.GetCellLineDataAt({ 0, 1 });
+            TestUtils::VerifyLineContains(iter1, L'A', red, 5u);
+            TestUtils::VerifyLineContains(iter1, L' ', green, 1u);
+            TestUtils::VerifyLineContains(iter1, L'B', green, 3u);
+            TestUtils::VerifyLineContains(iter1, L' ', green, 1u);
+
+            // We don't want to necessarily verify the contents of the rest of the
+            // line, but we will anyways. Right now, we expect the last attrs on the
+            // original row to fill the remainder of the row it flowed onto. We may
+            // want to change that in the future. If we do, this check can always be
+            // changed.
+            TestUtils::VerifyLineContains(iter1, L' ', defaultAttrs, static_cast<size_t>(width - 10));
+        }
+    };
+
+    Log::Comment(L"========== Checking the buffer state (before) ==========");
+    verifyBuffer(si.GetTextBuffer(), til::rect{ si.GetViewport().ToInclusive() }, true);
+
+    Log::Comment(L"========== resize buffer ==========");
+    const til::point delta{ -15, 0 };
+    const til::point oldSize{ si.GetBufferSize().Dimensions() };
+    const til::point newSize{ oldSize + delta };
+    VERIFY_SUCCEEDED(si.ResizeWithReflow(newSize.to_win32_coord()));
+
+    // Buffer is now 65 wide. 65 A's that wrapped onto the next row, where there
+    // are also 3 B's wrapped in spaces.
+    Log::Comment(L"========== Checking the buffer state (after) ==========");
+    verifyBuffer(si.GetTextBuffer(), til::rect{ si.GetViewport().ToInclusive() }, false);
+}
+
+void ScreenBufferTests::TestReflowBiggerLongLineWithColor()
+{
+    Log::Comment(L"Reflow the buffer such that a wrapped line of text 'de-flows' onto the previous line.");
+
+    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& stateMachine = si.GetStateMachine();
+
+    gci.SetWrapText(true);
+
+    auto defaultAttrs = si.GetAttributes();
+    auto red = defaultAttrs;
+    red.SetIndexedBackground(TextColor::DARK_RED);
+    auto green = defaultAttrs;
+    green.SetIndexedBackground(TextColor::DARK_GREEN);
+    auto blue = defaultAttrs;
+    blue.SetIndexedBackground(TextColor::DARK_BLUE);
+    auto yellow = defaultAttrs;
+    yellow.SetIndexedBackground(TextColor::DARK_YELLOW);
+
+    Log::Comment(L"Make sure the viewport is at 0,0");
+    VERIFY_SUCCEEDED(si.SetViewportOrigin(true, COORD({ 0, 0 }), true));
+
+    Log::Comment(L"Fill buffer with some data");
+    stateMachine.ProcessString(L"\x1b[H");
+    stateMachine.ProcessString(L"\x1b[41m"); // Red BG
+    stateMachine.ProcessString(L"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); // 40 A's
+    stateMachine.ProcessString(L"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); // 40 more, to wrap
+    stateMachine.ProcessString(L"AAAAA"); // 5 more. 85 total.
+    stateMachine.ProcessString(L"\x1b[42m"); // Green BG
+    stateMachine.ProcessString(L" BBB \n"); // " BBB " with spaces on either side).
+    // Attributes fill 85 red, 5 green, the rest are {whatever it was before}
+
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& /*viewport*/, const bool before) {
+        const short width = tb.GetSize().Width();
+        Log::Comment(NoThrowString().Format(L"Buffer width: %d", width));
+
+        if (before)
+        {
+            auto iter0 = TestUtils::VerifyLineContains(tb, { 0, 0 }, L'A', red, 80u);
+
+            auto iter1 = tb.GetCellLineDataAt({ 0, 1 });
+            TestUtils::VerifyLineContains(iter1, L'A', red, 5u);
+            TestUtils::VerifyLineContains(iter1, L' ', green, 1u);
+            TestUtils::VerifyLineContains(iter1, L'B', green, 3u);
+            TestUtils::VerifyLineContains(iter1, L' ', green, 1u);
+            TestUtils::VerifyLineContains(iter1, L' ', defaultAttrs, static_cast<size_t>(width - 10));
+        }
+        else
+        {
+            auto iter0 = TestUtils::VerifyLineContains(tb, { 0, 0 }, L'A', red, 85u);
+            TestUtils::VerifyLineContains(iter0, L' ', green, 1u);
+            TestUtils::VerifyLineContains(iter0, L'B', green, 3u);
+            TestUtils::VerifyLineContains(iter0, L' ', green, 1u);
+
+            // We don't want to necessarily verify the contents of the rest of
+            // the line, but we will anyways. Differently than
+            // TestReflowSmallerLongLineWithColor: In this test, the since we're
+            // de-flowing row 1 onto row 0, and the trailing spaces in row 1 are
+            // default attrs, then that's the attrs we'll use to finish filling
+            // up the 0th row in the new buffer. Again, we may want to change
+            // that in the future. If we do, this check can always be changed.
+            TestUtils::VerifyLineContains(iter0, L' ', defaultAttrs, static_cast<size_t>(width - 90));
+        }
+    };
+
+    Log::Comment(L"========== Checking the buffer state (before) ==========");
+    verifyBuffer(si.GetTextBuffer(), til::rect{ si.GetViewport().ToInclusive() }, true);
+
+    Log::Comment(L"========== resize buffer ==========");
+    const til::point delta{ 15, 0 };
+    const til::point oldSize{ si.GetBufferSize().Dimensions() };
+    const til::point newSize{ oldSize + delta };
+    VERIFY_SUCCEEDED(si.ResizeWithReflow(newSize.to_win32_coord()));
+
+    // Buffer is now 95 wide. 85 A's that de-flowed onto the first row, where
+    // there are also 3 B's wrapped in spaces, and finally 5 trailing spaces.
+    Log::Comment(L"========== Checking the buffer state (after) ==========");
+    verifyBuffer(si.GetTextBuffer(), til::rect{ si.GetViewport().ToInclusive() }, false);
+}