diff --git a/build/pipelines/templates/build-console-steps.yml b/build/pipelines/templates/build-console-steps.yml index 174c8cc5bc7..22705ee6a70 100644 --- a/build/pipelines/templates/build-console-steps.yml +++ b/build/pipelines/templates/build-console-steps.yml @@ -159,6 +159,8 @@ steps: $(Build.SourcesDirectory)/bin/$(RationalizedBuildPlatform)/$(BuildConfiguration)/*.dll $(Build.SourcesDirectory)/bin/$(RationalizedBuildPlatform)/$(BuildConfiguration)/*.xml **/Microsoft.VCLibs.*.appx + **/*unit.test*.dll + **/*unit.test*.manifest **/TestHostApp/*.exe **/TestHostApp/*.dll **/TestHostApp/*.xml diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index d6d7e8243b2..1939fafb506 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2199,7 +2199,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); @@ -2243,7 +2244,11 @@ 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; + auto chars{ row.GetCharRow().cbegin() }; + auto attrs{ row.GetAttrRow().begin() }; + const auto copyRight = iRight; + for (; iOldCol < copyRight; iOldCol++) { if (iOldCol == cOldCursorPos.X && iOldRow == cOldCursorPos.Y) { @@ -2254,9 +2259,9 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, try { // TODO: MSFT: 19446208 - this should just use an iterator and the inserter... - const auto glyph = row.GetCharRow().GlyphAt(iOldCol); - const auto dbcsAttr = row.GetCharRow().DbcsAttrAt(iOldCol); - const auto textAttr = row.GetAttrRow().GetAttrByColumn(iOldCol); + const auto glyph = chars->Char(); + const auto dbcsAttr = chars->DbcsAttr(); + const auto textAttr = *attrs; if (!newBuffer.InsertCharacter(glyph, dbcsAttr, textAttr)) { @@ -2265,6 +2270,54 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, } } CATCH_RETURN(); + + ++chars; + ++attrs; + } + + // 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. We already copied all _text_ with colors, + // but it's possible for someone to just put some color into the + // buffer to the right of that without any text (as just spaces). The + // buffer looks weird to the user when we resize and it starts losing + // those colors, so we need to copy them over too... as long as there + // is space. The last attr in the row will be extended to the end of + // the row in the new buffer. + // - if the old buffer is WIDER, 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 at 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 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 = *attrs; + 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 @@ -2300,7 +2353,7 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, // only because we ran out of space. if (iRight < cOldColsTotal && !row.WasWrapForced()) { - if (iRight == cOldCursorPos.X && iOldRow == cOldCursorPos.Y) + if (!fFoundCursorPos && (iRight == cOldCursorPos.X && iOldRow == cOldCursorPos.Y)) { cNewCursorPos = newCursor.GetPosition(); fFoundCursorPos = true; @@ -2351,6 +2404,34 @@ 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 last + // 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); + + // Optimization: Since all these rows are below the last printable char, + // we can reasonably assume that they are filled with just spaces. + // That's convenient, we can just copy the attr row from the old buffer + // into the new one, and resize the row to match. We'll rely on the + // behavior of ATTR_ROW::Resize to trim down when narrower, or extend + // the last attr when wider. + auto& newRow = newBuffer.GetRowByOffset(newRowY); + const auto newWidth = newBuffer.GetLineWidth(newRowY); + newRow.GetAttrRow() = row.GetAttrRow(); + newRow.GetAttrRow().Resize(newWidth); + + newRowY++; + } + if (SUCCEEDED(hr)) { // Finish copying remaining parameters from the old text buffer to the new one diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index fe671a65acd..db20468f42f 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -2968,7 +2968,7 @@ void ConptyRoundtripTests::ResizeInitializeBufferWithDefaultAttrs() else if (leaveTrailingChar && row == 3) { auto iter = TestUtils::VerifyLineContains(tb, { 0, row }, L'#', greenAttrs, 1u); - TestUtils::VerifyLineContains(iter, L' ', (afterResize ? greenAttrs : actualDefaultAttrs), static_cast(width - 1)); + TestUtils::VerifyLineContains(iter, L' ', (actualDefaultAttrs), static_cast(width - 1)); } else { diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 320e1148dc3..2d812d9f919 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1471,7 +1471,7 @@ bool SCREEN_INFORMATION::IsMaximizedY() const coordCursorHeightDiff.Y = sCursorHeightInViewportAfter - sCursorHeightInViewportBefore; LOG_IF_FAILED(SetViewportOrigin(false, coordCursorHeightDiff, true)); - _textBuffer->SetCurrentAttributes(oldPrimaryAttributes); + newTextBuffer->SetCurrentAttributes(oldPrimaryAttributes); _textBuffer.swap(newTextBuffer); } diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 0e31c2b32c2..8d1178efeb7 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/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 using namespace WEX::Common; @@ -25,6 +27,7 @@ using namespace WEX::TestExecution; using namespace Microsoft::Console::Types; using namespace Microsoft::Console::Interactivity; using namespace Microsoft::Console::VirtualTerminal; +using namespace TerminalCoreUnitTests; class ScreenBufferTests { @@ -220,6 +223,10 @@ class ScreenBufferTests TEST_METHOD(RetainHorizontalOffsetWhenMovingToBottom); TEST_METHOD(TestWriteConsoleVTQuirkMode); + + TEST_METHOD(TestReflowEndOfLineColor); + TEST_METHOD(TestReflowSmallerLongLineWithColor); + TEST_METHOD(TestReflowBiggerLongLineWithColor); }; void ScreenBufferTests::SingleAlternateBufferCreationTest() @@ -6250,3 +6257,239 @@ 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:dy", L"{-1, 0, 1}") + 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(width - 5)); + + auto iter1 = tb.GetCellLineDataAt({ 0, 1 }); + TestUtils::VerifyLineContains(iter1, L'B', green, 5u); + TestUtils::VerifyLineContains(iter1, L' ', defaultAttrs, static_cast(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(width - 5)); + + auto iter3 = tb.GetCellLineDataAt({ 0, 3 }); + TestUtils::VerifyLineContains(iter3, L' ', yellow, static_cast(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(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(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(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); +}