diff --git a/.github/actions/spell-check/patterns/patterns.txt b/.github/actions/spell-check/patterns/patterns.txt index 157451cf94c..11068abd8e4 100644 --- a/.github/actions/spell-check/patterns/patterns.txt +++ b/.github/actions/spell-check/patterns/patterns.txt @@ -5,3 +5,6 @@ https://(?:(?:www\.|)youtube\.com|youtu.be)/[-a-zA-Z0-9?&=]* Scro\&ll # selectionInput.cpp :\\windows\\syste\b +TestUtils::VerifyExpectedString\(tb, L"[^"]+" +hostSm\.ProcessString\(L"[^"]+" +\b([A-Za-z])\1{3,}\b diff --git a/.github/actions/spell-check/whitelist/alphabet.txt b/.github/actions/spell-check/whitelist/alphabet.txt index 2b4d4ba8471..596715059ea 100644 --- a/.github/actions/spell-check/whitelist/alphabet.txt +++ b/.github/actions/spell-check/whitelist/alphabet.txt @@ -7,10 +7,10 @@ ABCDEFGHIJ abcdefghijk abcdefghijklmnop ABCDEFGHIJKLMNOPQRST +ABCDEFGHIJKLMNOPQRSTQ abcdefghijklmnopqrstuvwxyz -QQQQQ -QQQQQQQQQ -QQQQQQQQQQ +ABCDEFGHIJPQRST +ABCDEFGHIJPQRSTQ QQQQQQQQQQABCDEFGHIJ QQQQQQQQQQABCDEFGHIJKLMNOPQRSTQQQQQQQQQ QQQQQQQQQQABCDEFGHIJKLMNOPQRSTQQQQQQQQQQ @@ -19,9 +19,6 @@ qrstuvwxyz qwerty QWERTYUIOP qwertyuiopasdfg -TTTTTTTTTTTTTTTTTTTTTTTTTT -VVVVVVVVVVVVVVVV -yyyy ZAAZZ ZABBZ ZBAZZ @@ -31,4 +28,3 @@ ZYXWVUT ZZBBZ ZZZBB ZZZBZ -ZZZZZ diff --git a/.github/actions/spell-check/whitelist/whitelist.txt b/.github/actions/spell-check/whitelist/whitelist.txt index e0017e6b097..be58c507ff0 100644 --- a/.github/actions/spell-check/whitelist/whitelist.txt +++ b/.github/actions/spell-check/whitelist/whitelist.txt @@ -2893,6 +2893,7 @@ ZCtrl zd zh ZM +zsh zu zxcvbnm zy diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index abfcaad234d..e85fd243ddd 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -23,6 +23,7 @@ class InputBuffer; // This for some reason needs to be fwd-decl'd #include "../host/inputBuffer.hpp" #include "../host/readDataCooked.hpp" +#include "../host/output.h" #include "test/CommonState.hpp" #include "../cascadia/TerminalCore/Terminal.hpp" @@ -50,6 +51,9 @@ using namespace TerminalCoreUnitTests; class TerminalCoreUnitTests::ConptyRoundtripTests final { + // !!! DANGER: Many tests in this class expect the Terminal and Host buffers + // to be 80x32. If you change these, you'll probably inadvertently break a + // bunch of tests !!! static const SHORT TerminalViewWidth = 80; static const SHORT TerminalViewHeight = 32; @@ -174,6 +178,16 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final TEST_METHOD(TestResizeHeight); + TEST_METHOD(OutputWrappedLinesAtTopOfBuffer); + TEST_METHOD(OutputWrappedLinesAtBottomOfBuffer); + TEST_METHOD(ScrollWithChangesInMiddle); + TEST_METHOD(DontWrapMoveCursorInSingleFrame); + TEST_METHOD(ClearHostTrickeryTest); + TEST_METHOD(OverstrikeAtBottomOfBuffer); + TEST_METHOD(MarginsWithStatusLine); + TEST_METHOD(OutputWrappedLineWithSpace); + TEST_METHOD(OutputWrappedLineWithSpaceAtBottomOfBuffer); + TEST_METHOD(ScrollWithMargins); private: @@ -950,7 +964,7 @@ void ConptyRoundtripTests::PassthroughClearScrollback() else { // After we hit the bottom of the viewport, the newlines come in - // separated for whatever reason. + // separated by empty writes for whatever reason. expectedOutput.push_back("\r"); expectedOutput.push_back("\n"); expectedOutput.push_back(""); @@ -1026,8 +1040,7 @@ void ConptyRoundtripTests::PassthroughHardReset() else { // After we hit the bottom of the viewport, the newlines come in - // separated for whatever reason. - + // separated by empty writes for whatever reason. expectedOutput.push_back("\r"); expectedOutput.push_back("\n"); expectedOutput.push_back(""); @@ -1061,6 +1074,293 @@ void ConptyRoundtripTests::PassthroughHardReset() } } +void ConptyRoundtripTests::OutputWrappedLinesAtTopOfBuffer() +{ + Log::Comment( + L"Case 1: Write a wrapped line right at the start of the buffer, before any circling"); + VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& sm = si.GetStateMachine(); + auto& hostTb = si.GetTextBuffer(); + auto& termTb = *term->_buffer; + + _flushFirstFrame(); + + const auto wrappedLineLength = TerminalViewWidth + 20; + + sm.ProcessString(std::wstring(wrappedLineLength, L'A')); + + auto verifyBuffer = [](const TextBuffer& tb) { + // Buffer contents should look like the following: (80 wide) + // (w) means we hard wrapped the line + // (b) means the line is _not_ wrapped (it's broken, the default state.) + // cursor is on the '_' + // + // |AAAAAAAA...AAAA| (w) + // |AAAAA_ ... | (b) (There are 20 'A's on this line.) + // | ... | (b) + + VERIFY_IS_TRUE(tb.GetRowByOffset(0).GetCharRow().WasWrapForced()); + VERIFY_IS_FALSE(tb.GetRowByOffset(1).GetCharRow().WasWrapForced()); + auto iter0 = tb.GetCellDataAt({ 0, 0 }); + TestUtils::VerifySpanOfText(L"A", iter0, 0, TerminalViewWidth); + auto iter1 = tb.GetCellDataAt({ 0, 1 }); + TestUtils::VerifySpanOfText(L"A", iter1, 0, 20); + auto iter2 = tb.GetCellDataAt({ 20, 1 }); + TestUtils::VerifySpanOfText(L" ", iter2, 0, TerminalViewWidth - 20); + }; + + verifyBuffer(hostTb); + + expectedOutput.push_back(std::string(TerminalViewWidth, 'A')); + expectedOutput.push_back(std::string(20, 'A')); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + verifyBuffer(termTb); +} + +void ConptyRoundtripTests::OutputWrappedLinesAtBottomOfBuffer() +{ + Log::Comment( + L"Case 2: Write a wrapped line at the end of the buffer, once the conpty started circling"); + VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& hostSm = si.GetStateMachine(); + auto& hostTb = si.GetTextBuffer(); + auto& termTb = *term->_buffer; + + _flushFirstFrame(); + + // First, fill the buffer with contents, so conpty starts circling + + const auto hostView = si.GetViewport(); + const auto end = 2 * hostView.Height(); + for (auto i = 0; i < end; i++) + { + Log::Comment(NoThrowString().Format(L"Writing line %d/%d", i, end)); + expectedOutput.push_back("X"); + if (i < hostView.BottomInclusive()) + { + expectedOutput.push_back("\r\n"); + } + else + { + // After we hit the bottom of the viewport, the newlines come in + // separated by empty writes for whatever reason. + expectedOutput.push_back("\r"); + expectedOutput.push_back("\n"); + expectedOutput.push_back(""); + } + + hostSm.ProcessString(L"X\n"); + + VERIFY_SUCCEEDED(renderer.PaintFrame()); + } + + const auto wrappedLineLength = TerminalViewWidth + 20; + + // The following diagrams show the buffer contents after each string emitted + // from conpty. For each of these diagrams: + // (w) means we hard wrapped the line + // (b) means the line is _not_ wrapped (it's broken, the default state.) + // cursor is on the '_' + + // Initial state: + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |_ | (b) + + expectedOutput.push_back(std::string(TerminalViewWidth, 'A')); + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AAAA|_ (w) The cursor is actually on the last A here + + // TODO GH#5228 might break the "newline & repaint the wrapped char" checks here, that's okay. + expectedOutput.push_back("\r"); // This \r\n is emitted by ScrollFrame to + expectedOutput.push_back("\n"); // add a newline to the bottom of the buffer + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AAAA| (b) + // |_ | (b) + + expectedOutput.push_back("\x1b[31;80H"); // Move the cursor BACK to the wrapped row + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AAAA| (b) The cursor is actually on the last A here + // | | (b) + + expectedOutput.push_back(std::string(1, 'A')); // Reprint the last character of the wrapped row + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AAAA|_ (w) The cursor is actually on the last A here + // | | (b) + + expectedOutput.push_back(std::string(20, 'A')); // Print the second line. + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AAAA| (w) + // |AAAAA_ | (b) There are 20 'A's on this line. + + hostSm.ProcessString(std::wstring(wrappedLineLength, L'A')); + + auto verifyBuffer = [](const TextBuffer& tb, const short wrappedRow) { + // Buffer contents should look like the following: (80 wide) + // (w) means we hard wrapped the line + // (b) means the line is _not_ wrapped (it's broken, the default state.) + // cursor is on the '_' + // + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AAAA| (w) + // |AAAAA_ ... | (b) (There are 20 'A's on this line.) + + VERIFY_IS_TRUE(tb.GetRowByOffset(wrappedRow).GetCharRow().WasWrapForced()); + VERIFY_IS_FALSE(tb.GetRowByOffset(wrappedRow + 1).GetCharRow().WasWrapForced()); + + auto iter0 = tb.GetCellDataAt({ 0, wrappedRow }); + TestUtils::VerifySpanOfText(L"A", iter0, 0, TerminalViewWidth); + auto iter1 = tb.GetCellDataAt({ 0, wrappedRow + 1 }); + TestUtils::VerifySpanOfText(L"A", iter1, 0, 20); + auto iter2 = tb.GetCellDataAt({ 20, wrappedRow + 1 }); + TestUtils::VerifySpanOfText(L" ", iter2, 0, TerminalViewWidth - 20); + }; + + verifyBuffer(hostTb, hostView.BottomInclusive() - 1); + + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + verifyBuffer(termTb, term->_mutableViewport.BottomInclusive() - 1); +} + +void ConptyRoundtripTests::ScrollWithChangesInMiddle() +{ + Log::Comment(L"This test checks emitting a wrapped line at the bottom of the" + L" viewport while _also_ emitting other text elsewhere in the same frame. This" + L" output will cause us to scroll the viewport in one frame, but we need to" + L" make sure the wrapped line _stays_ wrapped, and the scrolled text appears in" + L" the right place."); + VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& hostSm = si.GetStateMachine(); + auto& hostTb = si.GetTextBuffer(); + auto& termTb = *term->_buffer; + + _flushFirstFrame(); + + // First, fill the buffer with contents, so conpty starts circling + + const auto hostView = si.GetViewport(); + const auto end = 2 * hostView.Height(); + for (auto i = 0; i < end; i++) + { + Log::Comment(NoThrowString().Format(L"Writing line %d/%d", i, end)); + expectedOutput.push_back("X"); + if (i < hostView.BottomInclusive()) + { + expectedOutput.push_back("\r\n"); + } + else + { + // After we hit the bottom of the viewport, the newlines come in + // separated by empty writes for whatever reason. + expectedOutput.push_back("\r"); + expectedOutput.push_back("\n"); + expectedOutput.push_back(""); + } + + hostSm.ProcessString(L"X\n"); + + VERIFY_SUCCEEDED(renderer.PaintFrame()); + } + + const auto wrappedLineLength = TerminalViewWidth + 20; + + // In the Terminal, we're going to expect: + expectedOutput.push_back("\x1b[15;1H"); // Move the cursor to row 14, col 0 + expectedOutput.push_back("Y"); // Print a 'Y' + expectedOutput.push_back("\x1b[32;1H"); // Move the cursor to the last row + expectedOutput.push_back(std::string(TerminalViewWidth, 'A')); // Print the first 80 'A's + // This is going to be the end of the first frame - b/c we moved the cursor + // in the middle of the frame, we're going to hide/show the cursor during + // this frame + expectedOutput.push_back("\x1b[?25h"); // hide the cursor + // On the subsequent frame: + // TODO GH#5228 might break the "newline & repaint the wrapped char" checks here, that's okay. + expectedOutput.push_back("\r"); // This \r\n is emitted by ScrollFrame to + expectedOutput.push_back("\n"); // add a newline to the bottom of the buffer + expectedOutput.push_back("\x1b[31;80H"); // Move the cursor BACK to the wrapped row + expectedOutput.push_back(std::string(1, 'A')); // Reprint the last character of the wrapped row + expectedOutput.push_back(std::string(20, 'A')); // Print the second line. + + _logConpty = true; + + // To the host, we'll do something very similar: + hostSm.ProcessString(L"\x1b" + L"7"); // Save cursor + hostSm.ProcessString(L"\x1b[15;1H"); // Move the cursor to row 14, col 0 + hostSm.ProcessString(L"Y"); // Print a 'Y' + hostSm.ProcessString(L"\x1b" + L"8"); // Restore + hostSm.ProcessString(std::wstring(wrappedLineLength, L'A')); // Print 100 'A's + + auto verifyBuffer = [](const TextBuffer& tb, const til::rectangle viewport) { + const short wrappedRow = viewport.bottom() - 2; + const short start = viewport.top(); + for (short i = start; i < wrappedRow; i++) + { + Log::Comment(NoThrowString().Format(L"Checking row %d", i)); + TestUtils::VerifyExpectedString(tb, i == start + 13 ? L"Y" : L"X", { 0, i }); + } + + VERIFY_IS_TRUE(tb.GetRowByOffset(wrappedRow).GetCharRow().WasWrapForced()); + VERIFY_IS_FALSE(tb.GetRowByOffset(wrappedRow + 1).GetCharRow().WasWrapForced()); + + auto iter0 = tb.GetCellDataAt({ 0, wrappedRow }); + TestUtils::VerifySpanOfText(L"A", iter0, 0, TerminalViewWidth); + + auto iter1 = tb.GetCellDataAt({ 0, wrappedRow + 1 }); + TestUtils::VerifySpanOfText(L"A", iter1, 0, 20); + auto iter2 = tb.GetCellDataAt({ 20, wrappedRow + 1 }); + TestUtils::VerifySpanOfText(L" ", iter2, 0, TerminalViewWidth - 20); + }; + + Log::Comment(NoThrowString().Format(L"Checking the host buffer...")); + verifyBuffer(hostTb, hostView.ToInclusive()); + Log::Comment(NoThrowString().Format(L"... Done")); + + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(NoThrowString().Format(L"Checking the terminal buffer...")); + verifyBuffer(termTb, term->_mutableViewport.ToInclusive()); + Log::Comment(NoThrowString().Format(L"... Done")); +} + void ConptyRoundtripTests::ScrollWithMargins() { auto& g = ServiceLocator::LocateGlobals(); @@ -1087,6 +1387,7 @@ void ConptyRoundtripTests::ScrollWithMargins() // The letters represent the data in the TMUX pane. // The final *** line represents the mode line which we will // attempt to hold in place and not scroll. + // Note that the last line will contain one '*' less than the width of the window. Log::Comment(L"Fill host with text pattern by feeding it into VT parser."); const auto rowsToWrite = initialTermView.Height() - 1; @@ -1103,7 +1404,7 @@ void ConptyRoundtripTests::ScrollWithMargins() } // For the last one, write out the asterisks for the mode line. - for (auto i = 0; i < initialTermView.Width(); ++i) + for (auto i = 0; i < initialTermView.Width() - 1; ++i) { hostSm.ProcessCharacter('*'); } @@ -1127,7 +1428,7 @@ void ConptyRoundtripTests::ScrollWithMargins() } // For the last row, verify we have an entire row of asterisks for the mode line. - const std::wstring expectedModeLine(initialTermView.Width(), L'*'); + const std::wstring expectedModeLine(initialTermView.Width() - 1, L'*'); const COORD expectedPos{ 0, gsl::narrow(rowsToWrite) }; TestUtils::VerifyExpectedString(tb, expectedModeLine, expectedPos); }; @@ -1140,13 +1441,8 @@ void ConptyRoundtripTests::ScrollWithMargins() expectedOutput.push_back("\r\n"); } { - const std::string expectedString(initialTermView.Width(), '*'); + const std::string expectedString(initialTermView.Width() - 1, '*'); expectedOutput.push_back(expectedString); - - // Cursor gets reset into bottom right corner as we're writing all the way into that corner. - std::stringstream ss; - ss << "\x1b[" << initialTermView.Height() << ";" << initialTermView.Width() << "H"; - expectedOutput.push_back(ss.str()); } Log::Comment(L"Verify host buffer contains pattern."); @@ -1257,7 +1553,7 @@ void ConptyRoundtripTests::ScrollWithMargins() // For the last row, verify we have an entire row of asterisks for the mode line. { - const std::wstring expectedModeLine(initialTermView.Width(), L'*'); + const std::wstring expectedModeLine(initialTermView.Width() - 1, L'*'); const COORD modeLinePos{ 0, gsl::narrow(rowsToWrite) }; TestUtils::VerifyExpectedString(tb, expectedModeLine, modeLinePos); } @@ -1279,8 +1575,9 @@ void ConptyRoundtripTests::ScrollWithMargins() expectedOutput.push_back("\r\n"); } { - const std::string expectedString(initialTermView.Width(), '*'); - expectedOutput.push_back(expectedString); + const std::string expectedString(initialTermView.Width() - 1, '*'); + // There will be one extra blank space at the end of the line, to prevent delayed EOL wrapping + expectedOutput.push_back(expectedString + " "); } { // Cursor gets reset into second line from bottom, left most column @@ -1302,3 +1599,590 @@ void ConptyRoundtripTests::ScrollWithMargins() // Verify the terminal side. verifyBufferAfter(termTb); } + +void ConptyRoundtripTests::DontWrapMoveCursorInSingleFrame() +{ + // See https://github.com/microsoft/terminal/pull/5181#issuecomment-607427840 + Log::Comment(L"This is a test for when a line of text exactly wrapped, but " + L"the cursor didn't end the frame at the end of line (waiting " + L"for more wrapped text). We should still move the cursor in " + L"this case."); + VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& hostSm = si.GetStateMachine(); + auto& hostTb = si.GetTextBuffer(); + auto& termTb = *term->_buffer; + + _flushFirstFrame(); + + auto verifyBuffer = [](const TextBuffer& tb) { + // Simple verification: Make sure the cursor is in the correct place, + // and that it's visible. We don't care so much about the buffer + // contents in this test. + const COORD expectedCursor{ 8, 3 }; + VERIFY_ARE_EQUAL(expectedCursor, tb.GetCursor().GetPosition()); + VERIFY_IS_TRUE(tb.GetCursor().IsVisible()); + }; + + hostSm.ProcessString(L"\x1b[?25l"); + hostSm.ProcessString(L"\x1b[H"); + hostSm.ProcessString(L"\x1b[75C"); + hostSm.ProcessString(L"XXXXX"); + hostSm.ProcessString(L"\x1b[4;9H"); + hostSm.ProcessString(L"\x1b[?25h"); + + Log::Comment(L"Checking the host buffer state"); + verifyBuffer(hostTb); + + expectedOutput.push_back("\x1b[75C"); + expectedOutput.push_back("XXXXX"); + expectedOutput.push_back("\x1b[4;9H"); + // We're _not_ expecting a cursor on here, because we didn't actually hide + // the cursor during the course of this frame + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"Checking the terminal buffer state"); + verifyBuffer(termTb); +} + +void ConptyRoundtripTests::ClearHostTrickeryTest() +{ + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:paintEachNewline", L"{0, 1, 2}") + TEST_METHOD_PROPERTY(L"Data:cursorOnNextLine", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:paintAfterDECALN", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:changeAttributes", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:useLongSpaces", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:printTextAfterSpaces", L"{false, true}") + END_TEST_METHOD_PROPERTIES(); + constexpr int PaintEveryNewline = 0; + constexpr int PaintAfterAllNewlines = 1; + constexpr int DontPaintAfterNewlines = 2; + + INIT_TEST_PROPERTY(int, paintEachNewline, L"Any of: manually PaintFrame after each newline is emitted, once at the end of all newlines, or not at all"); + INIT_TEST_PROPERTY(bool, cursorOnNextLine, L"Either leave the cursor on the first line, or place it on the second line of the buffer"); + INIT_TEST_PROPERTY(bool, paintAfterDECALN, L"Controls whether we manually paint a frame after the DECALN sequence is emitted."); + INIT_TEST_PROPERTY(bool, changeAttributes, L"If true, change the text attributes after the 'A's and spaces"); + INIT_TEST_PROPERTY(bool, useLongSpaces, L"If true, print 10 spaces instead of 5, longer than a CUF sequence."); + INIT_TEST_PROPERTY(bool, printTextAfterSpaces, L"If true, print \"ZZZZZ\" after the spaces on the first line."); + + // See https://github.com/microsoft/terminal/issues/5039#issuecomment-606833841 + Log::Comment(L"This is a more than comprehensive test for GH#5039. We're " + L"going to print some text to the buffer, then fill the alt-" + L"buffer with text, then switch back to the main buffer. The " + L"text from the alt buffer should not pollute the main buffer."); + + // The text we're printing will look like one of the following, with the + // cursor on the _ + // * cursorOnNextLine=false, useLongSpaces=false: + // AAAAA ZZZZZ_ + // * cursorOnNextLine=false, useLongSpaces=true: + // AAAAA ZZZZZ_ + // * cursorOnNextLine=true, useLongSpaces=false: + // AAAAA ZZZZZ + // BBBBB_ + // * cursorOnNextLine=true, useLongSpaces=true: + // AAAAA ZZZZZ + // BBBBB_ + // + // If printTextAfterSpaces=false, then we won't print the "ZZZZZ" + // + // The interesting case that repros the bug in GH#5039 is + // - paintEachNewline=DontPaintAfterNewlines (2) + // - cursorOnNextLine=false + // - paintAfterDECALN= + // - changeAttributes=true + // - useLongSpaces= + // - printTextAfterSpaces= + // + // All the possible cases are left here though, to catch potential future regressions. + VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& hostSm = si.GetStateMachine(); + auto& hostTb = si.GetTextBuffer(); + auto& termTb = *term->_buffer; + + _flushFirstFrame(); + + auto verifyBuffer = [&cursorOnNextLine, &useLongSpaces, &printTextAfterSpaces](const TextBuffer& tb, + const til::rectangle viewport) { + // We _would_ expect the Terminal's cursor to be on { 8, 0 }, but this + // is currently broken due to #381/#4676. So we'll use the viewport + // provided to find the actual Y position of the cursor. + const short viewTop = viewport.origin().y(); + const short cursorRow = viewTop + (cursorOnNextLine ? 1 : 0); + const short cursorCol = (cursorOnNextLine ? 5 : + (10 + (useLongSpaces ? 5 : 0) + (printTextAfterSpaces ? 5 : 0))); + const COORD expectedCursor{ cursorCol, cursorRow }; + + VERIFY_ARE_EQUAL(expectedCursor, tb.GetCursor().GetPosition()); + VERIFY_IS_TRUE(tb.GetCursor().IsVisible()); + auto iter = TestUtils::VerifyExpectedString(tb, L"AAAAA", { 0, viewTop }); + TestUtils::VerifyExpectedString(useLongSpaces ? L" " : L" ", iter); + if (printTextAfterSpaces) + { + TestUtils::VerifyExpectedString(L"ZZZZZ", iter); + } + else + { + TestUtils::VerifyExpectedString(L" ", iter); + } + TestUtils::VerifyExpectedString(L" ", iter); + + if (cursorOnNextLine) + { + TestUtils::VerifyExpectedString(tb, L"BBBBB", { 0, cursorRow }); + } + }; + + // We're _not_ checking the conpty output during this test, only the side effects. + _checkConptyOutput = false; + + gci.LockConsole(); // Lock must be taken to manipulate alt/main buffer state. + auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); }); + + Log::Comment(L"Setting up the host buffer..."); + hostSm.ProcessString(L"AAAAA"); + hostSm.ProcessString(useLongSpaces ? L" " : L" "); + if (changeAttributes) + { + hostSm.ProcessString(L"\x1b[44m"); + } + if (printTextAfterSpaces) + { + hostSm.ProcessString(L"ZZZZZ"); + } + hostSm.ProcessString(L"\x1b[0m"); + + if (cursorOnNextLine) + { + hostSm.ProcessString(L"\n"); + hostSm.ProcessString(L"BBBBB"); + } + Log::Comment(L"Painting after the initial setup."); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"Switching to the alt buffer and using DECALN to fill it with 'E's"); + hostSm.ProcessString(L"\x1b[?1049h"); + hostSm.ProcessString(L"\x1b#8"); + if (paintAfterDECALN) + { + VERIFY_SUCCEEDED(renderer.PaintFrame()); + } + + for (auto i = 0; i < si.GetViewport().Height(); i++) + { + hostSm.ProcessString(L"\n"); + if (paintEachNewline == PaintEveryNewline) + { + VERIFY_SUCCEEDED(renderer.PaintFrame()); + } + } + if (paintEachNewline == PaintAfterAllNewlines) + { + VERIFY_SUCCEEDED(renderer.PaintFrame()); + } + Log::Comment(L"Returning to the main buffer."); + hostSm.ProcessString(L"\x1b[?1049l"); + + Log::Comment(L"Checking the host buffer state"); + verifyBuffer(hostTb, si.GetViewport().ToInclusive()); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"Checking the terminal buffer state"); + verifyBuffer(termTb, term->_mutableViewport.ToInclusive()); +} + +void ConptyRoundtripTests::OverstrikeAtBottomOfBuffer() +{ + // See https://github.com/microsoft/terminal/pull/5181#issuecomment-607545241 + Log::Comment(L"This test replicates the zsh menu-complete functionality. In" + L" the course of a single frame, we're going to both scroll " + L"the frame and print multiple lines of text above the bottom line."); + VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& hostSm = si.GetStateMachine(); + auto& hostTb = si.GetTextBuffer(); + auto& termTb = *term->_buffer; + + _flushFirstFrame(); + + auto verifyBuffer = [](const TextBuffer& tb, + const til::rectangle viewport) { + const auto lastRow = viewport.bottom() - 1; + const til::point expectedCursor{ 0, lastRow - 1 }; + VERIFY_ARE_EQUAL(expectedCursor, til::point{ tb.GetCursor().GetPosition() }); + VERIFY_IS_TRUE(tb.GetCursor().IsVisible()); + + TestUtils::VerifyExpectedString(tb, L"AAAAAAAAAA DDDDDDDDDD", til::point{ 0, lastRow - 2 }); + TestUtils::VerifyExpectedString(tb, L"BBBBBBBBBB", til::point{ 0, lastRow - 1 }); + TestUtils::VerifyExpectedString(tb, L"FFFFFFFFFE", til::point{ 0, lastRow }); + }; + + _logConpty = true; + // We're _not_ checking the conpty output during this test, only the side effects. + _checkConptyOutput = false; + + hostSm.ProcessString(L"\x1b#8"); + + hostSm.ProcessString(L"\x1b[32;1H"); + + hostSm.ProcessString(L"\x1b[J"); + hostSm.ProcessString(L"AAAAAAAAAA"); + hostSm.ProcessString(L"\x1b[K"); + hostSm.ProcessString(L"\r"); + hostSm.ProcessString(L"\n"); + hostSm.ProcessString(L"BBBBBBBBBB"); + hostSm.ProcessString(L"\x1b[K"); + hostSm.ProcessString(L"\n"); + hostSm.ProcessString(L"CCCCCCCCCC"); + hostSm.ProcessString(L"\x1b[2A"); + hostSm.ProcessString(L"\r"); + hostSm.ProcessString(L"\x1b[20C"); + hostSm.ProcessString(L"DDDDDDDDDD"); + hostSm.ProcessString(L"\x1b[K"); + hostSm.ProcessString(L"\r"); + hostSm.ProcessString(L"\n"); + hostSm.ProcessString(L"\x1b[1B"); + hostSm.ProcessString(L"EEEEEEEEEE"); + hostSm.ProcessString(L"\r"); + hostSm.ProcessString(L"FFFFFFFFF"); + hostSm.ProcessString(L"\r"); + hostSm.ProcessString(L"\x1b[A"); + hostSm.ProcessString(L"\x1b[A"); + hostSm.ProcessString(L"\n"); + + Log::Comment(L"========== Checking the host buffer state =========="); + verifyBuffer(hostTb, si.GetViewport().ToInclusive()); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Checking the terminal buffer state =========="); + + verifyBuffer(termTb, term->_mutableViewport.ToInclusive()); +} + +void ConptyRoundtripTests::MarginsWithStatusLine() +{ + // See https://github.com/microsoft/terminal/issues/5161 + // + // This test reproduces a case from the MSYS/cygwin (runtime < 3.1) vim. + // From what I can tell, they implement scrolling by emitting a newline at + // the bottom of the buffer (to create a new blank line), then they use + // ScrollConsoleScreenBuffer to shift the status line(s) down a line, and + // then they re-printing the status line. + Log::Comment(L"Newline, and scroll the bottom lines of the buffer down with" + L" ScrollConsoleScreenBuffer to emulate how cygwin VIM works"); + VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& hostSm = si.GetStateMachine(); + auto& hostTb = si.GetTextBuffer(); + auto& termTb = *term->_buffer; + + _flushFirstFrame(); + + auto verifyBuffer = [](const TextBuffer& tb, + const til::rectangle viewport) { + const auto lastRow = viewport.bottom() - 1; + const til::point expectedCursor{ 1, lastRow }; + VERIFY_ARE_EQUAL(expectedCursor, til::point{ tb.GetCursor().GetPosition() }); + VERIFY_IS_TRUE(tb.GetCursor().IsVisible()); + + TestUtils::VerifyExpectedString(tb, L"EEEEEEEEEE", til::point{ 0, lastRow - 4 }); + TestUtils::VerifyExpectedString(tb, L"AAAAAAAAAA", til::point{ 0, lastRow - 3 }); + TestUtils::VerifyExpectedString(tb, L" ", til::point{ 0, lastRow - 2 }); + TestUtils::VerifyExpectedString(tb, L"XBBBBBBBBB", til::point{ 0, lastRow - 1 }); + TestUtils::VerifyExpectedString(tb, L"YCCCCCCCCC", til::point{ 0, lastRow }); + }; + + // We're _not_ checking the conpty output during this test, only the side effects. + _checkConptyOutput = false; + + // Use DECALN to fill the buffer with 'E's. + hostSm.ProcessString(L"\x1b#8"); + + const short originalBottom = si.GetViewport().BottomInclusive(); + // Print 3 lines into the bottom of the buffer: + // AAAAAAAAAA + // BBBBBBBBBB + // CCCCCCCCCC + // In this test, the 'B' and 'C' lines represent the status lines at the + // bottom of vim, and the 'A' line is a buffer line. + hostSm.ProcessString(L"\x1b[30;1H"); + + hostSm.ProcessString(L"AAAAAAAAAA"); + hostSm.ProcessString(L"\n"); + hostSm.ProcessString(L"BBBBBBBBBB"); + hostSm.ProcessString(L"\n"); + hostSm.ProcessString(L"CCCCCCCCCC"); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + // After printing the 'C' line, the cursor is on the bottom line of the viewport. + // Emit a newline here to get a new line at the bottom of the viewport. + hostSm.ProcessString(L"\n"); + const short newBottom = si.GetViewport().BottomInclusive(); + + { + // Emulate calling ScrollConsoleScreenBuffer to scroll the B and C lines + // down one line. + SMALL_RECT src; + src.Top = newBottom - 2; + src.Left = 0; + src.Right = si.GetViewport().Width(); + src.Bottom = originalBottom; + COORD tgt = { 0, newBottom - 1 }; + TextAttribute useThisAttr(0x07); // We don't terribly care about the attributes so this is arbitrary + ScrollRegion(si, src, std::nullopt, tgt, L' ', useThisAttr); + } + + // Move the cursor to the location of the B line + hostSm.ProcessString(L"\x1b[31;1H"); + + // Print an 'X' on the 'B' line, and a 'Y' on the 'C' line. + hostSm.ProcessString(L"X"); + hostSm.ProcessString(L"\n"); + hostSm.ProcessString(L"Y"); + + Log::Comment(L"========== Checking the host buffer state =========="); + verifyBuffer(hostTb, si.GetViewport().ToInclusive()); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Checking the terminal buffer state =========="); + + verifyBuffer(termTb, term->_mutableViewport.ToInclusive()); +} + +void ConptyRoundtripTests::OutputWrappedLineWithSpace() +{ + // See https://github.com/microsoft/terminal/pull/5181#issuecomment-610110348 + Log::Comment(L"Ensures that a buffer line in conhost that wrapped _on a " + L"space_ will still be emitted as wrapped."); + VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& sm = si.GetStateMachine(); + auto& hostTb = si.GetTextBuffer(); + auto& termTb = *term->_buffer; + + _flushFirstFrame(); + + const auto firstTextLength = TerminalViewWidth - 2; + const auto spacesLength = 3; + const auto secondTextLength = 1; + + sm.ProcessString(std::wstring(firstTextLength, L'A')); + sm.ProcessString(std::wstring(spacesLength, L' ')); + sm.ProcessString(std::wstring(secondTextLength, L'B')); + + auto verifyBuffer = [&](const TextBuffer& tb) { + // Buffer contents should look like the following: (80 wide) + // (w) means we hard wrapped the line + // (b) means the line is _not_ wrapped (it's broken, the default state.) + // + // |AAAA...AA | (w) + // | B_ ... | (b) (cursor is on the '_') + // | ... | (b) + + VERIFY_IS_TRUE(tb.GetRowByOffset(0).GetCharRow().WasWrapForced()); + VERIFY_IS_FALSE(tb.GetRowByOffset(1).GetCharRow().WasWrapForced()); + + // First row + auto iter0 = tb.GetCellDataAt({ 0, 0 }); + TestUtils::VerifySpanOfText(L"A", iter0, 0, firstTextLength); + TestUtils::VerifySpanOfText(L" ", iter0, 0, 2); + + // Second row + auto iter1 = tb.GetCellDataAt({ 0, 1 }); + TestUtils::VerifySpanOfText(L" ", iter1, 0, 1); + auto iter2 = tb.GetCellDataAt({ 1, 1 }); + TestUtils::VerifySpanOfText(L"B", iter2, 0, secondTextLength); + }; + + Log::Comment(L"========== Checking the host buffer state =========="); + verifyBuffer(hostTb); + + std::string firstLine = std::string(firstTextLength, 'A'); + firstLine += " "; + std::string secondLine{ " B" }; + + expectedOutput.push_back(firstLine); + expectedOutput.push_back(secondLine); + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Checking the terminal buffer state =========="); + verifyBuffer(termTb); +} + +void ConptyRoundtripTests::OutputWrappedLineWithSpaceAtBottomOfBuffer() +{ + // See https://github.com/microsoft/terminal/pull/5181#issuecomment-610110348 + // This is the same test as OutputWrappedLineWithSpace, but at the bottom of + // the buffer, so we get scrolling behavior as well. + Log::Comment(L"Ensures that a buffer line in conhost that wrapped _on a " + L"space_ will still be emitted as wrapped."); + VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& sm = si.GetStateMachine(); + auto& hostTb = si.GetTextBuffer(); + auto& termTb = *term->_buffer; + + _flushFirstFrame(); + + // First, fill the buffer with contents, so conpty starts circling + const auto hostView = si.GetViewport(); + const auto end = 2 * hostView.Height(); + for (auto i = 0; i < end; i++) + { + Log::Comment(NoThrowString().Format(L"Writing line %d/%d", i, end)); + expectedOutput.push_back("X"); + if (i < hostView.BottomInclusive()) + { + expectedOutput.push_back("\r\n"); + } + else + { + // After we hit the bottom of the viewport, the newlines come in + // separated by empty writes for whatever reason. + expectedOutput.push_back("\r"); + expectedOutput.push_back("\n"); + expectedOutput.push_back(""); + } + + sm.ProcessString(L"X\n"); + + VERIFY_SUCCEEDED(renderer.PaintFrame()); + } + + const auto firstTextLength = TerminalViewWidth - 2; + const auto spacesLength = 3; + const auto secondTextLength = 1; + + std::string firstLine = std::string(firstTextLength, 'A'); + firstLine += " "; + std::string secondLine{ " B" }; + + // The following diagrams show the buffer contents after each string emitted + // from conpty. For each of these diagrams: + // (w) means we hard wrapped the line + // (b) means the line is _not_ wrapped (it's broken, the default state.) + // cursor is on the '_' + + // Initial state: + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |_ | (b) + + expectedOutput.push_back(firstLine); + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AA _| (w) The cursor is actually on the last ' ' here + + // TODO GH#5228 might break the "newline & repaint the wrapped char" checks here, that's okay. + expectedOutput.push_back("\r"); // This \r\n is emitted by ScrollFrame to + expectedOutput.push_back("\n"); // add a newline to the bottom of the buffer + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AA | (b) + // |_ | (b) + + expectedOutput.push_back("\x1b[31;80H"); // Move the cursor BACK to the wrapped row + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AA _| (b) The cursor is actually on the last ' ' here + // | | (b) + + expectedOutput.push_back(std::string(1, ' ')); // Reprint the last character of the wrapped row + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AA |_ (w) The cursor is actually on the last ' ' here + // | | (b) + + expectedOutput.push_back(secondLine); + // |X | (b) + // |X | (b) + // ... + // |X | (b) + // |AAAAAAAA...AA | (w) + // | B_ | (b) + + sm.ProcessString(std::wstring(firstTextLength, L'A')); + sm.ProcessString(std::wstring(spacesLength, L' ')); + sm.ProcessString(std::wstring(secondTextLength, L'B')); + + auto verifyBuffer = [&](const TextBuffer& tb, const til::rectangle viewport) { + // Buffer contents should look like the following: (80 wide) + // (w) means we hard wrapped the line + // (b) means the line is _not_ wrapped (it's broken, the default state.) + // + // |AAAA...AA | (w) + // | B_ ... | (b) (cursor is on the '_') + // | ... | (b) + + const short wrappedRow = viewport.bottom() - 2; + VERIFY_IS_TRUE(tb.GetRowByOffset(wrappedRow).GetCharRow().WasWrapForced()); + VERIFY_IS_FALSE(tb.GetRowByOffset(wrappedRow + 1).GetCharRow().WasWrapForced()); + + // First row + auto iter0 = tb.GetCellDataAt({ 0, wrappedRow }); + TestUtils::VerifySpanOfText(L"A", iter0, 0, firstTextLength); + TestUtils::VerifySpanOfText(L" ", iter0, 0, 2); + + // Second row + auto iter1 = tb.GetCellDataAt({ 0, wrappedRow + 1 }); + TestUtils::VerifySpanOfText(L" ", iter1, 0, 1); + auto iter2 = tb.GetCellDataAt({ 1, wrappedRow + 1 }); + TestUtils::VerifySpanOfText(L"B", iter2, 0, secondTextLength); + }; + + Log::Comment(L"========== Checking the host buffer state =========="); + verifyBuffer(hostTb, hostView.ToInclusive()); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Checking the terminal buffer state =========="); + verifyBuffer(termTb, term->_mutableViewport.ToInclusive()); +} diff --git a/src/host/ut_host/ConptyOutputTests.cpp b/src/host/ut_host/ConptyOutputTests.cpp index ecfd352af4b..6b438ab9d07 100644 --- a/src/host/ut_host/ConptyOutputTests.cpp +++ b/src/host/ut_host/ConptyOutputTests.cpp @@ -27,6 +27,12 @@ using namespace Microsoft::Console::Types; class ConptyOutputTests { + // !!! DANGER: Many tests in this class expect the Terminal and Host buffers + // to be 80x32. If you change these, you'll probably inadvertently break a + // bunch of tests !!! + static const SHORT TerminalViewWidth = 80; + static const SHORT TerminalViewHeight = 32; + // This test class is to write some things into the PTY and then check that // the rendering that is coming out of the VT-sequence generator is exactly // as we expect it to be. @@ -40,7 +46,7 @@ class ConptyOutputTests m_state->InitEvents(); m_state->PrepareGlobalFont(); - m_state->PrepareGlobalScreenBuffer(); + m_state->PrepareGlobalScreenBuffer(TerminalViewWidth, TerminalViewHeight, TerminalViewWidth, TerminalViewHeight); m_state->PrepareGlobalInputBuffer(); return true; @@ -66,7 +72,7 @@ class ConptyOutputTests gci.SetDefaultBackgroundColor(INVALID_COLOR); gci.SetFillAttribute(0x07); // DARK_WHITE on DARK_BLACK - m_state->PrepareNewTextBufferInfo(true); + m_state->PrepareNewTextBufferInfo(true, TerminalViewWidth, TerminalViewHeight); auto& currentBuffer = gci.GetActiveOutputBuffer(); // Make sure a test hasn't left us in the alt buffer on accident VERIFY_IS_FALSE(currentBuffer._IsAltBuffer()); @@ -347,7 +353,6 @@ void ConptyOutputTests::InvalidateUntilOneBeforeEnd() expectedOutput.push_back("\x1b[65C"); expectedOutput.push_back("ABCDEFGHIJKLMNO"); - expectedOutput.push_back("\x1b[1;80H"); // we move the cursor to the end of the line after paint VERIFY_SUCCEEDED(renderer.PaintFrame()); @@ -367,7 +372,5 @@ void ConptyOutputTests::InvalidateUntilOneBeforeEnd() expectedOutput.push_back("\x1b[13X"); expectedOutput.push_back("\x1b[13C"); - expectedOutput.push_back("\x1b[?25h"); // we turn the cursor back on for good measure - VERIFY_SUCCEEDED(renderer.PaintFrame()); } diff --git a/src/inc/consoletaeftemplates.hpp b/src/inc/consoletaeftemplates.hpp index aa169ad4e7a..592b9d09b7d 100644 --- a/src/inc/consoletaeftemplates.hpp +++ b/src/inc/consoletaeftemplates.hpp @@ -17,6 +17,11 @@ Revision History: #pragma once +// Helper for declaring a variable to store a TEST_METHOD_PROPERTY and get it's value from the test metadata +#define INIT_TEST_PROPERTY(type, identifer, description) \ + type identifer; \ + VERIFY_SUCCEEDED(TestData::TryGetValue(L#identifer, identifer), description); + namespace WEX::TestExecution { template<> diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index eaae0ad696e..dba15950b46 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -105,7 +105,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // by prepending a cursor off. if (_lastCursorIsVisible) { - _buffer.insert(0, "\x1b[25l"); + _buffer.insert(0, "\x1b[?25l"); _lastCursorIsVisible = false; } // If the cursor was NOT previously visible, then that's fine! we don't @@ -214,7 +214,35 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // _nextCursorIsVisible will still be false (from when we set it during // StartPaint) _nextCursorIsVisible = true; - return VtEngine::PaintCursor(options); + + // If we did a delayed EOL wrap because we actually wrapped the line here, + // then don't PaintCursor. When we're at the EOL because we've wrapped, our + // internal _lastText thinks the cursor is on the cell just past the right + // of the viewport (ex { 120, 0 }). However, conhost thinks the cursor is + // actually on the last cell of the row. So it'll tell us to paint the + // cursor at { 119, 0 }. If we do that movement, then we'll break line + // wrapping. + // See GH#5113, GH#1245, GH#357 + const auto nextCursorPosition = options.coordCursor; + // Only skip this paint when we think the cursor is in the cell + // immediately off the edge of the terminal, and the actual cursor is in + // the last cell of the row. We're in a deferred wrap, but the host + // thinks the cursor is actually in-frame. + // See ConptyRoundtripTests::DontWrapMoveCursorInSingleFrame + const bool cursorIsInDeferredWrap = (nextCursorPosition.X == _lastText.X - 1) && (nextCursorPosition.Y == _lastText.Y); + // If all three of these conditions are true, then: + // * cursorIsInDeferredWrap: The cursor is in a position where the line + // filled the last cell of the row, but the host tried to paint it in + // the last cell anyways + // * _delayedEolWrap && _wrappedRow.has_value(): We think we've deferred + // the wrap of a line. + // If they're all true, DON'T manually paint the cursor this frame. + if (!(cursorIsInDeferredWrap && _delayedEolWrap && _wrappedRow.has_value())) + { + return VtEngine::PaintCursor(options); + } + + return S_OK; } // Routine Description: @@ -232,9 +260,9 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, [[nodiscard]] HRESULT XtermEngine::_MoveCursor(COORD const coord) noexcept { HRESULT hr = S_OK; - + const auto originalPos = _lastText; _trace.TraceMoveCursor(_lastText, coord); - + bool performedSoftWrap = false; if (coord.X != _lastText.X || coord.Y != _lastText.Y) { if (coord.X == 0 && coord.Y == 0) @@ -260,6 +288,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, if (previousLineWrapped) { + performedSoftWrap = true; _trace.TraceWrapped(); hr = S_OK; } @@ -318,10 +347,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, _lastText = coord; } } - if (_lastText.Y != _lastViewport.ToOrigin().BottomInclusive()) - { - _newBottomLine = false; - } + _deferredCursorPos = INVALID_COORDS; _wrappedRow = std::nullopt; @@ -345,6 +371,8 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, [[nodiscard]] HRESULT XtermEngine::ScrollFrame() noexcept try { + _trace.TraceScrollFrame(_scrollDelta); + if (_scrollDelta.x() != 0) { // No easy way to shift left-right. Everything needs repainting. @@ -356,41 +384,92 @@ try return S_OK; } - const short dy = _scrollDelta.y(); + const short dy = _scrollDelta.y(); const short absDy = static_cast(abs(dy)); - HRESULT hr = S_OK; + // Save the old wrap state here. We're going to clear it so that + // _MoveCursor will definitely move us to the right position. We'll + // restore the state afterwards. + const auto oldWrappedRow = _wrappedRow; + const auto oldDelayedEolWrap = _delayedEolWrap; + _delayedEolWrap = false; + _wrappedRow = std::nullopt; + if (dy < 0) { - // Instead of deleting the first line (causing everything to move up) - // move to the bottom of the buffer, and newline. - // That will cause everything to move up, by moving the viewport down. - // This will let remote conhosts scroll up to see history like normal. - const short bottom = _lastViewport.ToOrigin().BottomInclusive(); - hr = _MoveCursor({ 0, bottom }); - if (SUCCEEDED(hr)) - { - std::string seq = std::string(absDy, '\n'); - hr = _Write(seq); - // Mark that the bottom line is new, so we won't spend time with an - // ECH on it. - _newBottomLine = true; - } - // We don't need to _MoveCursor the cursor again, because it's still - // at the bottom of the viewport. + // TODO GH#5228 - We could optimize this by only doing this newline work + // when there's more invalid than just the bottom line. If only the + // bottom line is invalid, then the next thing the Renderer is going to + // tell us to do is print the new line at the bottom of the viewport, + // and _MoveCursor will automatically give us the newline we want. + // When that's implemented, we'll probably want to make sure to add a + // _lastText.Y += dy; + // statement here. + + // Move the cursor to the bottom of the current viewport + const short bottom = _lastViewport.BottomInclusive(); + RETURN_IF_FAILED(_MoveCursor({ 0, bottom })); + // Emit some number of newlines to create space in the buffer. + RETURN_IF_FAILED(_Write(std::string(absDy, '\n'))); } else if (dy > 0) { - // Move to the top of the buffer, and insert some lines of text, to - // cause the viewport contents to shift down. - hr = _MoveCursor({ 0, 0 }); - if (SUCCEEDED(hr)) - { - hr = _InsertLine(absDy); - } + // If we've scrolled _down_, then move the cursor to the top of the + // buffer, and insert some newlines using the InsertLines VT sequence + RETURN_IF_FAILED(_MoveCursor({ 0, 0 })); + RETURN_IF_FAILED(_InsertLine(absDy)); } - return hr; + // Restore our wrap state. + _wrappedRow = oldWrappedRow; + _delayedEolWrap = oldDelayedEolWrap; + + // Shift our internal tracker of the last text position according to how + // much we've scrolled. If we manually scroll the buffer right now, by + // moving the cursor to the bottom row of the viewport and emitting a + // newline, we'll cause any wrapped lines to get broken. + // + // Instead, we'll just update our internal tracker of where the buffer + // contents are. On this frame, we'll then still move the cursor correctly + // relative to the new frame contents. To do this, we'll shift our + // coordinates we're tracking, like the row that we wrapped on and the + // position we think we left the cursor. + // + // See GH#5113 + _trace.TraceLastText(_lastText); + if (_wrappedRow.has_value()) + { + _wrappedRow.value() += dy; + _trace.TraceSetWrapped(_wrappedRow.value()); + } + + if (_delayedEolWrap && _wrappedRow.has_value()) + { + // If we wrapped the last line, and we're in the middle of painting it, + // then the newline we did above just manually broke the line. What + // we're doing here is a hack: we're going to manually re-invalidate the + // last character of the wrapped row. When the PaintBufferLine calls + // come back through, we'll paint this last character again, causing us + // to get into the wrapped state once again. This is the only way to + // ensure that if a line was wrapped, and we painted the first line in + // one frame, and the second line in another frame that included other + // changes _above_ the wrapped line, that we maintain the wrap state in + // the Terminal. + const til::rectangle lastCellOfWrappedRow{ + til::point{ _lastViewport.RightInclusive(), _wrappedRow.value() }, + til::size{ 1, 1 } + }; + _trace.TraceInvalidate(lastCellOfWrappedRow); + _invalidMap.set(lastCellOfWrappedRow); + } + + // If the entire viewport was invalidated this frame, don't mark the bottom + // line as new. There are cases where this can cause visual artifacts - see + // GH#5039 and ConptyRoundtripTests::ClearHostTrickeryTest + const bool allInvalidated = _invalidMap.all(); + _newBottomLine = !allInvalidated; + + return S_OK; } CATCH_RETURN(); diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index e643f51db5f..8ece3076288 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -32,7 +32,12 @@ using namespace Microsoft::Console::Types; _titleChanged; _quickReturn = !somethingToDo; - _trace.TraceStartPaint(_quickReturn, _invalidMap, _lastViewport.ToInclusive(), _scrollDelta, _cursorMoved); + _trace.TraceStartPaint(_quickReturn, + _invalidMap, + _lastViewport.ToInclusive(), + _scrollDelta, + _cursorMoved, + _wrappedRow); return _quickReturn ? S_FALSE : S_OK; } @@ -440,8 +445,13 @@ using namespace Microsoft::Console::Types; (!_clearedAllThisFrame); // If we're not using erase char, but we did erase all at the start of the - // frame, don't add spaces at the end. - const bool removeSpaces = (useEraseChar || (_clearedAllThisFrame) || (_newBottomLine)); + // frame, don't add spaces at the end. + // + // GH#5161: Only removeSpaces when we're in the _newBottomLine state and the + // line we're trying to print right now _actually is the bottom line_ + const bool removeSpaces = useEraseChar || + _clearedAllThisFrame || + (_newBottomLine && coord.Y == _lastViewport.BottomInclusive()); const size_t cchActual = removeSpaces ? (cchLine - numSpaces) : cchLine; @@ -458,6 +468,7 @@ using namespace Microsoft::Console::Types; // the cursor is still waiting on that character for the next character // to follow it. _wrappedRow = std::nullopt; + _trace.TraceClearWrapped(); } // Move the cursor to the start of this run. @@ -467,18 +478,16 @@ using namespace Microsoft::Console::Types; std::wstring wstr = std::wstring(unclusteredString.data(), cchActual); RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8(wstr)); - // If we've written text to the last column of the viewport, then mark + // GH#4415, GH#5181 + // If the renderer told us that this was a wrapped line, then mark // that we've wrapped this line. The next time we attempt to move the // cursor, if we're trying to move it to the start of the next line, // we'll remember that this line was wrapped, and not manually break the // line. - // Don't do this if the last character we're writing is a space - The last - // char will always be a space, but if we see that, we shouldn't wrap. - const short lastWrittenChar = base::ClampAdd(_lastText.X, base::ClampSub(totalWidth, numSpaces)); - if (lineWrapped && - lastWrittenChar > _lastViewport.RightInclusive()) + if (lineWrapped) { _wrappedRow = coord.Y; + _trace.TraceSetWrapped(coord.Y); } // Update our internal tracker of the cursor's position. @@ -538,7 +547,7 @@ using namespace Microsoft::Console::Types; RETURN_IF_FAILED(_EraseLine()); } } - else if (_newBottomLine) + else if (_newBottomLine && coord.Y == _lastViewport.BottomInclusive()) { // If we're on a new line, then we don't need to erase the line. The // line is already empty. @@ -546,7 +555,7 @@ using namespace Microsoft::Console::Types; { _deferredCursorPos = { _lastText.X + sNumSpaces, _lastText.Y }; } - else + else if (numSpaces > 0) { std::wstring spaces = std::wstring(numSpaces, L' '); RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8(spaces)); @@ -555,9 +564,12 @@ using namespace Microsoft::Console::Types; } } - // If we previously though that this was a new bottom line, it certainly - // isn't new any longer. - _newBottomLine = false; + // If we printed to the bottom line, and we previously thought that this was + // a new bottom line, it certainly isn't new any longer. + if (coord.Y == _lastViewport.BottomInclusive()) + { + _newBottomLine = false; + } return S_OK; } diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index bcbaf4994fd..8714f785561 100644 --- a/src/renderer/vt/state.cpp +++ b/src/renderer/vt/state.cpp @@ -36,7 +36,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe, _lastWasBold(false), _lastViewport(initialViewport), _invalidMap(initialViewport.Dimensions()), - _lastRealCursor({ 0 }), _lastText({ 0 }), _scrollDelta({ 0, 0 }), _quickReturn(false), diff --git a/src/renderer/vt/tracing.cpp b/src/renderer/vt/tracing.cpp index 304d15cc559..4b7ce05ce92 100644 --- a/src/renderer/vt/tracing.cpp +++ b/src/renderer/vt/tracing.cpp @@ -148,7 +148,8 @@ void RenderTracing::TraceStartPaint(const bool quickReturn, const til::bitmap invalidMap, const til::rectangle lastViewport, const til::point scrollDelt, - const bool cursorMoved) const + const bool cursorMoved, + const std::optional& wrappedRow) const { #ifndef UNIT_TESTING if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) @@ -159,14 +160,29 @@ void RenderTracing::TraceStartPaint(const bool quickReturn, const auto lastView = lastViewStr.c_str(); const auto scrollDeltaStr = scrollDelt.to_string(); const auto scrollDelta = scrollDeltaStr.c_str(); - TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, - "VtEngine_TraceStartPaint", - TraceLoggingBool(quickReturn), - TraceLoggingWideString(invalidated), - TraceLoggingWideString(lastView), - TraceLoggingWideString(scrollDelta), - TraceLoggingBool(cursorMoved), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + if (wrappedRow.has_value()) + { + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceStartPaint", + TraceLoggingBool(quickReturn), + TraceLoggingWideString(invalidated), + TraceLoggingWideString(lastView), + TraceLoggingWideString(scrollDelta), + TraceLoggingBool(cursorMoved), + TraceLoggingValue(wrappedRow.value()), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } + else + { + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceStartPaint", + TraceLoggingBool(quickReturn), + TraceLoggingWideString(invalidated), + TraceLoggingWideString(lastView), + TraceLoggingWideString(scrollDelta), + TraceLoggingBool(cursorMoved), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } } #else UNREFERENCED_PARAMETER(quickReturn); @@ -174,6 +190,7 @@ void RenderTracing::TraceStartPaint(const bool quickReturn, UNREFERENCED_PARAMETER(lastViewport); UNREFERENCED_PARAMETER(scrollDelt); UNREFERENCED_PARAMETER(cursorMoved); + UNREFERENCED_PARAMETER(wrappedRow); #endif UNIT_TESTING } @@ -203,6 +220,24 @@ void RenderTracing::TraceLastText(const til::point lastTextPos) const UNREFERENCED_PARAMETER(lastTextPos); #endif UNIT_TESTING } + +void RenderTracing::TraceScrollFrame(const til::point scrollDeltaPos) const +{ +#ifndef UNIT_TESTING + if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + const auto scrollDeltaStr = scrollDeltaPos.to_string(); + const auto scrollDelta = scrollDeltaStr.c_str(); + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceScrollFrame", + TraceLoggingWideString(scrollDelta), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } +#else + UNREFERENCED_PARAMETER(scrollDeltaPos); +#endif UNIT_TESTING +} + void RenderTracing::TraceMoveCursor(const til::point lastTextPos, const til::point cursor) const { #ifndef UNIT_TESTING @@ -241,6 +276,36 @@ void RenderTracing::TraceWrapped() const #endif UNIT_TESTING } +void RenderTracing::TraceSetWrapped(const short wrappedRow) const +{ +#ifndef UNIT_TESTING + if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceSetWrapped", + TraceLoggingValue(wrappedRow), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } +#else + UNREFERENCED_PARAMETER(wrappedRow); +#endif UNIT_TESTING +} + +void RenderTracing::TraceClearWrapped() const +{ +#ifndef UNIT_TESTING + if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + const auto* const msg = "Cleared wrap state"; + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceClearWrapped", + TraceLoggingString(msg), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } +#else +#endif UNIT_TESTING +} + void RenderTracing::TracePaintCursor(const til::point coordCursor) const { #ifndef UNIT_TESTING diff --git a/src/renderer/vt/tracing.hpp b/src/renderer/vt/tracing.hpp index 0c36b42f05a..f5ec8e9c5ff 100644 --- a/src/renderer/vt/tracing.hpp +++ b/src/renderer/vt/tracing.hpp @@ -29,7 +29,10 @@ namespace Microsoft::Console::VirtualTerminal void TraceString(const std::string_view& str) const; void TraceInvalidate(const til::rectangle view) const; void TraceLastText(const til::point lastText) const; + void TraceScrollFrame(const til::point scrollDelta) const; void TraceMoveCursor(const til::point lastText, const til::point cursor) const; + void TraceSetWrapped(const short wrappedRow) const; + void TraceClearWrapped() const; void TraceWrapped() const; void TracePaintCursor(const til::point coordCursor) const; void TraceInvalidateAll(const til::rectangle view) const; @@ -39,7 +42,8 @@ namespace Microsoft::Console::VirtualTerminal const til::bitmap invalidMap, const til::rectangle lastViewport, const til::point scrollDelta, - const bool cursorMoved) const; + const bool cursorMoved, + const std::optional& wrappedRow) const; void TraceEndPaint() const; }; } diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 2230410f4d2..3b84bc5cac8 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -124,7 +124,6 @@ namespace Microsoft::Console::Render til::bitmap _invalidMap; - COORD _lastRealCursor; COORD _lastText; til::point _scrollDelta;