Skip to content

Commit

Permalink
Make the alt buffer inherit cursor state from the main buffer (#10843)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@j4james @DHowett
j4james authored and DHowett committed Aug 25, 2021
1 parent a02e8ae commit 6c46b17
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 2 deletions.
18 changes: 16 additions & 2 deletions src/host/screenInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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();
}
Expand Down
70 changes: 70 additions & 0 deletions src/host/ut_host/ScreenBufferTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ class ScreenBufferTests

TEST_METHOD(MultipleAlternateBuffersFromMainCreationTest);

TEST_METHOD(AlternateBufferCursorInheritanceTest);

TEST_METHOD(TestReverseLineFeed);

TEST_METHOD(TestResetClearTabStops);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());

Expand Down

0 comments on commit 6c46b17

Please sign in to comment.