From c4b8153a894c6372a6267494f8c9f2df2b1fd8f8 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sat, 10 Apr 2021 10:22:50 +0100 Subject: [PATCH 1/4] Only move the virtual bottom down, unless it's outside the buffer range. --- src/host/screenInfo.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 31f36d7468a..6d589299506 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -113,7 +113,6 @@ SCREEN_INFORMATION::~SCREEN_INFORMATION() // Set up viewport pScreen->_viewport = Viewport::FromDimensions({ 0, 0 }, pScreen->_IsInPtyMode() ? coordScreenBufferSize : coordWindowSize); - pScreen->UpdateBottom(); // Set up text buffer pScreen->_textBuffer = std::make_unique(coordScreenBufferSize, @@ -121,6 +120,8 @@ SCREEN_INFORMATION::~SCREEN_INFORMATION() uiCursorSize, pScreen->_renderTarget); + pScreen->UpdateBottom(); + const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); pScreen->_textBuffer->GetCursor().SetColor(gci.GetCursorColor()); pScreen->_textBuffer->GetCursor().SetType(gci.GetCursorType()); @@ -2520,13 +2521,17 @@ TextBufferCellIterator SCREEN_INFORMATION::GetCellDataAt(const COORD at, const V // Method Description: // - Updates our internal "virtual bottom" tracker with wherever the viewport -// currently is. +// currently is. This is only used to move the bottom further down, unless +// the buffer has shrunk, and the viewport needs to be moved back up to +// fit within the new buffer range. // - // Return Value: // - void SCREEN_INFORMATION::UpdateBottom() { - _virtualBottom = _viewport.BottomInclusive(); + // We clamp it so it's at least as low as the current viewport bottom, + // but no lower than the bottom of the buffer. + _virtualBottom = std::clamp(_virtualBottom, _viewport.BottomInclusive(), GetBufferSize().BottomInclusive()); } // Method Description: From c6897dbab22436fd05f7f26abfa2cc557bb75aea Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sat, 10 Apr 2021 10:40:29 +0100 Subject: [PATCH 2/4] Reset the virtual bottom when the scrollback is erased. --- src/host/outputStream.cpp | 11 +++++++++++ src/host/outputStream.hpp | 1 + src/host/screenInfo.cpp | 13 +++++++++++++ src/host/screenInfo.hpp | 1 + src/terminal/adapter/adaptDispatch.cpp | 3 +++ src/terminal/adapter/conGetSet.hpp | 1 + src/terminal/adapter/ut_adapter/adapterTest.cpp | 5 +++++ 7 files changed, 35 insertions(+) diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index 1b04cbcab4f..1fbe74577fa 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -685,6 +685,17 @@ bool ConhostInternalGetSet::InsertLines(const size_t count) return true; } +// Method Description: +// - Resets the internal "virtual bottom" tracker to the top of the buffer. +// Arguments: +// - +// Return Value: +// - +void ConhostInternalGetSet::ResetBottom() +{ + _io.GetActiveOutputBuffer().ResetBottom(); +} + // Method Description: // - Connects the MoveToBottom call directly into our Driver Message servicing // call inside Conhost.exe diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index b55f8048360..506f9f98e06 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -126,6 +126,7 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: bool DeleteLines(const size_t count) override; bool InsertLines(const size_t count) override; + void ResetBottom() override; bool MoveToBottom() const override; bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const noexcept override; diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 6d589299506..9887e34ee1c 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -2534,6 +2534,19 @@ void SCREEN_INFORMATION::UpdateBottom() _virtualBottom = std::clamp(_virtualBottom, _viewport.BottomInclusive(), GetBufferSize().BottomInclusive()); } +// Method Description: +// - Resets the internal "virtual bottom" tracker to the top of the buffer. +// Used when the scrollback buffer has been completely cleared. +// - +// Return Value: +// - +void SCREEN_INFORMATION::ResetBottom() +{ + // The virtual bottom points to the last line of the viewport, so at the + // top of the buffer it should be one less than the viewport height. + _virtualBottom = _viewport.Height() - 1; +} + // Method Description: // - Initialize the row with the cursor on it to the standard erase attributes. // This is executed when we move the cursor below the current viewport in diff --git a/src/host/screenInfo.hpp b/src/host/screenInfo.hpp index f1afd46f5fd..65884d54943 100644 --- a/src/host/screenInfo.hpp +++ b/src/host/screenInfo.hpp @@ -226,6 +226,7 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console void SetTerminalConnection(_In_ Microsoft::Console::ITerminalOutputConnection* const pTtyConnection); void UpdateBottom(); + void ResetBottom(); void MoveToBottom(); Microsoft::Console::Render::IRenderTarget& GetRenderTarget() noexcept; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index fade27e61a6..18215bb8553 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -2030,6 +2030,9 @@ bool AdaptDispatch::_EraseScrollback() if (success) { + // Reset the virtual viewport bottom to the top of the buffer. + _pConApi->ResetBottom(); + // Move the viewport (CAN'T be done in one call with SetConsolescreenBufferInfoEx, because legacy) SMALL_RECT newViewport; newViewport.Left = screen.Left; diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp index ba93c100d46..a081ee28785 100644 --- a/src/terminal/adapter/conGetSet.hpp +++ b/src/terminal/adapter/conGetSet.hpp @@ -90,6 +90,7 @@ namespace Microsoft::Console::VirtualTerminal virtual bool DeleteLines(const size_t count) = 0; virtual bool InsertLines(const size_t count) = 0; + virtual void ResetBottom() = 0; virtual bool MoveToBottom() const = 0; virtual bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const = 0; diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index b2b1a2be2ec..2ee78fd0175 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -518,6 +518,11 @@ class TestGetSet final : public ConGetSet return TRUE; } + void ResetBottom() override + { + Log::Comment(L"ResetBottom MOCK called..."); + } + bool MoveToBottom() const override { Log::Comment(L"MoveToBottom MOCK called..."); From 87e74e431cf5cd0988ecf2d9d77fd125f105bbff Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sat, 10 Apr 2021 15:13:53 +0100 Subject: [PATCH 3/4] Make sure the virtual bottom is reset when necessary in ScreenBufferTests. --- src/host/ut_host/ScreenBufferTests.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index d8f2359db1d..74f36fda6be 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -67,6 +67,8 @@ class ScreenBufferTests auto& currentBuffer = gci.GetActiveOutputBuffer(); // Make sure a test hasn't left us in the alt buffer on accident VERIFY_IS_FALSE(currentBuffer._IsAltBuffer()); + // Make sure the virtual viewport bottom is reset. + currentBuffer._virtualBottom = 0; VERIFY_SUCCEEDED(currentBuffer.SetViewportOrigin(true, { 0, 0 }, true)); // Make sure the viewport always starts off at the default size. auto defaultSize = COORD{ CommonState::s_csWindowWidth, CommonState::s_csWindowHeight }; @@ -1542,6 +1544,7 @@ void ScreenBufferTests::VtNewlineOutsideMargins() VERIFY_ARE_EQUAL(COORD({ 0, viewportTop + 1 }), si.GetViewport().Origin()); Log::Comment(L"Reset viewport and apply DECSTBM margins"); + si._virtualBottom = 0; VERIFY_SUCCEEDED(si.SetViewportOrigin(true, COORD({ 0, viewportTop }), true)); stateMachine.ProcessString(L"\x1b[1;5r"); // Make sure we clear the margins on exit so they can't break other tests. @@ -3341,6 +3344,7 @@ void ScreenBufferTests::ScrollOperations() const auto bufferHeight = si.GetBufferSize().Height(); // Move the viewport down a few lines, and only cover part of the buffer width. + si._virtualBottom = 0; si.SetViewport(Viewport::FromDimensions({ 5, 10 }, { bufferWidth - 10, 10 }), true); const auto viewportStart = si.GetViewport().Top(); const auto viewportEnd = si.GetViewport().BottomExclusive(); @@ -3865,6 +3869,7 @@ void ScreenBufferTests::EraseTests() const auto bufferHeight = si.GetBufferSize().Height(); // Move the viewport down a few lines, and only cover part of the buffer width. + si._virtualBottom = 0; si.SetViewport(Viewport::FromDimensions({ 5, 10 }, { bufferWidth - 10, 10 }), true); // Fill the entire buffer with Zs. Blue on Green. @@ -3985,6 +3990,7 @@ void _CommonScrollingSetup() const auto oldView = si.GetViewport(); const auto view = Viewport::FromDimensions({ 0, 0 }, { oldView.Width(), 6 }); si.SetViewport(view, true); + si.ResetBottom(); cursor.SetPosition({ 0, 0 }); stateMachine.ProcessString(L"A"); cursor.SetPosition({ 0, 5 }); From 19f63dc6d97eaaaa0a72c771de6bbc8c3f1c2511 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sat, 10 Apr 2021 19:36:19 +0100 Subject: [PATCH 4/4] Add unit test to verify the virtual bottom doesn't move up. --- src/host/ut_host/ScreenBufferTests.cpp | 47 ++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 74f36fda6be..2e685df2a7e 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -217,6 +217,7 @@ class ScreenBufferTests TEST_METHOD(TestAddHyperlinkCustomIdDifferentUri); TEST_METHOD(UpdateVirtualBottomWhenCursorMovesBelowIt); + TEST_METHOD(DontUpdateVirtualBottomWhenMovedUp); TEST_METHOD(RetainHorizontalOffsetWhenMovingToBottom); TEST_METHOD(TestWriteConsoleVTQuirkMode); @@ -6027,6 +6028,52 @@ void ScreenBufferTests::UpdateVirtualBottomWhenCursorMovesBelowIt() VERIFY_ARE_EQUAL(newVirtualBottom, si.GetViewport().BottomInclusive()); } +void ScreenBufferTests::DontUpdateVirtualBottomWhenMovedUp() +{ + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + Log::Comment(L"Set the initial viewport one page from the top"); + const auto initialOrigin = COORD{ 0, si.GetViewport().Top() + si.GetViewport().Height() }; + VERIFY_SUCCEEDED(si.SetViewportOrigin(false, initialOrigin, false)); + VERIFY_ARE_EQUAL(initialOrigin, si.GetViewport().Origin()); + + Log::Comment(L"Make sure the initial virtual bottom is at the bottom of the viewport"); + si.UpdateBottom(); + const auto initialVirtualBottom = si.GetViewport().BottomInclusive(); + VERIFY_ARE_EQUAL(initialVirtualBottom, si._virtualBottom); + + Log::Comment(L"Set the initial cursor position 5 lines above virtual bottom line"); + const auto initialCursorPos = COORD{ 0, initialVirtualBottom - 5 }; + cursor.SetPosition(initialCursorPos); + VERIFY_ARE_EQUAL(initialCursorPos, cursor.GetPosition()); + + Log::Comment(L"Pan to the top of the buffer so the the cursor is out of view"); + const auto topOfBufferOrigin = COORD{ 0, 0 }; + VERIFY_SUCCEEDED(si.SetViewportOrigin(true, topOfBufferOrigin, false)); + VERIFY_ARE_EQUAL(topOfBufferOrigin, si.GetViewport().Origin()); + + Log::Comment(L"Confirm that the virtual bottom has not changed"); + VERIFY_ARE_EQUAL(initialVirtualBottom, si._virtualBottom); + + Log::Comment(L"Now write out some content using WriteCharsLegacy"); + const auto content = L"Hello World"; + auto numBytes = wcslen(content) * sizeof(wchar_t); + VERIFY_SUCCESS_NTSTATUS(WriteCharsLegacy(si, content, content, content, &numBytes, nullptr, 0, 0, nullptr)); + + Log::Comment(L"The viewport bottom should now align with the cursor pos"); + VERIFY_ARE_EQUAL(initialCursorPos.Y, si.GetViewport().BottomInclusive()); + + Log::Comment(L"But the virtual bottom should still not have changed"); + VERIFY_ARE_EQUAL(initialVirtualBottom, si._virtualBottom); + + Log::Comment(L"After MoveToBottom, the viewport should align with the virtual bottom"); + si.MoveToBottom(); + VERIFY_ARE_EQUAL(initialVirtualBottom, si.GetViewport().BottomInclusive()); +} + void ScreenBufferTests::RetainHorizontalOffsetWhenMovingToBottom() { auto& g = ServiceLocator::LocateGlobals();