diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 1aaec1bffbc..f37ff1a2810 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -573,27 +573,21 @@ bool TextBuffer::IncrementCircularBuffer(const bool inVtMode) } //Routine Description: -// - Retrieves the position of the last non-space character on the final line of the text buffer. -// - By default, we search the entire buffer to find the last non-space character -//Arguments: -// - -//Return Value: -// - Coordinate position in screen coordinates (offset coordinates, not array index coordinates). -COORD TextBuffer::GetLastNonSpaceCharacter() const -{ - return GetLastNonSpaceCharacter(GetSize()); -} - -//Routine Description: -// - Retrieves the position of the last non-space character in the given viewport -// - This is basically an optimized version of GetLastNonSpaceCharacter(), and can be called when -// - we know the last character is within the given viewport (so we don't need to check the entire buffer) +// - Retrieves the position of the last non-space character in the given +// viewport +// - By default, we search the entire buffer to find the last non-space +// character. +// - If we know the last character is within the given viewport (so we don't +// need to check the entire buffer), we can provide a value in viewOptional +// that we'll use to search for the last character in. //Arguments: // - The viewport //Return value: // - Coordinate position (relative to the text buffer) -COORD TextBuffer::GetLastNonSpaceCharacter(const Microsoft::Console::Types::Viewport viewport) const +COORD TextBuffer::GetLastNonSpaceCharacter(std::optional viewOptional) const { + const auto viewport = viewOptional.has_value() ? viewOptional.value() : GetSize(); + COORD coordEndOfText = { 0 }; // Search the given viewport by starting at the bottom. coordEndOfText.Y = viewport.BottomInclusive(); @@ -1872,9 +1866,18 @@ std::string TextBuffer::GenRTF(const TextAndColor& rows, const int fontHeightPoi // Arguments: // - oldBuffer - the text buffer to copy the contents FROM // - newBuffer - the text buffer to copy the contents TO +// - lastCharacterViewport - Optional. If the caller knows that the last +// nonspace character is in a particular Viewport, the caller can provide this +// parameter as an optimization, as opposed to searching the entire buffer. +// - oldViewportTop - Optional. The caller can provide a row in this parameter +// and we'll calculate the position of the _end_ of that row in the new +// buffer. The row's new value is placed back into this parameter. // Return Value: // - S_OK if we successfully copied the contents to the new buffer, otherwise an appropriate HRESULT. -HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer) +HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, + TextBuffer& newBuffer, + const std::optional lastCharacterViewport, + std::optional& oldViewportTop) { Cursor& oldCursor = oldBuffer.GetCursor(); Cursor& newCursor = newBuffer.GetCursor(); @@ -1886,14 +1889,14 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer) // place the new cursor back on the equivalent character in // the new buffer. const COORD cOldCursorPos = oldCursor.GetPosition(); - const COORD cOldLastChar = oldBuffer.GetLastNonSpaceCharacter(); + const COORD cOldLastChar = oldBuffer.GetLastNonSpaceCharacter(lastCharacterViewport); - short const cOldRowsTotal = cOldLastChar.Y + 1; - short const cOldColsTotal = oldBuffer.GetSize().Width(); + const short cOldRowsTotal = cOldLastChar.Y + 1; + const short cOldColsTotal = oldBuffer.GetSize().Width(); COORD cNewCursorPos = { 0 }; bool fFoundCursorPos = false; - + bool foundOldRow = false; HRESULT hr = S_OK; // Loop through all the rows of the old buffer and reprint them into the new buffer for (short iOldRow = 0; iOldRow < cOldRowsTotal; iOldRow++) @@ -1953,6 +1956,19 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer) } CATCH_RETURN(); } + + // If we found the old row that the caller was interested in, set the + // out value of that parameter to the cursor's current Y position (the + // new location of the _end_ of that row in the buffer). + if (oldViewportTop.has_value() && !foundOldRow) + { + if (iOldRow >= oldViewportTop.value()) + { + oldViewportTop = newCursor.GetPosition().Y; + foundOldRow = true; + } + } + if (SUCCEEDED(hr)) { // If we didn't have a full row to copy, insert a new diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index fe7a1e5d583..7b2ee52df39 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -103,8 +103,7 @@ class TextBuffer final // Scroll needs access to this to quickly rotate around the buffer. bool IncrementCircularBuffer(const bool inVtMode = false); - COORD GetLastNonSpaceCharacter() const; - COORD GetLastNonSpaceCharacter(const Microsoft::Console::Types::Viewport viewport) const; + COORD GetLastNonSpaceCharacter(std::optional viewOptional = std::nullopt) const; Cursor& GetCursor() noexcept; const Cursor& GetCursor() const noexcept; @@ -162,7 +161,10 @@ class TextBuffer final const std::wstring_view fontFaceName, const COLORREF backgroundColor); - static HRESULT Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer); + static HRESULT Reflow(TextBuffer& oldBuffer, + TextBuffer& newBuffer, + const std::optional lastCharacterViewport, + std::optional& oldViewportTop); private: std::deque _storage; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index e5f1824c649..fba7432b645 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -519,7 +519,6 @@ namespace winrt::TerminalApp::implementation // Create a connection based on the values in our settings object. const auto connection = _CreateConnectionFromSettings(profileGuid, settings); - TermControl term{ settings, connection }; // Add the new tab to the list of our tabs. diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp index d2f11dc1b0b..9c7b1cd26e8 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.cpp +++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp @@ -201,7 +201,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation try { const COORD dimensions{ gsl::narrow_cast(_initialCols), gsl::narrow_cast(_initialRows) }; - THROW_IF_FAILED(_CreatePseudoConsoleAndPipes(dimensions, 0, &_inPipe, &_outPipe, &_hPC)); + THROW_IF_FAILED(_CreatePseudoConsoleAndPipes(dimensions, PSEUDOCONSOLE_RESIZE_QUIRK, &_inPipe, &_outPipe, &_hPC)); THROW_IF_FAILED(_LaunchAttachedClient()); _startTime = std::chrono::high_resolution_clock::now(); diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index df55fafdaee..378a510bff2 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -173,24 +173,143 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting { return S_FALSE; } + const auto dx = viewportSize.X - oldDimensions.X; const auto oldTop = _mutableViewport.Top(); const short newBufferHeight = viewportSize.Y + _scrollbackLines; COORD bufferSize{ viewportSize.X, newBufferHeight }; - RETURN_IF_FAILED(_buffer->ResizeTraditional(bufferSize)); - auto proposedTop = oldTop; - const auto newView = Viewport::FromDimensions({ 0, proposedTop }, viewportSize); - const auto proposedBottom = newView.BottomExclusive(); + // Save cursor's relative height versus the viewport + const short sCursorHeightInViewportBefore = _buffer->GetCursor().GetPosition().Y - _mutableViewport.Top(); + + // This will be used to determine where the viewport should be in the new buffer. + const short oldViewportTop = _mutableViewport.Top(); + short newViewportTop = oldViewportTop; + + // First allocate a new text buffer to take the place of the current one. + std::unique_ptr newTextBuffer; + try + { + newTextBuffer = std::make_unique(bufferSize, + _buffer->GetCurrentAttributes(), + 0, // temporarily set size to 0 so it won't render. + _buffer->GetRenderTarget()); + + std::optional oldViewStart{ oldViewportTop }; + RETURN_IF_FAILED(TextBuffer::Reflow(*_buffer.get(), + *newTextBuffer.get(), + _mutableViewport, + oldViewStart)); + newViewportTop = oldViewStart.value(); + } + CATCH_RETURN(); + + // Conpty resizes a little oddly - if the height decreased, and there were + // blank lines at the bottom, those lines will get trimmed. If there's not + // blank lines, then the top will get "shifted down", moving the top line + // into scrollback. See GH#3490 for more details. + // + // If the final position in the buffer is on the bottom row of the new + // viewport, then we're going to need to move the top down. Otherwise, move + // the bottom up. + // + // There are also important things to consider with line wrapping. + // * If a line in scrollback wrapped that didn't previously, we'll need to + // make sure to have the new viewport down another line. This will cause + // our top to move down. + // * If a line _in the viewport_ wrapped that didn't previously, then the + // conpty buffer will also have that wrapped line, and will move the + // cursor & text down a line in response. This causes our bottom to move + // down. + // + // We're going to use a combo of both these things to calculate where the + // new viewport should be. To keep in sync with conpty, we'll need to make + // sure that any lines that entered the scrollback _stay in scrollback_. We + // do that by taking the max of + // * Where the old top line in the viewport exists in the new buffer (as + // calculated by TextBuffer::Reflow) + // * Where the bottom of the text in the new buffer is (and using that to + // calculate another proposed top location). + + const COORD newCursorPos = newTextBuffer->GetCursor().GetPosition(); +#pragma warning(push) +#pragma warning(disable : 26496) // cpp core checks wants this const, but it's assigned immediately below... + COORD newLastChar = newCursorPos; + try + { + newLastChar = newTextBuffer->GetLastNonSpaceCharacter(); + } + CATCH_LOG(); +#pragma warning(pop) + + const auto maxRow = std::max(newLastChar.Y, newCursorPos.Y); + + const short proposedTopFromLastLine = ::base::saturated_cast(maxRow - viewportSize.Y + 1); + const short proposedTopFromScrollback = newViewportTop; + + short proposedTop = std::max(proposedTopFromLastLine, + proposedTopFromScrollback); + + // If we're using the new location of the old top line to place the + // viewport, we might need to make an adjustment to it. + // + // We're using the last cell of the line to calculate where the top line is + // in the new buffer. If that line wrapped, then all the lines below it + // shifted down in the buffer. If there's space for all those lines in the + // conpty buffer, then the originally unwrapped top line will _still_ be in + // the buffer. In that case, don't stick to the _end_ of the old top line, + // instead stick to the _start_, which is one line up. + // + // We can know if there's space in the conpty buffer by checking if the + // maxRow (the highest row we've written text to) is above the viewport from + // this proposed top position. + if (proposedTop == proposedTopFromScrollback) + { + const auto proposedViewFromTop = Viewport::FromDimensions({ 0, proposedTopFromScrollback }, viewportSize); + if (maxRow < proposedViewFromTop.BottomInclusive()) + { + if (dx < 0 && proposedTop > 0) + { + try + { + auto row = newTextBuffer->GetRowByOffset(::base::saturated_cast(proposedTop - 1)); + if (row.GetCharRow().WasWrapForced()) + { + proposedTop--; + } + } + CATCH_LOG(); + } + } + } + + // If the new bottom would be higher than the last row of text, then we + // definitely want to use the last row of text to determine where the + // viewport should be. + const auto proposedViewFromTop = Viewport::FromDimensions({ 0, proposedTopFromScrollback }, viewportSize); + if (maxRow > proposedViewFromTop.BottomInclusive()) + { + proposedTop = proposedTopFromLastLine; + } + + // Make sure the proposed viewport is within the bounds of the buffer. + // First make sure the top is >=0 + proposedTop = std::max(static_cast(0), proposedTop); + // If the new bottom would be below the bottom of the buffer, then slide the // top up so that we'll still fit within the buffer. + const auto newView = Viewport::FromDimensions({ 0, proposedTop }, viewportSize); + const auto proposedBottom = newView.BottomExclusive(); if (proposedBottom > bufferSize.Y) { - proposedTop -= (proposedBottom - bufferSize.Y); + proposedTop = ::base::saturated_cast(proposedTop - (proposedBottom - bufferSize.Y)); } _mutableViewport = Viewport::FromDimensions({ 0, proposedTop }, viewportSize); + + _buffer.swap(newTextBuffer); + _scrollOffset = 0; _NotifyScrollEvent(); @@ -456,18 +575,15 @@ void Terminal::_WriteBuffer(const std::wstring_view& stringView) // Try the character again. i--; - // Mark the line we're currently on as wrapped + // If we write the last cell of the row here, TextBuffer::Write will + // mark this line as wrapped for us. If the next character we + // process is a newline, the Terminal::CursorLineFeed will unmark + // this line 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/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 3a6a543be07..f8cc24021ad 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -142,6 +142,11 @@ bool Terminal::CursorLineFeed(const bool withReturn) noexcept try { auto cursorPos = _buffer->GetCursor().GetPosition(); + + // since we explicitly just moved down a row, clear the wrap status on the + // row we just came from + _buffer->GetRowByOffset(cursorPos.Y).GetCharRow().SetWrapForced(false); + cursorPos.Y++; if (withReturn) { diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 0c79b9131f2..762f1ff7f9a 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -115,6 +115,9 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final auto pfn = std::bind(&ConptyRoundtripTests::_writeCallback, this, std::placeholders::_1, std::placeholders::_2); _pVtRenderEngine->SetTestCallback(pfn); + // Enable the resize quirk, as the Terminal is going to be reacting as if it's enabled. + _pVtRenderEngine->SetResizeQuirk(true); + // Configure the OutputStateMachine's _pfnFlushToTerminal // Use OutputStateMachineEngine::SetTerminalConnection g.pRender->AddRenderEngine(_pVtRenderEngine.get()); @@ -164,6 +167,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final TEST_METHOD(MoveCursorAtEOL); + TEST_METHOD(TestResizeHeight); + private: bool _writeCallback(const char* const pch, size_t const cch); void _flushFirstFrame(); @@ -504,21 +509,15 @@ void ConptyRoundtripTests::TestExactWrappingWithoutSpaces() hostSm.ProcessString(L"\n"); hostSm.ProcessString(L"1234567890"); - auto verifyBuffer = [&](const TextBuffer& tb, const bool isTerminal) { + auto verifyBuffer = [&](const TextBuffer& tb) { 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()); + VERIFY_IS_FALSE(row0.GetCharRow().WasWrapForced()); const auto& row1 = tb.GetRowByOffset(1); VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced()); @@ -527,7 +526,7 @@ void ConptyRoundtripTests::TestExactWrappingWithoutSpaces() TestUtils::VerifyExpectedString(tb, L"1234567890", { 0, 1 }); }; - verifyBuffer(hostTb, false); + verifyBuffer(hostTb); // First write the first 80 characters from the string expectedOutput.push_back(R"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)"); @@ -540,7 +539,7 @@ void ConptyRoundtripTests::TestExactWrappingWithoutSpaces() expectedOutput.push_back("\x1b[K"); VERIFY_SUCCEEDED(renderer.PaintFrame()); - verifyBuffer(termTb, true); + verifyBuffer(termTb); } void ConptyRoundtripTests::TestExactWrappingWithSpaces() @@ -552,6 +551,7 @@ void ConptyRoundtripTests::TestExactWrappingWithSpaces() 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(); @@ -573,21 +573,15 @@ void ConptyRoundtripTests::TestExactWrappingWithSpaces() hostSm.ProcessString(L" "); hostSm.ProcessString(L"1234567890"); - auto verifyBuffer = [&](const TextBuffer& tb, const bool isTerminal) { + auto verifyBuffer = [&](const TextBuffer& tb) { 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()); + VERIFY_IS_FALSE(row0.GetCharRow().WasWrapForced()); const auto& row1 = tb.GetRowByOffset(1); VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced()); @@ -596,7 +590,7 @@ void ConptyRoundtripTests::TestExactWrappingWithSpaces() TestUtils::VerifyExpectedString(tb, L" 1234567890", { 0, 1 }); }; - verifyBuffer(hostTb, false); + verifyBuffer(hostTb); // First write the first 80 characters from the string expectedOutput.push_back(R"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)"); @@ -609,7 +603,7 @@ void ConptyRoundtripTests::TestExactWrappingWithSpaces() expectedOutput.push_back("\x1b[K"); VERIFY_SUCCEEDED(renderer.PaintFrame()); - verifyBuffer(termTb, true); + verifyBuffer(termTb); } void ConptyRoundtripTests::MoveCursorAtEOL() @@ -622,6 +616,7 @@ void ConptyRoundtripTests::MoveCursorAtEOL() auto& gci = g.getConsoleInformation(); auto& si = gci.GetActiveOutputBuffer(); auto& hostSm = si.GetStateMachine(); + auto& hostTb = si.GetTextBuffer(); auto& termTb = *term->_buffer; _flushFirstFrame(); @@ -675,6 +670,230 @@ void ConptyRoundtripTests::MoveCursorAtEOL() verifyData1(termTb); } +void ConptyRoundtripTests::TestResizeHeight() +{ + // This test class is _60_ tests to ensure that resizing the terminal works + // with conpty correctly. There's a lot of min/maxing in expressions here, + // to account for the sheer number of cases here, and that we have to handle + // both resizing larger and smaller all in one test. + + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method") + TEST_METHOD_PROPERTY(L"Data:dx", L"{-1, 0, 1}") + TEST_METHOD_PROPERTY(L"Data:dy", L"{-10, -1, 0, 1, 10}") + TEST_METHOD_PROPERTY(L"Data:printedRows", L"{1, 10, 50, 200}") + END_TEST_METHOD_PROPERTIES() + int dx, dy; + int printedRows; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"dx", dx), L"change in width of buffer"); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"dy", dy), L"change in height of buffer"); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"printedRows", printedRows), L"Number of rows of text to print"); + + _checkConptyOutput = false; + + 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.get(); + const auto initialHostView = si.GetViewport(); + const auto initialTermView = term->GetViewport(); + const auto initialTerminalBufferHeight = term->GetTextBuffer().GetSize().Height(); + + VERIFY_ARE_EQUAL(0, initialHostView.Top()); + VERIFY_ARE_EQUAL(TerminalViewHeight, initialHostView.BottomExclusive()); + VERIFY_ARE_EQUAL(0, initialTermView.Top()); + VERIFY_ARE_EQUAL(TerminalViewHeight, initialTermView.BottomExclusive()); + + Log::Comment(NoThrowString().Format( + L"Print %d lines of output, which will scroll the viewport", printedRows)); + + for (auto i = 0; i < printedRows; i++) + { + // This looks insane, but this expression is carefully crafted to give + // us only printable characters, starting with `!` (0n33). + // Similar statements are used elsewhere throughout this test. + auto wstr = std::wstring(1, static_cast((i) % 93) + 33); + hostSm.ProcessString(wstr); + hostSm.ProcessString(L"\r\n"); + } + + // Conpty doesn't have a scrollback, it's view's origin is always 0,0 + const auto secondHostView = si.GetViewport(); + VERIFY_ARE_EQUAL(0, secondHostView.Top()); + VERIFY_ARE_EQUAL(TerminalViewHeight, secondHostView.BottomExclusive()); + + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + const auto secondTermView = term->GetViewport(); + // If we've printed more lines than the height of the buffer, then we're + // expecting the viewport to have moved down. Otherwise, the terminal's + // viewport will stay at 0,0. + const auto expectedTerminalViewBottom = std::max(std::min(::base::saturated_cast(printedRows + 1), + term->GetBufferHeight()), + term->GetViewport().Height()); + + VERIFY_ARE_EQUAL(expectedTerminalViewBottom, secondTermView.BottomExclusive()); + VERIFY_ARE_EQUAL(expectedTerminalViewBottom - initialTermView.Height(), secondTermView.Top()); + + auto verifyTermData = [&expectedTerminalViewBottom, &printedRows, this, &initialTerminalBufferHeight](TextBuffer& termTb, const int resizeDy = 0) { + // Some number of lines of text were lost from the scrollback. The + // number of lines lost will be determined by whichever of the initial + // or current buffer is smaller. + const auto numLostRows = std::max(0, + printedRows - std::min(term->GetTextBuffer().GetSize().Height(), initialTerminalBufferHeight) + 1); + + const auto rowsWithText = std::min(::base::saturated_cast(printedRows), + expectedTerminalViewBottom) - + 1 + std::min(resizeDy, 0); + + for (short row = 0; row < rowsWithText; row++) + { + SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures); + auto iter = termTb.GetCellDataAt({ 0, row }); + const wchar_t expectedChar = static_cast((row + numLostRows) % 93) + 33; + + auto expectedString = std::wstring(1, expectedChar); + + if (iter->Chars() != expectedString) + { + Log::Comment(NoThrowString().Format(L"row [%d] was mismatched", row)); + } + VERIFY_ARE_EQUAL(expectedString, (iter++)->Chars()); + VERIFY_ARE_EQUAL(L" ", (iter)->Chars()); + } + }; + auto verifyHostData = [&si, &initialHostView, &printedRows](TextBuffer& hostTb, const int resizeDy = 0) { + const auto hostView = si.GetViewport(); + + // In the host, there are two regions we're interested in: + + // 1. the first section of the buffer with the output in it. Before + // we're resized, this will be filled with one character on each row. + // 2. The second area below the first that's empty (filled with spaces). + // Initially, this is only one row. + // After we resize, different things will happen. + // * If we decrease the height of the buffer, the characters in the + // buffer will all move _up_ the same number of rows. We'll want to + // only check the first initialView+dy rows for characters. + // * If we increase the height, rows will be added at the bottom. We'll + // want to check the initial viewport height for the original + // characters, but then we'll want to look for more blank rows at the + // bottom. The characters in the initial viewport won't have moved. + + const short originalViewHeight = ::base::saturated_cast(resizeDy < 0 ? + initialHostView.Height() + resizeDy : + initialHostView.Height()); + const auto rowsWithText = std::min(originalViewHeight - 1, printedRows); + const bool scrolled = printedRows > initialHostView.Height(); + // The last row of the viewport should be empty + // The second last row will have '0'+50 + // The third last row will have '0'+49 + // ... + // The last row will have '0'+(50-height+1) + const auto firstChar = static_cast(scrolled ? + (printedRows - originalViewHeight + 1) : + 0); + + short row = 0; + // Don't include the last row of the viewport in this check, since it'll + // be blank. We'll check it in the below loop. + for (; row < rowsWithText; row++) + { + SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures); + auto iter = hostTb.GetCellDataAt({ 0, row }); + + const auto expectedChar = static_cast(((firstChar + row) % 93) + 33); + auto expectedString = std::wstring(1, static_cast(expectedChar)); + + if (iter->Chars() != expectedString) + { + Log::Comment(NoThrowString().Format(L"row [%d] was mismatched", row)); + } + VERIFY_ARE_EQUAL(expectedString, (iter++)->Chars(), NoThrowString().Format(L"%s", expectedString.data())); + VERIFY_ARE_EQUAL(L" ", (iter)->Chars()); + } + + // Check that the remaining rows in the viewport are empty. + for (; row < hostView.Height(); row++) + { + SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures); + auto iter = hostTb.GetCellDataAt({ 0, row }); + VERIFY_ARE_EQUAL(L" ", (iter)->Chars()); + } + }; + + verifyHostData(*hostTb); + verifyTermData(*termTb); + + const COORD newViewportSize{ + ::base::saturated_cast(TerminalViewWidth + dx), + ::base::saturated_cast(TerminalViewHeight + dy) + }; + + Log::Comment(NoThrowString().Format(L"Resize the Terminal and conpty here")); + auto resizeResult = term->UserResize(newViewportSize); + VERIFY_SUCCEEDED(resizeResult); + _resizeConpty(newViewportSize.X, newViewportSize.Y); + + // After we resize, make sure to get the new textBuffers + hostTb = &si.GetTextBuffer(); + termTb = term->_buffer.get(); + + // Conpty's doesn't have a scrollback, it's view's origin is always 0,0 + const auto thirdHostView = si.GetViewport(); + VERIFY_ARE_EQUAL(0, thirdHostView.Top()); + VERIFY_ARE_EQUAL(newViewportSize.Y, thirdHostView.BottomExclusive()); + + // The Terminal should be stuck to the top of the viewport, unless dy<0, + // rows=50. In that set of cases, we _didn't_ pin the top of the Terminal to + // the old top, we actually shifted it down (because the output was at the + // bottom of the window, not empty lines). + const auto thirdTermView = term->GetViewport(); + if (dy < 0 && (printedRows > initialTermView.Height() && printedRows < initialTerminalBufferHeight)) + { + VERIFY_ARE_EQUAL(secondTermView.Top() - dy, thirdTermView.Top()); + VERIFY_ARE_EQUAL(expectedTerminalViewBottom, thirdTermView.BottomExclusive()); + } + else + { + VERIFY_ARE_EQUAL(secondTermView.Top(), thirdTermView.Top()); + VERIFY_ARE_EQUAL(expectedTerminalViewBottom + dy, thirdTermView.BottomExclusive()); + } + + verifyHostData(*hostTb, dy); + // Note that at this point, nothing should have changed with the Terminal. + verifyTermData(*termTb, dy); + + Log::Comment(NoThrowString().Format(L"Paint a frame to update the Terminal")); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + // Conpty's doesn't have a scrollback, it's view's origin is always 0,0 + const auto fourthHostView = si.GetViewport(); + VERIFY_ARE_EQUAL(0, fourthHostView.Top()); + VERIFY_ARE_EQUAL(newViewportSize.Y, fourthHostView.BottomExclusive()); + + // The Terminal should be stuck to the top of the viewport, unless dy<0, + // rows=50. In that set of cases, we _didn't_ pin the top of the Terminal to + // the old top, we actually shifted it down (because the output was at the + // bottom of the window, not empty lines). + const auto fourthTermView = term->GetViewport(); + if (dy < 0 && (printedRows > initialTermView.Height() && printedRows < initialTerminalBufferHeight)) + { + VERIFY_ARE_EQUAL(secondTermView.Top() - dy, thirdTermView.Top()); + VERIFY_ARE_EQUAL(expectedTerminalViewBottom, thirdTermView.BottomExclusive()); + } + else + { + VERIFY_ARE_EQUAL(secondTermView.Top(), thirdTermView.Top()); + VERIFY_ARE_EQUAL(expectedTerminalViewBottom + dy, thirdTermView.BottomExclusive()); + } + verifyHostData(*hostTb, dy); + verifyTermData(*termTb, dy); +} + void ConptyRoundtripTests::PassthroughCursorShapeImmediately() { // This is a test for GH#4106, and more indirectly, GH #2011. @@ -715,6 +934,7 @@ void ConptyRoundtripTests::PassthroughClearScrollback() auto& gci = g.getConsoleInformation(); auto& si = gci.GetActiveOutputBuffer(); auto& hostSm = si.GetStateMachine(); + auto& termTb = *term->_buffer; _flushFirstFrame(); diff --git a/src/host/ConsoleArguments.cpp b/src/host/ConsoleArguments.cpp index 8a18f6df288..84daa6734ae 100644 --- a/src/host/ConsoleArguments.cpp +++ b/src/host/ConsoleArguments.cpp @@ -18,6 +18,7 @@ const std::wstring_view ConsoleArguments::FILEPATH_LEADER_PREFIX = L"\\??\\"; const std::wstring_view ConsoleArguments::WIDTH_ARG = L"--width"; const std::wstring_view ConsoleArguments::HEIGHT_ARG = L"--height"; const std::wstring_view ConsoleArguments::INHERIT_CURSOR_ARG = L"--inheritcursor"; +const std::wstring_view ConsoleArguments::RESIZE_QUIRK = L"--resizeQuirk"; const std::wstring_view ConsoleArguments::FEATURE_ARG = L"--feature"; const std::wstring_view ConsoleArguments::FEATURE_PTY_ARG = L"pty"; @@ -479,6 +480,12 @@ void ConsoleArguments::s_ConsumeArg(_Inout_ std::vector& args, _In s_ConsumeArg(args, i); hr = S_OK; } + else if (arg == RESIZE_QUIRK) + { + _resizeQuirk = true; + s_ConsumeArg(args, i); + hr = S_OK; + } else if (arg == CLIENT_COMMANDLINE_ARG) { // Everything after this is the explicit commandline @@ -611,6 +618,10 @@ bool ConsoleArguments::GetInheritCursor() const { return _inheritCursor; } +bool ConsoleArguments::IsResizeQuirkEnabled() const +{ + return _resizeQuirk; +} // Method Description: // - Tell us to use a different size than the one parsed as the size of the diff --git a/src/host/ConsoleArguments.hpp b/src/host/ConsoleArguments.hpp index e63dbd7badf..248248abb70 100644 --- a/src/host/ConsoleArguments.hpp +++ b/src/host/ConsoleArguments.hpp @@ -50,6 +50,7 @@ class ConsoleArguments short GetWidth() const; short GetHeight() const; bool GetInheritCursor() const; + bool IsResizeQuirkEnabled() const; void SetExpectedSize(COORD dimensions) noexcept; @@ -68,6 +69,7 @@ class ConsoleArguments static const std::wstring_view WIDTH_ARG; static const std::wstring_view HEIGHT_ARG; static const std::wstring_view INHERIT_CURSOR_ARG; + static const std::wstring_view RESIZE_QUIRK; static const std::wstring_view FEATURE_ARG; static const std::wstring_view FEATURE_PTY_ARG; @@ -100,6 +102,7 @@ class ConsoleArguments _serverHandle(serverHandle), _signalHandle(signalHandle), _inheritCursor(inheritCursor), + _resizeQuirk(false), _receivedEarlySizeChange{ false }, _originalWidth{ -1 }, _originalHeight{ -1 } @@ -127,6 +130,7 @@ class ConsoleArguments DWORD _serverHandle; DWORD _signalHandle; bool _inheritCursor; + bool _resizeQuirk{ false }; bool _receivedEarlySizeChange; short _originalWidth; diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index 462771bc9de..6743983fadf 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -73,6 +73,7 @@ VtIo::VtIo() : [[nodiscard]] HRESULT VtIo::Initialize(const ConsoleArguments* const pArgs) { _lookingForCursorPosition = pArgs->GetInheritCursor(); + _resizeQuirk = pArgs->IsResizeQuirkEnabled(); // If we were already given VT handles, set up the VT IO engine to use those. if (pArgs->InConptyMode()) @@ -192,6 +193,7 @@ VtIo::VtIo() : if (_pVtRenderEngine) { _pVtRenderEngine->SetTerminalOwner(this); + _pVtRenderEngine->SetResizeQuirk(_resizeQuirk); } } } @@ -446,3 +448,19 @@ void VtIo::EnableConptyModeForTests() _objectsCreated = true; } #endif + +// Method Description: +// - Returns true if the Resize Quirk is enabled. This changes the behavior of +// conpty to _not_ InvalidateAll the entire viewport on a resize operation. +// This is used by the Windows Terminal, because it is prepared to be +// connected to a conpty, and handles it's own buffer specifically for a +// conpty scenario. +// - See also: GH#3490, #4354, #4741 +// Arguments: +// - +// Return Value: +// - true iff we were started with the `--resizeQuirk` flag enabled. +bool VtIo::IsResizeQuirkEnabled() const +{ + return _resizeQuirk; +} diff --git a/src/host/VtIo.hpp b/src/host/VtIo.hpp index b2600c10270..37186ef0c85 100644 --- a/src/host/VtIo.hpp +++ b/src/host/VtIo.hpp @@ -43,6 +43,8 @@ namespace Microsoft::Console::VirtualTerminal void EnableConptyModeForTests(); #endif + bool IsResizeQuirkEnabled() const; + private: // After CreateIoHandlers is called, these will be invalid. wil::unique_hfile _hInput; @@ -57,6 +59,8 @@ namespace Microsoft::Console::VirtualTerminal bool _lookingForCursorPosition; std::mutex _shutdownLock; + bool _resizeQuirk{ false }; + std::unique_ptr _pVtRenderEngine; std::unique_ptr _pVtInputThread; std::unique_ptr _pPtySignalInputThread; diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 85a8a1e4eaa..e55ba97c8f1 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -759,16 +759,16 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont // if we're headless, not so much. However, GetMaxWindowSizeInCharacters // will only return the buffer size, so we can't use that to clip the arg here. // So only clip the requested size if we're not headless + if (g.getConsoleInformation().IsInVtIoMode()) + { + // SetViewportRect doesn't cause the buffer to resize. Manually resize the buffer. + RETURN_IF_NTSTATUS_FAILED(context.ResizeScreenBuffer(Viewport::FromInclusive(Window).Dimensions(), false)); + } if (!g.IsHeadless()) { COORD const coordMax = context.GetMaxWindowSizeInCharacters(); RETURN_HR_IF(E_INVALIDARG, (NewWindowSize.X > coordMax.X || NewWindowSize.Y > coordMax.Y)); } - else if (g.getConsoleInformation().IsInVtIoMode()) - { - // SetViewportRect doesn't cause the buffer to resize. Manually resize the buffer. - RETURN_IF_NTSTATUS_FAILED(context.ResizeScreenBuffer(Viewport::FromInclusive(Window).Dimensions(), false)); - } // Even if it's the same size, we need to post an update in case the scroll bars need to go away. context.SetViewport(Viewport::FromInclusive(Window), true); @@ -776,7 +776,15 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont { // TODO: MSFT: 9574827 - shouldn't we be looking at or at least logging the failure codes here? (Or making them non-void?) context.PostUpdateWindowSize(); - WriteToScreen(context, context.GetViewport()); + + // Use WriteToScreen to invalidate the viewport with the renderer. + // GH#3490 - If we're in conpty mode, don't invalidate the entire + // viewport. In conpty mode, the VtEngine will later decide what + // part of the buffer actually needs to be re-sent to the terminal. + if (!(g.getConsoleInformation().IsInVtIoMode() && g.getConsoleInformation().GetVtIo()->IsResizeQuirkEnabled())) + { + WriteToScreen(context, context.GetViewport()); + } } return S_OK; } diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 477910b59e9..6b9d6ab340f 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1414,7 +1414,9 @@ bool SCREEN_INFORMATION::IsMaximizedY() const // Save cursor's relative height versus the viewport SHORT const sCursorHeightInViewportBefore = _textBuffer->GetCursor().GetPosition().Y - _viewport.Top(); - HRESULT hr = TextBuffer::Reflow(*_textBuffer.get(), *newTextBuffer.get()); + // Reflow requires a optional& , which can't just be done inline with the call. + std::optional unused{ std::nullopt }; + HRESULT hr = TextBuffer::Reflow(*_textBuffer.get(), *newTextBuffer.get(), std::nullopt, unused); if (SUCCEEDED(hr)) { @@ -2176,8 +2178,13 @@ void SCREEN_INFORMATION::SetDefaultAttributes(const TextAttribute& attributes, commandLine.UpdatePopups(attributes, popupAttributes, oldPrimaryAttributes, oldPopupAttributes); } - // force repaint of entire viewport - GetRenderTarget().TriggerRedrawAll(); + // Force repaint of entire viewport, unless we're in conpty mode. In that + // case, we don't really need to force a redraw of the entire screen just + // because the text attributes changed. + if (!(gci.IsInVtIoMode())) + { + GetRenderTarget().TriggerRedrawAll(); + } gci.ConsoleIme.RefreshAreaAttributes(); diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index ec3ac8e33e0..92410495822 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -262,7 +262,6 @@ void VtRendererTest::Xterm256TestInvalidate() Log::Comment(NoThrowString().Format( L"Make sure that invalidating all invalidates the whole viewport.")); VERIFY_SUCCEEDED(engine->InvalidateAll()); - qExpectedInput.push_back("\x1b[2J"); TestPaint(*engine, [&]() { VERIFY_ARE_EQUAL(view, engine->_invalidRect); }); @@ -730,7 +729,6 @@ void VtRendererTest::XtermTestInvalidate() Log::Comment(NoThrowString().Format( L"Make sure that invalidating all invalidates the whole viewport.")); VERIFY_SUCCEEDED(engine->InvalidateAll()); - qExpectedInput.push_back("\x1b[2J"); TestPaint(*engine, [&]() { VERIFY_ARE_EQUAL(view, engine->_invalidRect); }); diff --git a/src/inc/conpty-static.h b/src/inc/conpty-static.h index 7f85b22eddf..a6561b7b30f 100644 --- a/src/inc/conpty-static.h +++ b/src/inc/conpty-static.h @@ -15,6 +15,8 @@ extern "C" { #endif +#define PSEUDOCONSOLE_RESIZE_QUIRK (2u) + HRESULT WINAPI ConptyCreatePseudoConsole(COORD size, HANDLE hInput, HANDLE hOutput, DWORD dwFlags, HPCON* phPC); HRESULT WINAPI ConptyResizePseudoConsole(HPCON hPC, COORD size); diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 3242b5fe390..145d972fdba 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -73,18 +73,6 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, { dirtyView = Viewport::Union(dirtyView, Viewport::FromInclusive(til::at(dirty, i))); } - - // This is expecting the dirty view to be the union of all dirty regions as one big - // rectangle descrbing them all. - if (!_resized && dirtyView == _lastViewport) - { - // TODO: MSFT:21096414 - This is never actually hit. We set - // _resized=true on every frame (see VtEngine::UpdateViewport). - // Unfortunately, not always setting _resized is not a good enough - // solution, see that work item for a description why. - RETURN_IF_FAILED(_ClearScreen()); - _clearedAllThisFrame = true; - } } if (!_quickReturn) @@ -254,6 +242,10 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, _needToDisableCursor = true; hr = _CursorHome(); } + else if (_resized && _resizeQuirk) + { + hr = _CursorPosition(coord); + } else if (coord.X == 0 && coord.Y == (_lastText.Y + 1)) { // Down one line, at the start of the line. diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index 97cacc60447..c83ac780ddd 100644 --- a/src/renderer/vt/state.cpp +++ b/src/renderer/vt/state.cpp @@ -298,6 +298,7 @@ CATCH_RETURN(); { hr = _ResizeWindow(newView.Width(), newView.Height()); } + _resized = true; } // See MSFT:19408543 @@ -309,39 +310,50 @@ CATCH_RETURN(); // lead to the first _actual_ resize being suppressed. _suppressResizeRepaint = false; - if (SUCCEEDED(hr)) + if (_resizeQuirk) { - // Viewport is smaller now - just update it all. - if (oldView.Height() > newView.Height() || oldView.Width() > newView.Width()) - { - hr = InvalidateAll(); - } - else + // GH#3490 - When the viewport width changed, don't do anything extra here. + // If the buffer had areas that were invalid due to the resize, then the + // buffer will have triggered it's own invalidations for what it knows is + // invalid. Previously, we'd invalidate everything if the width changed, + // because we couldn't be sure if lines were reflowed. + } + else + { + if (SUCCEEDED(hr)) { - // At least one of the directions grew. - // First try and add everything to the right of the old viewport, - // then everything below where the old viewport ended. - if (oldView.Width() < newView.Width()) + // Viewport is smaller now - just update it all. + if (oldView.Height() > newView.Height() || oldView.Width() > newView.Width()) { - short left = oldView.RightExclusive(); - short top = 0; - short right = newView.RightInclusive(); - short bottom = oldView.BottomInclusive(); - Viewport rightOfOldViewport = Viewport::FromInclusive({ left, top, right, bottom }); - hr = _InvalidCombine(rightOfOldViewport); + hr = InvalidateAll(); } - if (SUCCEEDED(hr) && oldView.Height() < newView.Height()) + else { - short left = 0; - short top = oldView.BottomExclusive(); - short right = newView.RightInclusive(); - short bottom = newView.BottomInclusive(); - Viewport belowOldViewport = Viewport::FromInclusive({ left, top, right, bottom }); - hr = _InvalidCombine(belowOldViewport); + // At least one of the directions grew. + // First try and add everything to the right of the old viewport, + // then everything below where the old viewport ended. + if (oldView.Width() < newView.Width()) + { + short left = oldView.RightExclusive(); + short top = 0; + short right = newView.RightInclusive(); + short bottom = oldView.BottomInclusive(); + Viewport rightOfOldViewport = Viewport::FromInclusive({ left, top, right, bottom }); + hr = _InvalidCombine(rightOfOldViewport); + } + if (SUCCEEDED(hr) && oldView.Height() < newView.Height()) + { + short left = 0; + short top = oldView.BottomExclusive(); + short right = newView.RightInclusive(); + short bottom = newView.BottomInclusive(); + Viewport belowOldViewport = Viewport::FromInclusive({ left, top, right, bottom }); + hr = _InvalidCombine(belowOldViewport); + } } } } - _resized = true; + return hr; } @@ -487,3 +499,19 @@ void VtEngine::EndResizeRequest() { _inResizeRequest = false; } + +// Method Description: +// - Configure the renderer for the resize quirk. This changes the behavior of +// conpty to _not_ InvalidateAll the entire viewport on a resize operation. +// This is used by the Windows Terminal, because it is prepared to be +// connected to a conpty, and handles it's own buffer specifically for a +// conpty scenario. +// - See also: GH#3490, #4354, #4741 +// Arguments: +// - +// Return Value: +// - true iff we were started with the `--resizeQuirk` flag enabled. +void VtEngine::SetResizeQuirk(const bool resizeQuirk) +{ + _resizeQuirk = resizeQuirk; +} diff --git a/src/renderer/vt/tracing.cpp b/src/renderer/vt/tracing.cpp index 8246e6d920e..d9698a332e2 100644 --- a/src/renderer/vt/tracing.cpp +++ b/src/renderer/vt/tracing.cpp @@ -228,17 +228,20 @@ void RenderTracing::TraceLastText(const COORD lastTextPos) const void RenderTracing::TraceMoveCursor(const COORD lastTextPos, const COORD cursor) const { #ifndef UNIT_TESTING - const auto lastTextStr = _CoordToString(lastTextPos); - const auto lastText = lastTextStr.c_str(); + if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + const auto lastTextStr = _CoordToString(lastTextPos); + const auto lastText = lastTextStr.c_str(); - const auto cursorStr = _CoordToString(cursor); - const auto cursorPos = cursorStr.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)); + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceMoveCursor", + TraceLoggingString(lastText), + TraceLoggingString(cursorPos), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } #else UNREFERENCED_PARAMETER(lastTextPos); UNREFERENCED_PARAMETER(cursor); @@ -248,11 +251,14 @@ void RenderTracing::TraceMoveCursor(const COORD lastTextPos, const COORD cursor) 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)); + if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + const auto* const msg = "Wrapped instead of \\r\\n"; + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceWrapped", + TraceLoggingString(msg), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } #else #endif UNIT_TESTING } @@ -260,12 +266,15 @@ void RenderTracing::TraceWrapped() const 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)); + if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + 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/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index be386680edc..8ce51bbcf1c 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -106,6 +106,8 @@ namespace Microsoft::Console::Render void BeginResizeRequest(); void EndResizeRequest(); + void SetResizeQuirk(const bool resizeQuirk); + protected: wil::unique_hfile _hFile; std::string _buffer; @@ -151,6 +153,8 @@ namespace Microsoft::Console::Render bool _delayedEolWrap{ false }; + bool _resizeQuirk{ false }; + [[nodiscard]] HRESULT _Write(std::string_view const str) noexcept; [[nodiscard]] HRESULT _WriteFormattedString(const std::string* const pFormat, ...) noexcept; [[nodiscard]] HRESULT _Flush() noexcept; diff --git a/src/winconpty/winconpty.cpp b/src/winconpty/winconpty.cpp index be440ce00e3..ed250b55a6e 100644 --- a/src/winconpty/winconpty.cpp +++ b/src/winconpty/winconpty.cpp @@ -83,15 +83,17 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken, RETURN_IF_WIN32_BOOL_FALSE(SetHandleInformation(signalPipeConhostSide.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT)); // GH4061: Ensure that the path to executable in the format is escaped so C:\Program.exe cannot collide with C:\Program Files - const wchar_t* pwszFormat = L"\"%s\" --headless %s--width %hu --height %hu --signal 0x%x --server 0x%x"; + const wchar_t* pwszFormat = L"\"%s\" --headless %s%s--width %hu --height %hu --signal 0x%x --server 0x%x"; // This is plenty of space to hold the formatted string wchar_t cmd[MAX_PATH]{}; const BOOL bInheritCursor = (dwFlags & PSEUDOCONSOLE_INHERIT_CURSOR) == PSEUDOCONSOLE_INHERIT_CURSOR; + const BOOL bResizeQuirk = (dwFlags & PSEUDOCONSOLE_RESIZE_QUIRK) == PSEUDOCONSOLE_RESIZE_QUIRK; swprintf_s(cmd, MAX_PATH, pwszFormat, _ConsoleHostPath(), bInheritCursor ? L"--inheritcursor " : L"", + bResizeQuirk ? L"--resizeQuirk " : L"", size.X, size.Y, signalPipeConhostSide.get(), diff --git a/src/winconpty/winconpty.h b/src/winconpty/winconpty.h index 829befc5473..53c2279a3f2 100644 --- a/src/winconpty/winconpty.h +++ b/src/winconpty/winconpty.h @@ -19,6 +19,11 @@ typedef struct _PseudoConsole // the signal pipe. #define PTY_SIGNAL_RESIZE_WINDOW (8u) +// CreatePseudoConsole Flags +// The other flag (PSEUDOCONSOLE_INHERIT_CURSOR) is actually defined in consoleapi.h in the OS repo +// #define PSEUDOCONSOLE_INHERIT_CURSOR (0x1) +#define PSEUDOCONSOLE_RESIZE_QUIRK (0x2) + // Implementations of the various PseudoConsole functions. HRESULT _CreatePseudoConsole(const HANDLE hToken, const COORD size,