Skip to content

Commit 6936ee1

Browse files
authored
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
1 parent a151607 commit 6936ee1

File tree

2 files changed

+86
-2
lines changed

2 files changed

+86
-2
lines changed

src/host/screenInfo.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1904,10 +1904,17 @@ const SCREEN_INFORMATION& SCREEN_INFORMATION::GetMainBuffer() const
19041904
ppsiNewScreenBuffer);
19051905
if (NT_SUCCESS(Status))
19061906
{
1907-
// Update the alt buffer's cursor style to match our own.
1907+
// Update the alt buffer's cursor style, visibility, and position to match our own.
19081908
auto& myCursor = GetTextBuffer().GetCursor();
19091909
auto* const createdBuffer = *ppsiNewScreenBuffer;
1910-
createdBuffer->GetTextBuffer().GetCursor().SetStyle(myCursor.GetSize(), myCursor.GetColor(), myCursor.GetType());
1910+
auto& altCursor = createdBuffer->GetTextBuffer().GetCursor();
1911+
altCursor.SetStyle(myCursor.GetSize(), myCursor.GetColor(), myCursor.GetType());
1912+
altCursor.SetIsVisible(myCursor.IsVisible());
1913+
altCursor.SetBlinkingAllowed(myCursor.IsBlinkingAllowed());
1914+
// The new position should match the viewport-relative position of the main buffer.
1915+
auto altCursorPos = myCursor.GetPosition();
1916+
altCursorPos.Y -= GetVirtualViewport().Top();
1917+
altCursor.SetPosition(altCursorPos);
19111918

19121919
s_InsertScreenBuffer(createdBuffer);
19131920

@@ -1998,6 +2005,13 @@ void SCREEN_INFORMATION::UseMainScreenBuffer()
19982005
s_RemoveScreenBuffer(psiAlt); // this will also delete the alt buffer
19992006
// deleting the alt buffer will give the GetSet back to its main
20002007

2008+
// Copy the alt buffer's cursor style and visibility back to the main buffer.
2009+
const auto& altCursor = psiAlt->GetTextBuffer().GetCursor();
2010+
auto& mainCursor = psiMain->GetTextBuffer().GetCursor();
2011+
mainCursor.SetStyle(altCursor.GetSize(), altCursor.GetColor(), altCursor.GetType());
2012+
mainCursor.SetIsVisible(altCursor.IsVisible());
2013+
mainCursor.SetBlinkingAllowed(altCursor.IsBlinkingAllowed());
2014+
20012015
// Tell the VT MouseInput handler that we're in the main buffer now
20022016
gci.GetActiveInputBuffer()->GetTerminalInput().UseMainScreenBuffer();
20032017
}

src/host/ut_host/ScreenBufferTests.cpp

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ class ScreenBufferTests
8989

9090
TEST_METHOD(MultipleAlternateBuffersFromMainCreationTest);
9191

92+
TEST_METHOD(AlternateBufferCursorInheritanceTest);
93+
9294
TEST_METHOD(TestReverseLineFeed);
9395

9496
TEST_METHOD(TestResetClearTabStops);
@@ -344,6 +346,71 @@ void ScreenBufferTests::MultipleAlternateBuffersFromMainCreationTest()
344346
}
345347
}
346348

349+
void ScreenBufferTests::AlternateBufferCursorInheritanceTest()
350+
{
351+
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
352+
gci.LockConsole(); // Lock must be taken to manipulate buffer.
353+
auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); });
354+
355+
auto& mainBuffer = gci.GetActiveOutputBuffer();
356+
auto& mainCursor = mainBuffer.GetTextBuffer().GetCursor();
357+
358+
Log::Comment(L"Set the cursor attributes in the main buffer.");
359+
auto mainCursorPos = COORD{ 3, 5 };
360+
auto mainCursorVisible = false;
361+
auto mainCursorSize = 33u;
362+
auto mainCursorColor = RGB(1, 2, 3);
363+
auto mainCursorType = CursorType::DoubleUnderscore;
364+
auto mainCursorBlinking = false;
365+
mainCursor.SetPosition(mainCursorPos);
366+
mainCursor.SetIsVisible(mainCursorVisible);
367+
mainCursor.SetStyle(mainCursorSize, mainCursorColor, mainCursorType);
368+
mainCursor.SetBlinkingAllowed(mainCursorBlinking);
369+
370+
Log::Comment(L"Switch to the alternate buffer.");
371+
VERIFY_SUCCEEDED(mainBuffer.UseAlternateScreenBuffer());
372+
auto& altBuffer = gci.GetActiveOutputBuffer();
373+
auto& altCursor = altBuffer.GetTextBuffer().GetCursor();
374+
auto useMain = wil::scope_exit([&] { altBuffer.UseMainScreenBuffer(); });
375+
376+
Log::Comment(L"Confirm the cursor position is inherited from the main buffer.");
377+
VERIFY_ARE_EQUAL(mainCursorPos, altCursor.GetPosition());
378+
Log::Comment(L"Confirm the cursor visibility is inherited from the main buffer.");
379+
VERIFY_ARE_EQUAL(mainCursorVisible, altCursor.IsVisible());
380+
Log::Comment(L"Confirm the cursor style is inherited from the main buffer.");
381+
VERIFY_ARE_EQUAL(mainCursorSize, altCursor.GetSize());
382+
VERIFY_ARE_EQUAL(mainCursorColor, altCursor.GetColor());
383+
VERIFY_ARE_EQUAL(mainCursorType, altCursor.GetType());
384+
VERIFY_ARE_EQUAL(mainCursorBlinking, altCursor.IsBlinkingAllowed());
385+
386+
Log::Comment(L"Set the cursor attributes in the alt buffer.");
387+
auto altCursorPos = COORD{ 5, 3 };
388+
auto altCursorVisible = true;
389+
auto altCursorSize = 66u;
390+
auto altCursorColor = RGB(3, 2, 1);
391+
auto altCursorType = CursorType::EmptyBox;
392+
auto altCursorBlinking = true;
393+
altCursor.SetPosition(altCursorPos);
394+
altCursor.SetIsVisible(altCursorVisible);
395+
altCursor.SetStyle(altCursorSize, altCursorColor, altCursorType);
396+
altCursor.SetBlinkingAllowed(altCursorBlinking);
397+
398+
Log::Comment(L"Switch back to the main buffer.");
399+
useMain.release();
400+
altBuffer.UseMainScreenBuffer();
401+
VERIFY_ARE_EQUAL(&mainBuffer, &gci.GetActiveOutputBuffer());
402+
403+
Log::Comment(L"Confirm the cursor position is restored to what it was.");
404+
VERIFY_ARE_EQUAL(mainCursorPos, mainCursor.GetPosition());
405+
Log::Comment(L"Confirm the cursor visibility is inherited from the alt buffer.");
406+
VERIFY_ARE_EQUAL(altCursorVisible, mainCursor.IsVisible());
407+
Log::Comment(L"Confirm the cursor style is inherited from the alt buffer.");
408+
VERIFY_ARE_EQUAL(altCursorSize, mainCursor.GetSize());
409+
VERIFY_ARE_EQUAL(altCursorColor, mainCursor.GetColor());
410+
VERIFY_ARE_EQUAL(altCursorType, mainCursor.GetType());
411+
VERIFY_ARE_EQUAL(altCursorBlinking, mainCursor.IsBlinkingAllowed());
412+
}
413+
347414
void ScreenBufferTests::TestReverseLineFeed()
348415
{
349416
CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
@@ -4948,6 +5015,9 @@ void ScreenBufferTests::ClearAlternateBuffer()
49485015

49495016
auto useMain = wil::scope_exit([&] { altBuffer.UseMainScreenBuffer(); });
49505017

5018+
// Set the position to home, otherwise it's inherited from the main buffer.
5019+
VERIFY_SUCCEEDED(altBuffer.SetCursorPosition({ 0, 0 }, true));
5020+
49515021
WriteText(altBuffer.GetTextBuffer());
49525022
VerifyText(altBuffer.GetTextBuffer());
49535023

0 commit comments

Comments
 (0)