From 8c614812134664a2c43d2a6b968c15c5177133b4 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 24 Jan 2020 13:05:23 -0600 Subject: [PATCH] I've got a crazy idea, we dont need to invalidate the entire view, just the lines that were below wrapped lines? --- src/cascadia/TerminalCore/Terminal.cpp | 3 +- .../ConptyRoundtripTests.cpp | 39 +++++++++------ src/host/screenInfo.cpp | 25 +++++++++- src/renderer/vt/state.cpp | 48 +++++++++---------- 4 files changed, 72 insertions(+), 43 deletions(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 049dacafd6..67b5948fed 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -198,8 +198,7 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting const auto maxRow = std::max(cOldLastChar.Y, cOldCursorPos.Y); - const bool beforeLastRow = maxRow < bufferSize.Y - 1; // -1? 0? - // const bool beforeLastRow = maxRow < bufferSize.Y; // -1? 0? + const bool beforeLastRow = maxRow < bufferSize.Y - 1; const auto adjustment = beforeLastRow ? 0 : std::max(0, -dy); auto proposedTop = oldTop + adjustment; diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 6a7a5371e7..d7f587b849 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -413,15 +413,16 @@ void ConptyRoundtripTests::WriteAFewSimpleLines() 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:dx", L"{0}") TEST_METHOD_PROPERTY(L"Data:dy", L"{-10, -1, 0, 1, 10}") - // TEST_METHOD_PROPERTY(L"Data:dy", L"{-1, 0, 1}") - // TEST_METHOD_PROPERTY(L"Data:dy", L"{-1, 1}") TEST_METHOD_PROPERTY(L"Data:printedRows", L"{1, 10, 50, 200}") - // TEST_METHOD_PROPERTY(L"Data:printedRows", L"{1, 10, 50}") END_TEST_METHOD_PROPERTIES() int dx, dy; int printedRows; @@ -452,13 +453,15 @@ void ConptyRoundtripTests::TestResizeHeight() 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); - // Log::Comment(NoThrowString().Format(L"Writing \"%s\\r\\n\" to conpty", wstr.c_str())); hostSm.ProcessString(wstr); hostSm.ProcessString(L"\r\n"); } - // Conpty's doesn't have a scrollback, it's view's origin is always 0,0 + // 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()); @@ -466,20 +469,20 @@ void ConptyRoundtripTests::TestResizeHeight() 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(std::max(0, expectedTerminalViewBottom - initialTermView.Height() + 1), secondTermView.Top()); VERIFY_ARE_EQUAL(expectedTerminalViewBottom, secondTermView.BottomExclusive()); - // VERIFY_ARE_EQUAL(expectedTerminalViewBottom - initialTermView.Height() + 1, secondTermView.Top()); VERIFY_ARE_EQUAL(expectedTerminalViewBottom - initialTermView.Height(), secondTermView.Top()); auto verifyTermData = [&expectedTerminalViewBottom, &printedRows, this, &initialTerminalBufferHeight](TextBuffer& termTb, const int resizeDy = 0) { - // for (short row = 0; row < expectedTerminalViewBottom; row++) - short row = 0; - // const auto numLostRows = std::max(0, printedRows - term->GetBufferHeight() + 1); - // const auto numLostRows = std::max(0, printedRows - initialTerminalBufferHeight + 1); + // 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); @@ -487,9 +490,9 @@ void ConptyRoundtripTests::TestResizeHeight() expectedTerminalViewBottom) - 1 + std::min(resizeDy, 0); - for (; row < rowsWithText; row++) + for (short row = 0; row < rowsWithText; row++) { - SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures); + // SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures); auto iter = termTb.GetCellDataAt({ 0, row }); const wchar_t expectedChar = static_cast((row + numLostRows) % 93) + 33; @@ -540,6 +543,7 @@ void ConptyRoundtripTests::TestResizeHeight() // 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); @@ -556,11 +560,12 @@ void ConptyRoundtripTests::TestResizeHeight() // 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()); } }; - // DebugBreak(); + verifyHostData(*hostTb); verifyTermData(*termTb); @@ -606,4 +611,8 @@ void ConptyRoundtripTests::TestResizeHeight() VERIFY_ARE_EQUAL(secondTermView.Top(), fourthTermView.Top()); VERIFY_ARE_EQUAL(expectedTerminalViewBottom + dy, fourthTermView.BottomExclusive()); + + verifyHostData(*hostTb, dy); + // Note that at this point, nothing should have changed with the Terminal. + verifyTermData(*termTb, dy); } diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index e9ad04689d..815e1f808c 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1425,8 +1425,8 @@ bool SCREEN_INFORMATION::IsMaximizedY() const // Save cursor's relative height versus the viewport short cursorHeightInViewportBefore = _textBuffer->GetCursor().GetPosition().Y - _viewport.Top(); - // auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - // const bool isConpty = gci.IsInVtIoMode(); + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + const bool isConpty = gci.IsInVtIoMode(); HRESULT hr = TextBuffer::Reflow(*_textBuffer.get(), *newTextBuffer.get(), false, false); if (SUCCEEDED(hr)) @@ -1460,6 +1460,27 @@ bool SCREEN_INFORMATION::IsMaximizedY() const _textBuffer.swap(newTextBuffer); } + if (isConpty) + { + // Loop through all the rows of the old buffer and reprint them into the new buffer + // for (short iOldRow = 0; iOldRow < cOldRowsTotal; iOldRow++) + 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); } diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index 705dd70024..a8624122df 100644 --- a/src/renderer/vt/state.cpp +++ b/src/renderer/vt/state.cpp @@ -279,30 +279,30 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe, if (SUCCEEDED(hr)) { - if (oldView.Width() != newView.Width()) - { - // Viewport is a different width now - just update it all. We may - // have re-wrapped the buffer contents. - hr = InvalidateAll(); - } - else if (oldView.Height() < newView.Height()) - { - // // We grew in height. We inserted empty lines at the top of the - // // buffer. The cursor is now _actually_ lower (larger Y/row value) - // // than it was before. - // _lastText.Y += gsl::narrow_cast(newView.Height() - oldView.Height()); - // // The text content will try and stay "stuck" at the bottom of the - // // viewport of the terminal, and invalidating the bottom here can - // // cause unnecessary lines to get written to the terminal. See - // // GH#3490. - } - else if (oldView.Height() > newView.Height()) - { - // We shrunk in height. We don't really need to do anything here. - // Shrinking in height will remove lines from the top of the buffer - // (pushing them into scrollback in the terminal). - // The cursor will stay in the same relative position. - } + // if (oldView.Width() != newView.Width()) + // { + // // Viewport is a different width now - just update it all. We may + // // have re-wrapped the buffer contents. + // hr = InvalidateAll(); + // } + // else if (oldView.Height() < newView.Height()) + // { + // // // We grew in height. We inserted empty lines at the top of the + // // // buffer. The cursor is now _actually_ lower (larger Y/row value) + // // // than it was before. + // // _lastText.Y += gsl::narrow_cast(newView.Height() - oldView.Height()); + // // // The text content will try and stay "stuck" at the bottom of the + // // // viewport of the terminal, and invalidating the bottom here can + // // // cause unnecessary lines to get written to the terminal. See + // // // GH#3490. + // } + // else if (oldView.Height() > newView.Height()) + // { + // // We shrunk in height. We don't really need to do anything here. + // // Shrinking in height will remove lines from the top of the buffer + // // (pushing them into scrollback in the terminal). + // // The cursor will stay in the same relative position. + // } } _resized = true; return hr;