From 6936ee15fe8da9b9835a431df24f06e61e4d2412 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Mon, 2 Aug 2021 20:56:12 +0100 Subject: [PATCH] Make the alt buffer inherit cursor state from the main buffer (#10843) When switching to the alt buffer, the starting cursor position, style, and visibility is meant to be inherited from the main buffer. Similarly, when returning to the main buffer, any changes made to those attributes should be copied back (with the exception of the cursor position, which is restored to its original state). This PR makes sure we handle that cursor state correctly. At some point I'd like to move the cursor state out of the `SCREEN_INFORMATION` class, which would make this inheritance problem a non-issue. For now, though, I've just made it copy the state from the main buffer when creating the alt buffer, and copy it back when returning to the main buffer. ## Validation Steps Performed I've added some unit tests to verify the cursor state is inherited correctly when switching to the alt buffer and back again. I also had to make a small change to one of the existing alt buffer test that relied on the initial cursor position being at 0;0, which is no longer the case. I've verified that the test case in issue #3545 is now working correctly. I've also confirmed that this fixes a problem in the _notcurses_ demo, where the cursor was showing when it should have been hidden. Closes #3545 --- src/host/screenInfo.cpp | 18 ++++++- src/host/ut_host/ScreenBufferTests.cpp | 70 ++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 81bbfd39100..2506bf4506b 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1904,10 +1904,17 @@ const SCREEN_INFORMATION& SCREEN_INFORMATION::GetMainBuffer() const ppsiNewScreenBuffer); if (NT_SUCCESS(Status)) { - // Update the alt buffer's cursor style to match our own. + // Update the alt buffer's cursor style, visibility, and position to match our own. auto& myCursor = GetTextBuffer().GetCursor(); auto* const createdBuffer = *ppsiNewScreenBuffer; - createdBuffer->GetTextBuffer().GetCursor().SetStyle(myCursor.GetSize(), myCursor.GetColor(), myCursor.GetType()); + auto& altCursor = createdBuffer->GetTextBuffer().GetCursor(); + altCursor.SetStyle(myCursor.GetSize(), myCursor.GetColor(), myCursor.GetType()); + altCursor.SetIsVisible(myCursor.IsVisible()); + altCursor.SetBlinkingAllowed(myCursor.IsBlinkingAllowed()); + // The new position should match the viewport-relative position of the main buffer. + auto altCursorPos = myCursor.GetPosition(); + altCursorPos.Y -= GetVirtualViewport().Top(); + altCursor.SetPosition(altCursorPos); s_InsertScreenBuffer(createdBuffer); @@ -1998,6 +2005,13 @@ void SCREEN_INFORMATION::UseMainScreenBuffer() s_RemoveScreenBuffer(psiAlt); // this will also delete the alt buffer // deleting the alt buffer will give the GetSet back to its main + // Copy the alt buffer's cursor style and visibility back to the main buffer. + const auto& altCursor = psiAlt->GetTextBuffer().GetCursor(); + auto& mainCursor = psiMain->GetTextBuffer().GetCursor(); + mainCursor.SetStyle(altCursor.GetSize(), altCursor.GetColor(), altCursor.GetType()); + mainCursor.SetIsVisible(altCursor.IsVisible()); + mainCursor.SetBlinkingAllowed(altCursor.IsBlinkingAllowed()); + // Tell the VT MouseInput handler that we're in the main buffer now gci.GetActiveInputBuffer()->GetTerminalInput().UseMainScreenBuffer(); } diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index d8f2359db1d..0e31c2b32c2 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -89,6 +89,8 @@ class ScreenBufferTests TEST_METHOD(MultipleAlternateBuffersFromMainCreationTest); + TEST_METHOD(AlternateBufferCursorInheritanceTest); + TEST_METHOD(TestReverseLineFeed); TEST_METHOD(TestResetClearTabStops); @@ -344,6 +346,71 @@ void ScreenBufferTests::MultipleAlternateBuffersFromMainCreationTest() } } +void ScreenBufferTests::AlternateBufferCursorInheritanceTest() +{ + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + gci.LockConsole(); // Lock must be taken to manipulate buffer. + auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); }); + + auto& mainBuffer = gci.GetActiveOutputBuffer(); + auto& mainCursor = mainBuffer.GetTextBuffer().GetCursor(); + + Log::Comment(L"Set the cursor attributes in the main buffer."); + auto mainCursorPos = COORD{ 3, 5 }; + auto mainCursorVisible = false; + auto mainCursorSize = 33u; + auto mainCursorColor = RGB(1, 2, 3); + auto mainCursorType = CursorType::DoubleUnderscore; + auto mainCursorBlinking = false; + mainCursor.SetPosition(mainCursorPos); + mainCursor.SetIsVisible(mainCursorVisible); + mainCursor.SetStyle(mainCursorSize, mainCursorColor, mainCursorType); + mainCursor.SetBlinkingAllowed(mainCursorBlinking); + + Log::Comment(L"Switch to the alternate buffer."); + VERIFY_SUCCEEDED(mainBuffer.UseAlternateScreenBuffer()); + auto& altBuffer = gci.GetActiveOutputBuffer(); + auto& altCursor = altBuffer.GetTextBuffer().GetCursor(); + auto useMain = wil::scope_exit([&] { altBuffer.UseMainScreenBuffer(); }); + + Log::Comment(L"Confirm the cursor position is inherited from the main buffer."); + VERIFY_ARE_EQUAL(mainCursorPos, altCursor.GetPosition()); + Log::Comment(L"Confirm the cursor visibility is inherited from the main buffer."); + VERIFY_ARE_EQUAL(mainCursorVisible, altCursor.IsVisible()); + Log::Comment(L"Confirm the cursor style is inherited from the main buffer."); + VERIFY_ARE_EQUAL(mainCursorSize, altCursor.GetSize()); + VERIFY_ARE_EQUAL(mainCursorColor, altCursor.GetColor()); + VERIFY_ARE_EQUAL(mainCursorType, altCursor.GetType()); + VERIFY_ARE_EQUAL(mainCursorBlinking, altCursor.IsBlinkingAllowed()); + + Log::Comment(L"Set the cursor attributes in the alt buffer."); + auto altCursorPos = COORD{ 5, 3 }; + auto altCursorVisible = true; + auto altCursorSize = 66u; + auto altCursorColor = RGB(3, 2, 1); + auto altCursorType = CursorType::EmptyBox; + auto altCursorBlinking = true; + altCursor.SetPosition(altCursorPos); + altCursor.SetIsVisible(altCursorVisible); + altCursor.SetStyle(altCursorSize, altCursorColor, altCursorType); + altCursor.SetBlinkingAllowed(altCursorBlinking); + + Log::Comment(L"Switch back to the main buffer."); + useMain.release(); + altBuffer.UseMainScreenBuffer(); + VERIFY_ARE_EQUAL(&mainBuffer, &gci.GetActiveOutputBuffer()); + + Log::Comment(L"Confirm the cursor position is restored to what it was."); + VERIFY_ARE_EQUAL(mainCursorPos, mainCursor.GetPosition()); + Log::Comment(L"Confirm the cursor visibility is inherited from the alt buffer."); + VERIFY_ARE_EQUAL(altCursorVisible, mainCursor.IsVisible()); + Log::Comment(L"Confirm the cursor style is inherited from the alt buffer."); + VERIFY_ARE_EQUAL(altCursorSize, mainCursor.GetSize()); + VERIFY_ARE_EQUAL(altCursorColor, mainCursor.GetColor()); + VERIFY_ARE_EQUAL(altCursorType, mainCursor.GetType()); + VERIFY_ARE_EQUAL(altCursorBlinking, mainCursor.IsBlinkingAllowed()); +} + void ScreenBufferTests::TestReverseLineFeed() { CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); @@ -4948,6 +5015,9 @@ void ScreenBufferTests::ClearAlternateBuffer() auto useMain = wil::scope_exit([&] { altBuffer.UseMainScreenBuffer(); }); + // Set the position to home, otherwise it's inherited from the main buffer. + VERIFY_SUCCEEDED(altBuffer.SetCursorPosition({ 0, 0 }, true)); + WriteText(altBuffer.GetTextBuffer()); VerifyText(altBuffer.GetTextBuffer());