diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 4179e19cc39..e4acabbdc93 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -454,6 +454,22 @@ void Terminal::_WriteBuffer(const std::wstring_view& stringView) // With well behaving shells during normal operation this safeguard should normally not be encountered. proposedCursorPosition.X = 0; proposedCursorPosition.Y++; + + // Try the character again. + i--; + + // Mark the line we're currently on as wrapped + + // TODO: GH#780 - This should really be a _deferred_ newline. If + // the next character to come in is a newline or a cursor + // movement or anything, then we should _not_ wrap this line + // here. + // + // This is more WriteCharsLegacy2ElectricBoogaloo work. I'm + // leaving it like this for now - it'll break for lines that + // _exactly_ wrap, but we can't re-wrap lines now anyways, so it + // doesn't matter. + _buffer->GetRowByOffset(cursorPosBefore.Y).GetCharRow().SetWrapForced(true); } _AdjustCursorPosition(proposedCursorPosition); diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 112cf396432..5c031185c51 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -44,7 +44,7 @@ using namespace Microsoft::Terminal::Core; namespace TerminalCoreUnitTests { - class TerminalBufferTests; + class ConptyRoundtripTests; }; using namespace TerminalCoreUnitTests; @@ -152,8 +152,14 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final TEST_METHOD(SimpleWriteOutputTest); TEST_METHOD(WriteTwoLinesUsesNewline); TEST_METHOD(WriteAFewSimpleLines); + TEST_METHOD(PassthroughClearScrollback); + TEST_METHOD(TestWrappingALongString); + TEST_METHOD(TestAdvancedWrapping); + TEST_METHOD(TestExactWrappingWithoutSpaces); + TEST_METHOD(TestExactWrappingWithSpaces); + TEST_METHOD(MoveCursorAtEOL); private: @@ -348,6 +354,262 @@ void ConptyRoundtripTests::WriteAFewSimpleLines() verifyData(termTb); } +void ConptyRoundtripTests::TestWrappingALongString() +{ + 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(); + _checkConptyOutput = false; + + const auto initialTermView = term->GetViewport(); + + const auto charsToWrite = gsl::narrow_cast(TestUtils::Test100CharsString.size()); + VERIFY_ARE_EQUAL(100, charsToWrite); + + VERIFY_ARE_EQUAL(0, initialTermView.Top()); + VERIFY_ARE_EQUAL(32, initialTermView.BottomExclusive()); + + hostSm.ProcessString(TestUtils::Test100CharsString); + + const auto secondView = term->GetViewport(); + + VERIFY_ARE_EQUAL(0, secondView.Top()); + VERIFY_ARE_EQUAL(32, secondView.BottomExclusive()); + + auto verifyBuffer = [&](const TextBuffer& tb) { + auto& cursor = tb.GetCursor(); + // Verify the cursor wrapped to the second line + VERIFY_ARE_EQUAL(charsToWrite % initialTermView.Width(), cursor.GetPosition().X); + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + + // Verify that we marked the 0th row as _wrapped_ + const auto& row0 = tb.GetRowByOffset(0); + VERIFY_IS_TRUE(row0.GetCharRow().WasWrapForced()); + + const auto& row1 = tb.GetRowByOffset(1); + VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced()); + + TestUtils::VerifyExpectedString(tb, TestUtils::Test100CharsString, { 0, 0 }); + }; + + verifyBuffer(hostTb); + + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + verifyBuffer(termTb); +} + +void ConptyRoundtripTests::TestAdvancedWrapping() +{ + 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; + const auto initialTermView = term->GetViewport(); + + _flushFirstFrame(); + + const auto charsToWrite = gsl::narrow_cast(TestUtils::Test100CharsString.size()); + VERIFY_ARE_EQUAL(100, charsToWrite); + + hostSm.ProcessString(TestUtils::Test100CharsString); + hostSm.ProcessString(L"\n"); + hostSm.ProcessString(L" "); + hostSm.ProcessString(L"1234567890"); + + auto verifyBuffer = [&](const TextBuffer& tb) { + auto& cursor = tb.GetCursor(); + // Verify the cursor wrapped to the second line + VERIFY_ARE_EQUAL(2, cursor.GetPosition().Y); + VERIFY_ARE_EQUAL(20, cursor.GetPosition().X); + + // Verify that we marked the 0th row as _wrapped_ + const auto& row0 = tb.GetRowByOffset(0); + VERIFY_IS_TRUE(row0.GetCharRow().WasWrapForced()); + + const auto& row1 = tb.GetRowByOffset(1); + VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced()); + + TestUtils::VerifyExpectedString(tb, TestUtils::Test100CharsString, { 0, 0 }); + TestUtils::VerifyExpectedString(tb, L" 1234567890", { 0, 2 }); + }; + + verifyBuffer(hostTb); + + // First write the first 80 characters from the string + expectedOutput.push_back(R"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)"); + // Without line breaking, write the remaining 20 chars + expectedOutput.push_back(R"(qrstuvwxyz{|}~!"#$%&)"); + // Clear the rest of row 1 + expectedOutput.push_back("\x1b[K"); + // This is the hard line break + expectedOutput.push_back("\r\n"); + // Now write row 2 of the buffer + expectedOutput.push_back(" 1234567890"); + // and clear everything after the text, because the buffer is empty. + expectedOutput.push_back("\x1b[K"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + verifyBuffer(termTb); +} + +void ConptyRoundtripTests::TestExactWrappingWithoutSpaces() +{ + // This test (and TestExactWrappingWitSpaces) reveals a bug in the old + // implementation. + // + // If a line _exactly_ wraps to the next line, we can't tell if the line + // should really wrap, or manually break. The client app is writing a line + // that's exactly the width of the buffer that manually linebreaked at the + // end of the line, followed by another line. + // + // With the old PaintBufferLine interface, there's no way to know if this + // case is because the line wrapped or not. Hence, the addition of the + // `lineWrapped` parameter + + 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; + + const auto initialTermView = term->GetViewport(); + + _flushFirstFrame(); + + const auto charsToWrite = initialTermView.Width(); + VERIFY_ARE_EQUAL(80, charsToWrite); + + for (auto i = 0; i < charsToWrite; i++) + { + // This is a handy way of just printing the printable characters that + // _aren't_ the space character. + const wchar_t wch = static_cast(33 + (i % 94)); + hostSm.ProcessCharacter(wch); + } + + hostSm.ProcessString(L"\n"); + hostSm.ProcessString(L"1234567890"); + + auto verifyBuffer = [&](const TextBuffer& tb, const bool isTerminal) { + auto& cursor = tb.GetCursor(); + // Verify the cursor wrapped to the second line + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + VERIFY_ARE_EQUAL(10, cursor.GetPosition().X); + + // TODO: GH#780 - In the Terminal, neither line should be wrapped. + // Unfortunately, until WriteCharsLegacy2ElectricBoogaloo is complete, + // the Terminal will still treat the first line as wrapped. When #780 is + // implemented, these tests will fail, and should again expect the first + // line to not be wrapped. + + // Verify that we marked the 0th row as _not wrapped_ + const auto& row0 = tb.GetRowByOffset(0); + VERIFY_ARE_EQUAL(isTerminal, row0.GetCharRow().WasWrapForced()); + + const auto& row1 = tb.GetRowByOffset(1); + VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced()); + + TestUtils::VerifyExpectedString(tb, LR"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)", { 0, 0 }); + TestUtils::VerifyExpectedString(tb, L"1234567890", { 0, 1 }); + }; + + verifyBuffer(hostTb, false); + + // First write the first 80 characters from the string + expectedOutput.push_back(R"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)"); + + // This is the hard line break + expectedOutput.push_back("\r\n"); + // Now write row 2 of the buffer + expectedOutput.push_back("1234567890"); + // and clear everything after the text, because the buffer is empty. + expectedOutput.push_back("\x1b[K"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + verifyBuffer(termTb, true); +} + +void ConptyRoundtripTests::TestExactWrappingWithSpaces() +{ + // This test is also explained by the comment at the top of TestExactWrappingWithoutSpaces + + 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; + const auto initialTermView = term->GetViewport(); + + _flushFirstFrame(); + + const auto charsToWrite = initialTermView.Width(); + VERIFY_ARE_EQUAL(80, charsToWrite); + + for (auto i = 0; i < charsToWrite; i++) + { + // This is a handy way of just printing the printable characters that + // _aren't_ the space character. + const wchar_t wch = static_cast(33 + (i % 94)); + hostSm.ProcessCharacter(wch); + } + + hostSm.ProcessString(L"\n"); + hostSm.ProcessString(L" "); + hostSm.ProcessString(L"1234567890"); + + auto verifyBuffer = [&](const TextBuffer& tb, const bool isTerminal) { + auto& cursor = tb.GetCursor(); + // Verify the cursor wrapped to the second line + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + VERIFY_ARE_EQUAL(20, cursor.GetPosition().X); + + // TODO: GH#780 - In the Terminal, neither line should be wrapped. + // Unfortunately, until WriteCharsLegacy2ElectricBoogaloo is complete, + // the Terminal will still treat the first line as wrapped. When #780 is + // implemented, these tests will fail, and should again expect the first + // line to not be wrapped. + + // Verify that we marked the 0th row as _not wrapped_ + const auto& row0 = tb.GetRowByOffset(0); + VERIFY_ARE_EQUAL(isTerminal, row0.GetCharRow().WasWrapForced()); + + const auto& row1 = tb.GetRowByOffset(1); + VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced()); + + TestUtils::VerifyExpectedString(tb, LR"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)", { 0, 0 }); + TestUtils::VerifyExpectedString(tb, L" 1234567890", { 0, 1 }); + }; + + verifyBuffer(hostTb, false); + + // First write the first 80 characters from the string + expectedOutput.push_back(R"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)"); + + // This is the hard line break + expectedOutput.push_back("\r\n"); + // Now write row 2 of the buffer + expectedOutput.push_back(" 1234567890"); + // and clear everything after the text, because the buffer is empty. + expectedOutput.push_back("\x1b[K"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + verifyBuffer(termTb, true); +} + void ConptyRoundtripTests::MoveCursorAtEOL() { // This is a test for GH#1245 @@ -360,7 +622,6 @@ void ConptyRoundtripTests::MoveCursorAtEOL() auto& hostSm = si.GetStateMachine(); auto& hostTb = si.GetTextBuffer(); auto& termTb = *term->_buffer; - _flushFirstFrame(); Log::Comment(NoThrowString().Format( diff --git a/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp b/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp index 57285c4e390..08788414c67 100644 --- a/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp @@ -29,6 +29,9 @@ class TerminalCoreUnitTests::TerminalBufferTests final TEST_METHOD(TestSimpleBufferWriting); + TEST_METHOD(TestWrappingCharByChar); + TEST_METHOD(TestWrappingALongString); + TEST_METHOD_SETUP(MethodSetup) { // STEP 1: Set up the Terminal @@ -66,3 +69,76 @@ void TerminalBufferTests::TestSimpleBufferWriting() TestUtils::VerifyExpectedString(termTb, L"Hello World", { 0, 0 }); } + +void TerminalBufferTests::TestWrappingCharByChar() +{ + auto& termTb = *term->_buffer; + auto& termSm = *term->_stateMachine; + const auto initialView = term->GetViewport(); + auto& cursor = termTb.GetCursor(); + + const auto charsToWrite = gsl::narrow_cast(TestUtils::Test100CharsString.size()); + + VERIFY_ARE_EQUAL(0, initialView.Top()); + VERIFY_ARE_EQUAL(32, initialView.BottomExclusive()); + + for (auto i = 0; i < charsToWrite; i++) + { + // This is a handy way of just printing the printable characters that + // _aren't_ the space character. + const wchar_t wch = static_cast(33 + (i % 94)); + termSm.ProcessCharacter(wch); + } + + const auto secondView = term->GetViewport(); + + VERIFY_ARE_EQUAL(0, secondView.Top()); + VERIFY_ARE_EQUAL(32, secondView.BottomExclusive()); + + // Verify the cursor wrapped to the second line + VERIFY_ARE_EQUAL(charsToWrite % initialView.Width(), cursor.GetPosition().X); + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + + // Verify that we marked the 0th row as _wrapped_ + const auto& row0 = termTb.GetRowByOffset(0); + VERIFY_IS_TRUE(row0.GetCharRow().WasWrapForced()); + + const auto& row1 = termTb.GetRowByOffset(1); + VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced()); + + TestUtils::VerifyExpectedString(termTb, TestUtils::Test100CharsString, { 0, 0 }); +} + +void TerminalBufferTests::TestWrappingALongString() +{ + auto& termTb = *term->_buffer; + auto& termSm = *term->_stateMachine; + const auto initialView = term->GetViewport(); + auto& cursor = termTb.GetCursor(); + + const auto charsToWrite = gsl::narrow_cast(TestUtils::Test100CharsString.size()); + VERIFY_ARE_EQUAL(100, charsToWrite); + + VERIFY_ARE_EQUAL(0, initialView.Top()); + VERIFY_ARE_EQUAL(32, initialView.BottomExclusive()); + + termSm.ProcessString(TestUtils::Test100CharsString); + + const auto secondView = term->GetViewport(); + + VERIFY_ARE_EQUAL(0, secondView.Top()); + VERIFY_ARE_EQUAL(32, secondView.BottomExclusive()); + + // Verify the cursor wrapped to the second line + VERIFY_ARE_EQUAL(charsToWrite % initialView.Width(), cursor.GetPosition().X); + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + + // Verify that we marked the 0th row as _wrapped_ + const auto& row0 = termTb.GetRowByOffset(0); + VERIFY_IS_TRUE(row0.GetCharRow().WasWrapForced()); + + const auto& row1 = termTb.GetRowByOffset(1); + VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced()); + + TestUtils::VerifyExpectedString(termTb, TestUtils::Test100CharsString, { 0, 0 }); +} diff --git a/src/cascadia/UnitTests_TerminalCore/TestUtils.h b/src/cascadia/UnitTests_TerminalCore/TestUtils.h index eb915445928..f7ddcdac0d1 100644 --- a/src/cascadia/UnitTests_TerminalCore/TestUtils.h +++ b/src/cascadia/UnitTests_TerminalCore/TestUtils.h @@ -21,6 +21,10 @@ namespace TerminalCoreUnitTests class TerminalCoreUnitTests::TestUtils { public: + static constexpr std::wstring_view Test100CharsString{ + LR"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~!"#$%&)" + }; + // Function Description: // - Helper function to validate that a number of characters in a row are all // the same. Validates that the next end-start characters are all equal to the diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index 8b1ea38ee9d..0e27578d4ff 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -604,7 +604,7 @@ void VtRendererTest::Xterm256TestCursor() clusters.emplace_back(std::wstring_view{ &line[i], 1 }, static_cast(rgWidths[i])); } - VERIFY_SUCCEEDED(engine->PaintBufferLine({ clusters.data(), clusters.size() }, { 1, 1 }, false)); + VERIFY_SUCCEEDED(engine->PaintBufferLine({ clusters.data(), clusters.size() }, { 1, 1 }, false, false)); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); VERIFY_SUCCEEDED(engine->_MoveCursor({ 10, 1 })); @@ -1020,7 +1020,7 @@ void VtRendererTest::XtermTestCursor() clusters.emplace_back(std::wstring_view{ &line[i], 1 }, static_cast(rgWidths[i])); } - VERIFY_SUCCEEDED(engine->PaintBufferLine({ clusters.data(), clusters.size() }, { 1, 1 }, false)); + VERIFY_SUCCEEDED(engine->PaintBufferLine({ clusters.data(), clusters.size() }, { 1, 1 }, false, false)); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); VERIFY_SUCCEEDED(engine->_MoveCursor({ 10, 1 })); @@ -1250,7 +1250,7 @@ void VtRendererTest::WinTelnetTestCursor() clusters.emplace_back(std::wstring_view{ &line[i], 1 }, static_cast(rgWidths[i])); } - VERIFY_SUCCEEDED(engine->PaintBufferLine({ clusters.data(), clusters.size() }, { 1, 1 }, false)); + VERIFY_SUCCEEDED(engine->PaintBufferLine({ clusters.data(), clusters.size() }, { 1, 1 }, false, false)); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); VERIFY_SUCCEEDED(engine->_MoveCursor({ 10, 1 })); @@ -1315,8 +1315,8 @@ void VtRendererTest::TestWrapping() clusters2.emplace_back(std::wstring_view{ &line2[i], 1 }, static_cast(rgWidths[i])); } - VERIFY_SUCCEEDED(engine->PaintBufferLine({ clusters1.data(), clusters1.size() }, { 0, 0 }, false)); - VERIFY_SUCCEEDED(engine->PaintBufferLine({ clusters2.data(), clusters2.size() }, { 0, 1 }, false)); + VERIFY_SUCCEEDED(engine->PaintBufferLine({ clusters1.data(), clusters1.size() }, { 0, 0 }, false, false)); + VERIFY_SUCCEEDED(engine->PaintBufferLine({ clusters2.data(), clusters2.size() }, { 0, 1 }, false, false)); }); } diff --git a/src/interactivity/onecore/BgfxEngine.cpp b/src/interactivity/onecore/BgfxEngine.cpp index 32d52760bdf..11af29783c8 100644 --- a/src/interactivity/onecore/BgfxEngine.cpp +++ b/src/interactivity/onecore/BgfxEngine.cpp @@ -147,7 +147,8 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid [[nodiscard]] HRESULT BgfxEngine::PaintBufferLine(const std::basic_string_view clusters, const COORD coord, - const bool /*trimLeft*/) noexcept + const bool /*trimLeft*/, + const bool /*lineWrapped*/) noexcept { try { diff --git a/src/interactivity/onecore/BgfxEngine.hpp b/src/interactivity/onecore/BgfxEngine.hpp index 9ce46e35708..dcb20b99735 100644 --- a/src/interactivity/onecore/BgfxEngine.hpp +++ b/src/interactivity/onecore/BgfxEngine.hpp @@ -51,7 +51,8 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT PaintBackground() noexcept override; [[nodiscard]] HRESULT PaintBufferLine(const std::basic_string_view clusters, const COORD coord, - const bool trimLeft) noexcept override; + const bool trimLeft, + const bool lineWrapped) noexcept override; [[nodiscard]] HRESULT PaintBufferGridLines(GridLines const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; [[nodiscard]] HRESULT PaintSelection(const SMALL_RECT rect) noexcept override; diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index e1b0b970f8c..ae6a25ab364 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -597,15 +597,23 @@ void Renderer::_PaintBufferOutput(_In_ IRenderEngine* const pEngine) // Retrieve the cell information iterator limited to just this line we want to redraw. auto it = buffer.GetCellDataAt(bufferLine.Origin(), bufferLine); + // Calculate if two things are true: + // 1. this row wrapped + // 2. We're painting the last col of the row. + // In that case, set lineWrapped=true for the _PaintBufferOutputHelper call. + const auto lineWrapped = (buffer.GetRowByOffset(bufferLine.Origin().Y).GetCharRow().WasWrapForced()) && + (bufferLine.RightExclusive() == buffer.GetSize().Width()); + // Ask the helper to paint through this specific line. - _PaintBufferOutputHelper(pEngine, it, screenLine.Origin()); + _PaintBufferOutputHelper(pEngine, it, screenLine.Origin(), lineWrapped); } } } void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine, TextBufferCellIterator it, - const COORD target) + const COORD target, + const bool lineWrapped) { // If we have valid data, let's figure out how to draw it. if (it) @@ -694,7 +702,7 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine, } while (it); // Do the painting. - THROW_IF_FAILED(pEngine->PaintBufferLine({ clusters.data(), clusters.size() }, screenPoint, trimLeft)); + THROW_IF_FAILED(pEngine->PaintBufferLine({ clusters.data(), clusters.size() }, screenPoint, trimLeft, lineWrapped)); // If we're allowed to do grid drawing, draw that now too (since it will be coupled with the color data) if (_pData->IsGridLineDrawingAllowed()) @@ -843,7 +851,7 @@ void Renderer::_PaintOverlay(IRenderEngine& engine, auto it = overlay.buffer.GetCellLineDataAt(source); - _PaintBufferOutputHelper(&engine, it, target); + _PaintBufferOutputHelper(&engine, it, target, false); } } } diff --git a/src/renderer/base/renderer.hpp b/src/renderer/base/renderer.hpp index c77df20df24..3e02cc7aea1 100644 --- a/src/renderer/base/renderer.hpp +++ b/src/renderer/base/renderer.hpp @@ -96,7 +96,8 @@ namespace Microsoft::Console::Render void _PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine, TextBufferCellIterator it, - const COORD target); + const COORD target, + const bool lineWrapped); static IRenderEngine::GridLines s_GetGridlines(const TextAttribute& textAttribute) noexcept; diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index cb6baeb6fa4..21898d5f74d 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1220,7 +1220,8 @@ void DxEngine::_InvalidOr(RECT rc) noexcept // - S_OK or relevant DirectX error [[nodiscard]] HRESULT DxEngine::PaintBufferLine(std::basic_string_view const clusters, COORD const coord, - const bool /*trimLeft*/) noexcept + const bool /*trimLeft*/, + const bool /*lineWrapped*/) noexcept { try { diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index f0358d81495..c744bcf30e3 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -76,7 +76,8 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT PaintBackground() noexcept override; [[nodiscard]] HRESULT PaintBufferLine(std::basic_string_view const clusters, COORD const coord, - bool const fTrimLeft) noexcept override; + bool const fTrimLeft, + const bool lineWrapped) noexcept override; [[nodiscard]] HRESULT PaintBufferGridLines(GridLines const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; [[nodiscard]] HRESULT PaintSelection(const SMALL_RECT rect) noexcept override; diff --git a/src/renderer/gdi/gdirenderer.hpp b/src/renderer/gdi/gdirenderer.hpp index 4840a181985..1fb1c09b05c 100644 --- a/src/renderer/gdi/gdirenderer.hpp +++ b/src/renderer/gdi/gdirenderer.hpp @@ -44,7 +44,8 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT PaintBackground() noexcept override; [[nodiscard]] HRESULT PaintBufferLine(std::basic_string_view const clusters, const COORD coord, - const bool trimLeft) noexcept override; + const bool trimLeft, + const bool lineWrapped) noexcept override; [[nodiscard]] HRESULT PaintBufferGridLines(const GridLines lines, const COLORREF color, const size_t cchLine, diff --git a/src/renderer/gdi/paint.cpp b/src/renderer/gdi/paint.cpp index bbce43702aa..a6683085eaf 100644 --- a/src/renderer/gdi/paint.cpp +++ b/src/renderer/gdi/paint.cpp @@ -286,7 +286,8 @@ using namespace Microsoft::Console::Render; //#define MAX_POLY_LINES 80 [[nodiscard]] HRESULT GdiEngine::PaintBufferLine(std::basic_string_view const clusters, const COORD coord, - const bool trimLeft) noexcept + const bool trimLeft, + const bool /*lineWrapped*/) noexcept { try { diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp index aff56a5bb58..455b6e8bb22 100644 --- a/src/renderer/inc/IRenderEngine.hpp +++ b/src/renderer/inc/IRenderEngine.hpp @@ -93,7 +93,8 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT PaintBackground() noexcept = 0; [[nodiscard]] virtual HRESULT PaintBufferLine(std::basic_string_view const clusters, const COORD coord, - const bool fTrimLeft) noexcept = 0; + const bool fTrimLeft, + const bool lineWrapped) noexcept = 0; [[nodiscard]] virtual HRESULT PaintBufferGridLines(const GridLines lines, const COLORREF color, const size_t cchLine, diff --git a/src/renderer/uia/UiaRenderer.cpp b/src/renderer/uia/UiaRenderer.cpp index 1d919b99809..f63ccd3db28 100644 --- a/src/renderer/uia/UiaRenderer.cpp +++ b/src/renderer/uia/UiaRenderer.cpp @@ -276,7 +276,8 @@ UiaEngine::UiaEngine(IUiaEventDispatcher* dispatcher) : // - S_FALSE [[nodiscard]] HRESULT UiaEngine::PaintBufferLine(std::basic_string_view const /*clusters*/, COORD const /*coord*/, - const bool /*trimLeft*/) noexcept + const bool /*trimLeft*/, + const bool /*lineWrapped*/) noexcept { return S_FALSE; } diff --git a/src/renderer/uia/UiaRenderer.hpp b/src/renderer/uia/UiaRenderer.hpp index 056a63b00a5..4dfbdccce41 100644 --- a/src/renderer/uia/UiaRenderer.hpp +++ b/src/renderer/uia/UiaRenderer.hpp @@ -53,7 +53,8 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT PaintBackground() noexcept override; [[nodiscard]] HRESULT PaintBufferLine(std::basic_string_view const clusters, COORD const coord, - bool const fTrimLeft) noexcept override; + bool const fTrimLeft, + const bool lineWrapped) noexcept override; [[nodiscard]] HRESULT PaintBufferGridLines(GridLines const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; [[nodiscard]] HRESULT PaintSelection(const SMALL_RECT rect) noexcept override; diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index dfbe434f2d2..3a8491c7204 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -19,7 +19,6 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, _ColorTable(ColorTable), _cColorTable(cColorTable), _fUseAsciiOnly(fUseAsciiOnly), - _previousLineWrapped(false), _usingUnderLine(false), _needToDisableCursor(false), _lastCursorIsVisible(false), @@ -235,6 +234,8 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, { HRESULT hr = S_OK; + _trace.TraceMoveCursor(_lastText, coord); + if (coord.X != _lastText.X || coord.Y != _lastText.Y) { if (coord.X == 0 && coord.Y == 0) @@ -248,8 +249,15 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // If the previous line wrapped, then the cursor is already at this // position, we just don't know it yet. Don't emit anything. - if (_previousLineWrapped) + bool previousLineWrapped = false; + if (_wrappedRow.has_value()) + { + previousLineWrapped = coord.Y == _wrappedRow.value() + 1; + } + + if (previousLineWrapped) { + _trace.TraceWrapped(); hr = S_OK; } else @@ -312,7 +320,11 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, _newBottomLine = false; } _deferredCursorPos = INVALID_COORDS; + + _wrappedRow = std::nullopt; + _delayedEolWrap = false; + return hr; } @@ -430,15 +442,19 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // - trimLeft - This specifies whether to trim one character width off the left // side of the output. Used for drawing the right-half only of a // double-wide character. +// - lineWrapped: true if this run we're painting is the end of a line that +// wrapped. If we're not painting the last column of a wrapped line, then this +// will be false. // Return Value: // - S_OK or suitable HRESULT error from writing pipe. [[nodiscard]] HRESULT XtermEngine::PaintBufferLine(std::basic_string_view const clusters, const COORD coord, - const bool /*trimLeft*/) noexcept + const bool /*trimLeft*/, + const bool lineWrapped) noexcept { return _fUseAsciiOnly ? VtEngine::_PaintAsciiBufferLine(clusters, coord) : - VtEngine::_PaintUtf8BufferLine(clusters, coord); + VtEngine::_PaintUtf8BufferLine(clusters, coord, lineWrapped); } // Method Description: diff --git a/src/renderer/vt/XtermEngine.hpp b/src/renderer/vt/XtermEngine.hpp index 28bdfc44080..e6a0a868c18 100644 --- a/src/renderer/vt/XtermEngine.hpp +++ b/src/renderer/vt/XtermEngine.hpp @@ -48,7 +48,8 @@ namespace Microsoft::Console::Render const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT PaintBufferLine(std::basic_string_view const clusters, const COORD coord, - const bool trimLeft) noexcept override; + const bool trimLeft, + const bool lineWrapped) noexcept override; [[nodiscard]] HRESULT ScrollFrame() noexcept override; [[nodiscard]] HRESULT InvalidateScroll(const COORD* const pcoordDelta) noexcept override; @@ -59,7 +60,6 @@ namespace Microsoft::Console::Render const COLORREF* const _ColorTable; const WORD _cColorTable; const bool _fUseAsciiOnly; - bool _previousLineWrapped; bool _usingUnderLine; bool _needToDisableCursor; bool _lastCursorIsVisible; diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index 1c20a85a1a9..1ca25594230 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -115,11 +115,15 @@ using namespace Microsoft::Console::Types; // - trimLeft - This specifies whether to trim one character width off the left // side of the output. Used for drawing the right-half only of a // double-wide character. +// - lineWrapped: true if this run we're painting is the end of a line that +// wrapped. If we're not painting the last column of a wrapped line, then this +// will be false. // Return Value: // - S_OK or suitable HRESULT error from writing pipe. [[nodiscard]] HRESULT VtEngine::PaintBufferLine(std::basic_string_view const clusters, const COORD coord, - const bool /*trimLeft*/) noexcept + const bool /*trimLeft*/, + const bool /*lineWrapped*/) noexcept { return VtEngine::_PaintAsciiBufferLine(clusters, coord); } @@ -149,6 +153,8 @@ using namespace Microsoft::Console::Types; // - S_OK or suitable HRESULT error from writing pipe. [[nodiscard]] HRESULT VtEngine::PaintCursor(const IRenderEngine::CursorOptions& options) noexcept { + _trace.TracePaintCursor(options.coordCursor); + // MSFT:15933349 - Send the terminal the updated cursor information, if it's changed. LOG_IF_FAILED(_MoveCursor(options.coordCursor)); @@ -369,15 +375,14 @@ using namespace Microsoft::Console::Types; // Return Value: // - S_OK or suitable HRESULT error from writing pipe. [[nodiscard]] HRESULT VtEngine::_PaintUtf8BufferLine(std::basic_string_view const clusters, - const COORD coord) noexcept + const COORD coord, + const bool lineWrapped) noexcept { if (coord.Y < _virtualTop) { return S_OK; } - RETURN_IF_FAILED(_MoveCursor(coord)); - std::wstring unclusteredString; unclusteredString.reserve(clusters.size()); short totalWidth = 0; @@ -445,10 +450,37 @@ using namespace Microsoft::Console::Types; (totalWidth - numSpaces) : totalWidth; + if (cchActual == 0) + { + // If the previous row wrapped, but this line is empty, then we actually + // do want to move the cursor down. Otherwise, we'll possibly end up + // accidentally erasing the last character from the previous line, as + // the cursor is still waiting on that character for the next character + // to follow it. + _wrappedRow = std::nullopt; + } + + // Move the cursor to the start of this run. + RETURN_IF_FAILED(_MoveCursor(coord)); + // Write the actual text string 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 + // 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()) + { + _wrappedRow = coord.Y; + } + // Update our internal tracker of the cursor's position. // See MSFT:20266233 (which is also GH#357) // If the cursor is at the rightmost column of the terminal, and we write a diff --git a/src/renderer/vt/tracing.cpp b/src/renderer/vt/tracing.cpp index 805835f1359..8246e6d920e 100644 --- a/src/renderer/vt/tracing.cpp +++ b/src/renderer/vt/tracing.cpp @@ -225,3 +225,48 @@ void RenderTracing::TraceLastText(const COORD lastTextPos) const UNREFERENCED_PARAMETER(lastTextPos); #endif UNIT_TESTING } +void RenderTracing::TraceMoveCursor(const COORD lastTextPos, const COORD cursor) const +{ +#ifndef UNIT_TESTING + const auto lastTextStr = _CoordToString(lastTextPos); + const auto lastText = lastTextStr.c_str(); + + const auto cursorStr = _CoordToString(cursor); + const auto cursorPos = cursorStr.c_str(); + + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceMoveCursor", + TraceLoggingString(lastText), + TraceLoggingString(cursorPos), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); +#else + UNREFERENCED_PARAMETER(lastTextPos); + UNREFERENCED_PARAMETER(cursor); +#endif UNIT_TESTING +} + +void RenderTracing::TraceWrapped() const +{ +#ifndef UNIT_TESTING + const auto* const msg = "Wrapped instead of \\r\\n"; + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceWrapped", + TraceLoggingString(msg), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); +#else +#endif UNIT_TESTING +} + +void RenderTracing::TracePaintCursor(const COORD coordCursor) const +{ +#ifndef UNIT_TESTING + const auto cursorPosString = _CoordToString(coordCursor); + const auto cursorPos = cursorPosString.c_str(); + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TracePaintCursor", + TraceLoggingString(cursorPos), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); +#else + UNREFERENCED_PARAMETER(coordCursor); +#endif UNIT_TESTING +} diff --git a/src/renderer/vt/tracing.hpp b/src/renderer/vt/tracing.hpp index 8d923adc813..0fcd1a97aac 100644 --- a/src/renderer/vt/tracing.hpp +++ b/src/renderer/vt/tracing.hpp @@ -29,6 +29,9 @@ namespace Microsoft::Console::VirtualTerminal void TraceString(const std::string_view& str) const; void TraceInvalidate(const Microsoft::Console::Types::Viewport view) const; void TraceLastText(const COORD lastText) const; + void TraceMoveCursor(const COORD lastText, const COORD cursor) const; + void TraceWrapped() const; + void TracePaintCursor(const COORD coordCursor) const; void TraceInvalidateAll(const Microsoft::Console::Types::Viewport view) const; void TraceTriggerCircling(const bool newFrame) const; void TraceStartPaint(const bool quickReturn, diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 5e9cdb1e716..b3a4c47deab 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -65,7 +65,8 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT PaintBackground() noexcept override; [[nodiscard]] virtual HRESULT PaintBufferLine(std::basic_string_view const clusters, const COORD coord, - const bool trimLeft) noexcept override; + const bool trimLeft, + const bool lineWrapped) noexcept override; [[nodiscard]] HRESULT PaintBufferGridLines(const GridLines lines, const COLORREF color, const size_t cchLine, @@ -144,6 +145,8 @@ namespace Microsoft::Console::Render Microsoft::Console::VirtualTerminal::RenderTracing _trace; bool _inResizeRequest{ false }; + std::optional _wrappedRow{ std::nullopt }; + bool _delayedEolWrap{ false }; [[nodiscard]] HRESULT _Write(std::string_view const str) noexcept; @@ -214,7 +217,8 @@ namespace Microsoft::Console::Render bool _WillWriteSingleChar() const; [[nodiscard]] HRESULT _PaintUtf8BufferLine(std::basic_string_view const clusters, - const COORD coord) noexcept; + const COORD coord, + const bool lineWrapped) noexcept; [[nodiscard]] HRESULT _PaintAsciiBufferLine(std::basic_string_view const clusters, const COORD coord) noexcept; diff --git a/src/renderer/wddmcon/WddmConRenderer.cpp b/src/renderer/wddmcon/WddmConRenderer.cpp index d9404f9dade..157309071cd 100644 --- a/src/renderer/wddmcon/WddmConRenderer.cpp +++ b/src/renderer/wddmcon/WddmConRenderer.cpp @@ -260,7 +260,8 @@ bool WddmConEngine::IsInitialized() [[nodiscard]] HRESULT WddmConEngine::PaintBufferLine(std::basic_string_view const clusters, const COORD coord, - const bool /*trimLeft*/) noexcept + const bool /*trimLeft*/, + const bool /*lineWrapped*/) noexcept { try { diff --git a/src/renderer/wddmcon/WddmConRenderer.hpp b/src/renderer/wddmcon/WddmConRenderer.hpp index c214d585a8e..70213ca95f8 100644 --- a/src/renderer/wddmcon/WddmConRenderer.hpp +++ b/src/renderer/wddmcon/WddmConRenderer.hpp @@ -43,7 +43,8 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT PaintBackground() noexcept override; [[nodiscard]] HRESULT PaintBufferLine(std::basic_string_view const clusters, const COORD coord, - const bool trimLeft) noexcept override; + const bool trimLeft, + const bool lineWrapped) noexcept override; [[nodiscard]] HRESULT PaintBufferGridLines(GridLines const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; [[nodiscard]] HRESULT PaintSelection(const SMALL_RECT rect) noexcept override;