diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index f71ffb76472..507615b9dde 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -176,14 +176,40 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting return S_FALSE; } + const auto dy = viewportSize.Y - oldDimensions.Y; + + // We're going to attempt to "stick to the top" of where the old viewport was. 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); + // However 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. + const COORD cOldCursorPos = _buffer->GetCursor().GetPosition(); + COORD cOldLastChar = cOldCursorPos; + try + { + cOldLastChar = _buffer->GetLastNonSpaceCharacter(); + } + CATCH_LOG(); + + const auto maxRow = std::max(cOldLastChar.Y, cOldCursorPos.Y); + + const bool beforeLastRow = maxRow < bufferSize.Y - 1; + const auto adjustment = beforeLastRow ? 0 : std::max(0, -dy); + + auto proposedTop = oldTop + adjustment; + + const auto newView = Viewport::FromDimensions({ 0, gsl::narrow_cast(proposedTop) }, viewportSize); const auto proposedBottom = newView.BottomExclusive(); // 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. @@ -192,7 +218,8 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting proposedTop -= (proposedBottom - bufferSize.Y); } - _mutableViewport = Viewport::FromDimensions({ 0, proposedTop }, viewportSize); + _mutableViewport = Viewport::FromDimensions({ 0, gsl::narrow_cast(proposedTop) }, viewportSize); + _scrollOffset = 0; _NotifyScrollEvent(); diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 0980b51393a..bf487622313 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -30,7 +30,11 @@ namespace Microsoft::Terminal::Core // fwdecl unittest classes #ifdef UNIT_TESTING -class ConptyRoundtripTests; +namespace TerminalCoreUnitTests +{ + class TerminalBufferTests; + class ConptyRoundtripTests; +}; #endif class Microsoft::Terminal::Core::Terminal final : @@ -252,6 +256,7 @@ class Microsoft::Terminal::Core::Terminal final : #pragma endregion #ifdef UNIT_TESTING - friend class ::ConptyRoundtripTests; + friend class TerminalCoreUnitTests::TerminalBufferTests; + friend class TerminalCoreUnitTests::ConptyRoundtripTests; #endif }; diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index badc0548e9b..864d451c0cb 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -40,8 +40,17 @@ using namespace Microsoft::Console::Types; using namespace Microsoft::Terminal::Core; -class ConptyRoundtripTests +namespace TerminalCoreUnitTests { + class TerminalBufferTests; +}; +using namespace TerminalCoreUnitTests; + +class TerminalCoreUnitTests::ConptyRoundtripTests final +{ + static const SHORT TerminalViewWidth = 80; + static const SHORT TerminalViewHeight = 32; + TEST_CLASS(ConptyRoundtripTests); TEST_CLASS_SETUP(ClassSetup) @@ -50,7 +59,7 @@ class ConptyRoundtripTests m_state->InitEvents(); m_state->PrepareGlobalFont(); - m_state->PrepareGlobalScreenBuffer(); + m_state->PrepareGlobalScreenBuffer(TerminalViewWidth, TerminalViewHeight, TerminalViewWidth, TerminalViewHeight); m_state->PrepareGlobalInputBuffer(); return true; @@ -71,18 +80,19 @@ class ConptyRoundtripTests { // STEP 1: Set up the Terminal term = std::make_unique(); - term->Create({ CommonState::s_csBufferWidth, CommonState::s_csBufferHeight }, 0, emptyRT); + term->Create({ TerminalViewWidth, TerminalViewHeight }, 100, emptyRT); // STEP 2: Set up the Conpty // Set up some sane defaults auto& g = ServiceLocator::LocateGlobals(); auto& gci = g.getConsoleInformation(); + gci.SetDefaultForegroundColor(INVALID_COLOR); 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()); @@ -106,6 +116,13 @@ class ConptyRoundtripTests g.pRender->AddRenderEngine(_pVtRenderEngine.get()); gci.GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get()); + _pConApi = std::make_unique(gci); + + // Manually set the console into conpty mode. We're not actually going + // to set up the pipes for conpty, but we want the console to behave + // like it would in conpty mode. + g.EnableConptyModeForTests(); + expectedOutput.clear(); return true; @@ -129,13 +146,20 @@ class ConptyRoundtripTests TEST_METHOD(SimpleWriteOutputTest); TEST_METHOD(WriteTwoLinesUsesNewline); TEST_METHOD(WriteAFewSimpleLines); + TEST_METHOD(TestResizeHeight); private: bool _writeCallback(const char* const pch, size_t const cch); void _flushFirstFrame(); + void _resizeConpty(const unsigned short sx, const unsigned short sy); std::deque expectedOutput; std::unique_ptr _pVtRenderEngine; std::unique_ptr m_state; + std::unique_ptr _pConApi; + + // Tests can set these variables how they link to configure the behavior of the test harness. + bool _checkConptyOutput{ true }; // If true, the test class will check that the output from conpty was expected + bool _logConpty{ false }; // If true, the test class will log all the output from conpty. Helpful for debugging. DummyRenderTarget emptyRT; std::unique_ptr term; @@ -144,18 +168,27 @@ class ConptyRoundtripTests bool ConptyRoundtripTests::_writeCallback(const char* const pch, size_t const cch) { std::string actualString = std::string(pch, cch); - VERIFY_IS_GREATER_THAN(expectedOutput.size(), - static_cast(0), - NoThrowString().Format(L"writing=\"%hs\", expecting %u strings", actualString.c_str(), expectedOutput.size())); - std::string first = expectedOutput.front(); - expectedOutput.pop_front(); + if (_checkConptyOutput) + { + VERIFY_IS_GREATER_THAN(expectedOutput.size(), + static_cast(0), + NoThrowString().Format(L"writing=\"%hs\", expecting %u strings", actualString.c_str(), expectedOutput.size())); + + std::string first = expectedOutput.front(); + expectedOutput.pop_front(); - Log::Comment(NoThrowString().Format(L"Expected =\t\"%hs\"", first.c_str())); - Log::Comment(NoThrowString().Format(L"Actual =\t\"%hs\"", actualString.c_str())); + Log::Comment(NoThrowString().Format(L"Expected =\t\"%hs\"", first.c_str())); + Log::Comment(NoThrowString().Format(L"Actual =\t\"%hs\"", actualString.c_str())); - VERIFY_ARE_EQUAL(first.length(), cch); - VERIFY_ARE_EQUAL(first, actualString); + VERIFY_ARE_EQUAL(first.length(), cch); + VERIFY_ARE_EQUAL(first, actualString); + } + else if (_logConpty) + { + Log::Comment(NoThrowString().Format( + L"Writing \"%hs\" to Terminal", actualString.c_str())); + } // Write the string back to our Terminal const auto converted = ConvertToW(CP_UTF8, actualString); @@ -177,6 +210,19 @@ void ConptyRoundtripTests::_flushFirstFrame() VERIFY_SUCCEEDED(renderer.PaintFrame()); } +void ConptyRoundtripTests::_resizeConpty(const unsigned short sx, + const unsigned short sy) +{ + // Largely taken from implementation in PtySignalInputThread::_InputThread + if (DispatchCommon::s_ResizeWindow(*_pConApi, sx, sy)) + { + // Instead of going through the VtIo to suppress the resize repaint, + // just call the method directly on the renderer. This is implemented in + // VtIo::SuppressResizeRepaint + VERIFY_SUCCEEDED(_pVtRenderEngine->SuppressResizeRepaint()); + } +} + // 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 @@ -364,3 +410,208 @@ void ConptyRoundtripTests::WriteAFewSimpleLines() verifyData(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(gsl::narrow_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(gsl::narrow_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 = gsl::narrow_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{ + gsl::narrow_cast(TerminalViewWidth + dx), + gsl::narrow_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. + const auto thirdTermView = term->GetViewport(); + + 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. + const auto fourthTermView = term->GetViewport(); + + VERIFY_ARE_EQUAL(secondTermView.Top(), fourthTermView.Top()); + VERIFY_ARE_EQUAL(expectedTerminalViewBottom + dy, fourthTermView.BottomExclusive()); + + verifyHostData(*hostTb, dy); + verifyTermData(*termTb, dy); +} diff --git a/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp b/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp new file mode 100644 index 00000000000..9734c4062d0 --- /dev/null +++ b/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp @@ -0,0 +1,108 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" +#include + +#include "../renderer/inc/DummyRenderTarget.hpp" +#include "../cascadia/TerminalCore/Terminal.hpp" +#include "MockTermSettings.h" +#include "consoletaeftemplates.hpp" + +using namespace winrt::Microsoft::Terminal::Settings; +using namespace Microsoft::Terminal::Core; + +using namespace WEX::Common; +using namespace WEX::Logging; +using namespace WEX::TestExecution; + +namespace TerminalCoreUnitTests +{ + class TerminalBufferTests; +}; +using namespace TerminalCoreUnitTests; + +class TerminalCoreUnitTests::TerminalBufferTests final +{ + TEST_CLASS(TerminalBufferTests); + + TEST_METHOD(TestResizeHeight); + + TEST_METHOD_SETUP(MethodSetup) + { + // STEP 1: Set up the Terminal + term = std::make_unique(); + term->Create({ 80, 32 }, 100, emptyRT); + return true; + } + + TEST_METHOD_CLEANUP(MethodCleanup) + { + term = nullptr; + return true; + } + +private: + DummyRenderTarget emptyRT; + std::unique_ptr term; +}; + +void TerminalBufferTests::TestResizeHeight() +{ + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method") + TEST_METHOD_PROPERTY(L"Data:dy", L"{-10, -1, 0, 1, 10}") + END_TEST_METHOD_PROPERTIES() + int dy; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"dy", dy), L"change in height of buffer"); + + auto& termTb = *term->_buffer; + auto& termSm = *term->_stateMachine; + const auto initialView = term->GetViewport(); + + VERIFY_ARE_EQUAL(0, initialView.Top()); + VERIFY_ARE_EQUAL(32, initialView.BottomExclusive()); + + Log::Comment(NoThrowString().Format( + L"Print 50 lines of output, which will scroll the viewport")); + + for (auto i = 0; i < 50; i++) + { + auto wstr = std::wstring(1, static_cast(L'0' + i)); + termSm.ProcessString(wstr); + termSm.ProcessString(L"\r\n"); + } + + const auto secondView = term->GetViewport(); + + VERIFY_ARE_EQUAL(50 - initialView.Height() + 1, secondView.Top()); + VERIFY_ARE_EQUAL(50, secondView.BottomInclusive()); + + auto verifyBufferContents = [&termTb]() { + for (short row = 0; row < 50; row++) + { + SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures); + auto iter = termTb.GetCellDataAt({ 0, row }); + auto expectedString = std::wstring(1, static_cast(L'0' + row)); + + 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()); + } + }; + verifyBufferContents(); + + auto resizeResult = term->UserResize({ 80, gsl::narrow_cast(32 + dy) }); + VERIFY_SUCCEEDED(resizeResult); + + const auto thirdView = term->GetViewport(); + + VERIFY_ARE_EQUAL(secondView.Top(), thirdView.Top()); + VERIFY_ARE_EQUAL(50 + dy, thirdView.BottomInclusive()); + // VERIFY_ARE_EQUAL(50 - thirdView.Height() + 1, thirdView.Top()); + + verifyBufferContents(); +} diff --git a/src/cascadia/UnitTests_TerminalCore/UnitTests.vcxproj b/src/cascadia/UnitTests_TerminalCore/UnitTests.vcxproj index abb6ec70d7b..3b60e6b222c 100644 --- a/src/cascadia/UnitTests_TerminalCore/UnitTests.vcxproj +++ b/src/cascadia/UnitTests_TerminalCore/UnitTests.vcxproj @@ -11,6 +11,7 @@ + diff --git a/src/host/ConsoleArguments.cpp b/src/host/ConsoleArguments.cpp index c6e5e12c22e..b33256a84aa 100644 --- a/src/host/ConsoleArguments.cpp +++ b/src/host/ConsoleArguments.cpp @@ -638,3 +638,17 @@ void ConsoleArguments::SetExpectedSize(COORD dimensions) noexcept _recievedEarlySizeChange = true; } } + +#ifdef UNIT_TESTING +// Method Description: +// - This is a test helper method. It can be used to trick us into thinking +// we're headless (in conpty mode), even without parsing any arguments. +// Arguments: +// - +// Return Value: +// - +void ConsoleArguments::EnableConptyModeForTests() +{ + _headless = true; +} +#endif diff --git a/src/host/ConsoleArguments.hpp b/src/host/ConsoleArguments.hpp index 9a903ea18df..8b0daeb7d68 100644 --- a/src/host/ConsoleArguments.hpp +++ b/src/host/ConsoleArguments.hpp @@ -53,6 +53,10 @@ class ConsoleArguments void SetExpectedSize(COORD dimensions) noexcept; +#ifdef UNIT_TESTING + void EnableConptyModeForTests(); +#endif + static const std::wstring_view VT_MODE_ARG; static const std::wstring_view HEADLESS_ARG; static const std::wstring_view SERVER_HANDLE_ARG; diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index cf7ab1ecbe5..3c446e6c925 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -206,6 +206,21 @@ bool VtIo::IsUsingVt() const return _objectsCreated; } +#ifdef UNIT_TESTING +// Method Description: +// - This is a test helper method. It can be used to trick VtIo into responding +// true to `IsUsingVt`, which will cause the console host to act in conpty +// mode. +// Arguments: +// - +// Return Value: +// - +void VtIo::EnableConptyModeForTests() +{ + _objectsCreated = true; +} +#endif + // Routine Description: // Potentially starts this VtIo's input thread and render engine. // If the VtIo hasn't yet been given pipes, then this function will diff --git a/src/host/VtIo.hpp b/src/host/VtIo.hpp index 0516d9ab0ea..b2600c10270 100644 --- a/src/host/VtIo.hpp +++ b/src/host/VtIo.hpp @@ -39,6 +39,10 @@ namespace Microsoft::Console::VirtualTerminal void BeginResize(); void EndResize(); +#ifdef UNIT_TESTING + void EnableConptyModeForTests(); +#endif + private: // After CreateIoHandlers is called, these will be invalid. wil::unique_hfile _hInput; diff --git a/src/host/getset.cpp b/src/host/getset.cpp index eaff59cb3e8..380b4a76033 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -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()) + { + WriteToScreen(context, context.GetViewport()); + } } return S_OK; } diff --git a/src/host/globals.cpp b/src/host/globals.cpp index b9f82175cb4..f39a0d106ca 100644 --- a/src/host/globals.cpp +++ b/src/host/globals.cpp @@ -16,3 +16,19 @@ bool Globals::IsHeadless() const { return launchArgs.IsHeadless(); } + +#ifdef UNIT_TESTING +// Method Description: +// - This is a test helper method. It can be used to trick us into responding +// true to `IsHeadless`, which will cause the console host to act in conpty +// mode. +// Arguments: +// - +// Return Value: +// - +void Globals::EnableConptyModeForTests() +{ + launchArgs.EnableConptyModeForTests(); + getConsoleInformation().GetVtIo()->EnableConptyModeForTests(); +} +#endif diff --git a/src/host/globals.h b/src/host/globals.h index 459bd3689e7..bd320d90d0f 100644 --- a/src/host/globals.h +++ b/src/host/globals.h @@ -69,6 +69,10 @@ class Globals ApiRoutines api; +#ifdef UNIT_TESTING + void EnableConptyModeForTests(); +#endif + private: CONSOLE_INFORMATION ciConsoleInformation; }; diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index e5740bdce40..0471c2bda07 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1423,22 +1423,51 @@ bool SCREEN_INFORMATION::IsMaximizedY() const } // Save cursor's relative height versus the viewport - SHORT const sCursorHeightInViewportBefore = _textBuffer->GetCursor().GetPosition().Y - _viewport.Top(); + short cursorHeightInViewportBefore = _textBuffer->GetCursor().GetPosition().Y - _viewport.Top(); + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + const bool isConpty = gci.IsInVtIoMode(); HRESULT hr = TextBuffer::Reflow(*_textBuffer.get(), *newTextBuffer.get()); + const bool widthChanged = coordNewScreenSize.X != _textBuffer->GetSize().Width(); if (SUCCEEDED(hr)) { Cursor& newCursor = newTextBuffer->GetCursor(); // Adjust the viewport so the cursor doesn't wildly fly off up or down. - SHORT const sCursorHeightInViewportAfter = newCursor.GetPosition().Y - _viewport.Top(); + const short cursorHeightInViewportAfter = newCursor.GetPosition().Y - _viewport.Top(); COORD coordCursorHeightDiff = { 0 }; - coordCursorHeightDiff.Y = sCursorHeightInViewportAfter - sCursorHeightInViewportBefore; + coordCursorHeightDiff.Y = cursorHeightInViewportAfter - cursorHeightInViewportBefore; LOG_IF_FAILED(SetViewportOrigin(false, coordCursorHeightDiff, true)); _textBuffer.swap(newTextBuffer); } + // GH#3490 - In conpty mode, We want to invalidate all of the viewport that + // might have been below any wrapped lines, up until the last character of + // the buffer. Lines that were wrapped may have been re-wrapped during this + // resize, so we want them repainted to the terminal. We don't want to just + // invalidate everything though - if there were blank lines at the bottom, + // those can just be ignored. + if (isConpty && widthChanged) + { + // Loop through all the rows of the old buffer and reprint them into the new buffer + const auto bottom = std::max(_textBuffer->GetCursor().GetPosition().Y, + std::min(_viewport.BottomInclusive(), + _textBuffer->GetLastNonSpaceCharacter().Y)); + bool foundWrappedLine = false; + for (short y = _viewport.Top(); y <= bottom; y++) + { + // Fetch the row and its "right" which is the last printable character. + const ROW& row = _textBuffer->GetRowByOffset(y); + const CharRow& charRow = row.GetCharRow(); + if (foundWrappedLine || charRow.WasWrapForced()) + { + foundWrappedLine = true; + _renderTarget.TriggerRedraw(Viewport::FromDimensions({ 0, y }, _viewport.Width(), 1)); + } + } + } + return NTSTATUS_FROM_HRESULT(hr); } @@ -2176,8 +2205,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/screenInfo.hpp b/src/host/screenInfo.hpp index eb257081227..1e8eb3c75e5 100644 --- a/src/host/screenInfo.hpp +++ b/src/host/screenInfo.hpp @@ -50,6 +50,14 @@ Revision History: #include "../types/IConsoleWindow.hpp" class ConversionAreaInfo; // forward decl window. circular reference +// fwdecl unittest classes +#ifdef UNIT_TESTING +namespace TerminalCoreUnitTests +{ + class ConptyRoundtripTests; +}; +#endif + class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console::IIoProvider { public: @@ -308,6 +316,6 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console friend class ScreenBufferTests; friend class CommonState; friend class ConptyOutputTests; - friend class ConptyRoundtripTests; + friend class TerminalCoreUnitTests::ConptyRoundtripTests; #endif }; diff --git a/src/host/ut_host/ConptyOutputTests.cpp b/src/host/ut_host/ConptyOutputTests.cpp index 1bc6a189900..1fcfa484e18 100644 --- a/src/host/ut_host/ConptyOutputTests.cpp +++ b/src/host/ut_host/ConptyOutputTests.cpp @@ -85,6 +85,11 @@ class ConptyOutputTests g.pRender->AddRenderEngine(_pVtRenderEngine.get()); gci.GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get()); + // Manually set the console into conpty mode. We're not actually going + // to set up the pipes for conpty, but we want the console to behave + // like it would in conpty mode. + g.EnableConptyModeForTests(); + expectedOutput.clear(); return true; diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index de4767efed6..aeba3a243c1 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -1343,15 +1343,15 @@ void VtRendererTest::TestResize() VERIFY_IS_FALSE(engine->_suppressResizeRepaint); }); - // Resize the viewport to 120x30 - // Everything should be invalidated, and a resize message sent. + // Resize the viewport to 120x30. A resize message should be sent. + // GH#3490: Not everything will be invalidated here. The console host will + // determine which lines need to be invalidated for us. const auto newView = Viewport::FromDimensions({ 0, 0 }, { 120, 30 }); qExpectedInput.push_back("\x1b[8;30;120t"); VERIFY_SUCCEEDED(engine->UpdateViewport(newView.ToInclusive())); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(newView, engine->_invalidRect); VERIFY_IS_FALSE(engine->_firstPaint); VERIFY_IS_FALSE(engine->_suppressResizeRepaint); }); diff --git a/src/inc/test/CommonState.hpp b/src/inc/test/CommonState.hpp index 85dfeb960c1..7d1fbd7f4ea 100644 --- a/src/inc/test/CommonState.hpp +++ b/src/inc/test/CommonState.hpp @@ -81,16 +81,19 @@ class CommonState } } - void PrepareGlobalScreenBuffer() + void PrepareGlobalScreenBuffer(const short viewWidth = s_csWindowWidth, + const short viewHeight = s_csWindowHeight, + const short bufferWidth = s_csBufferWidth, + const short bufferHeight = s_csBufferHeight) { CONSOLE_INFORMATION& gci = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals().getConsoleInformation(); COORD coordWindowSize; - coordWindowSize.X = s_csWindowWidth; - coordWindowSize.Y = s_csWindowHeight; + coordWindowSize.X = viewWidth; + coordWindowSize.Y = viewHeight; COORD coordScreenBufferSize; - coordScreenBufferSize.X = s_csBufferWidth; - coordScreenBufferSize.Y = s_csBufferHeight; + coordScreenBufferSize.X = bufferWidth; + coordScreenBufferSize.Y = bufferHeight; UINT uiCursorSize = 12; @@ -143,12 +146,14 @@ class CommonState gci.SetCookedReadData(nullptr); } - void PrepareNewTextBufferInfo(const bool useDefaultAttributes = false) + void PrepareNewTextBufferInfo(const bool useDefaultAttributes = false, + const short bufferWidth = s_csBufferWidth, + const short bufferHeight = s_csBufferHeight) { CONSOLE_INFORMATION& gci = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals().getConsoleInformation(); COORD coordScreenBufferSize; - coordScreenBufferSize.X = s_csBufferWidth; - coordScreenBufferSize.Y = s_csBufferHeight; + coordScreenBufferSize.X = bufferWidth; + coordScreenBufferSize.Y = bufferHeight; UINT uiCursorSize = 12; diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index acb014f12e5..771102c7e4b 100644 --- a/src/renderer/vt/state.cpp +++ b/src/renderer/vt/state.cpp @@ -37,7 +37,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe, _lastViewport(initialViewport), _invalidRect(Viewport::Empty()), _fInvalidRectUsed(false), - _lastRealCursor({ 0 }), _lastText({ 0 }), _scrollDelta({ 0 }), _quickReturn(false), @@ -278,38 +277,12 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe, // lead to the first _actual_ resize being suppressed. _suppressResizeRepaint = false; - if (SUCCEEDED(hr)) - { - // Viewport is smaller now - just update it all. - if (oldView.Height() > newView.Height() || oldView.Width() > newView.Width()) - { - hr = InvalidateAll(); - } - else - { - // 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); - } - } - } + // 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. + _resized = true; return hr; } diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index cc6fed764c5..f9c452ca291 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -26,7 +26,10 @@ Author(s): // fwdecl unittest classes #ifdef UNIT_TESTING -class ConptyRoundtripTests; +namespace TerminalCoreUnitTests +{ + class ConptyRoundtripTests; +}; #endif namespace Microsoft::Console::Render @@ -116,7 +119,6 @@ namespace Microsoft::Console::Render Microsoft::Console::Types::Viewport _invalidRect; bool _fInvalidRectUsed; - COORD _lastRealCursor; COORD _lastText; COORD _scrollDelta; @@ -226,7 +228,7 @@ namespace Microsoft::Console::Render friend class VtRendererTest; friend class ConptyOutputTests; - friend class ConptyRoundtripTests; + friend class TerminalCoreUnitTests::ConptyRoundtripTests; #endif void SetTestCallback(_In_ std::function pfn);