From 0e6f290806ef066e8ef29c0a933c83a0007eac98 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Wed, 10 Jul 2019 17:42:13 +0100 Subject: [PATCH] Fix margin boundary tests in the RI, DL, and IL escape sequences. (#1807) * Fix margin boundary tests in the RI, DL, and IL sequences. * Refactor the margin boundary tests into a reusable SCREEN_INFORMATION method. * Add screen buffer unit tests for the RI, DL, and IL sequences. --- src/host/getset.cpp | 15 +- src/host/screenInfo.cpp | 18 ++ src/host/screenInfo.hpp | 1 + src/host/ut_host/ScreenBufferTests.cpp | 229 +++++++++++++++++++++++++ 4 files changed, 252 insertions(+), 11 deletions(-) diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 3606bd093b7..f576c1d9414 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -1350,13 +1350,10 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool } else { - const auto margins = screenInfo.GetAbsoluteScrollMargins(); - const bool marginsSet = margins.BottomInclusive() > margins.Top(); - // If we don't have margins, or the cursor is within the boundaries of the margins // It's important to check if the cursor is in the margins, // If it's not, but the margins are set, then we don't want to scroll anything - if (!marginsSet || margins.IsInBounds(oldCursorPosition)) + if (screenInfo.IsCursorInMargins(oldCursorPosition)) { // Cursor is at the top of the viewport const COORD bufferSize = screenInfo.GetBufferSize().Dimensions(); @@ -1396,20 +1393,17 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool [[nodiscard]] HRESULT DoSrvMoveCursorVertically(SCREEN_INFORMATION& screenInfo, const short lines) { auto& cursor = screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor(); - const int currentCursorY = cursor.GetPosition().Y; - SMALL_RECT margins = screenInfo.GetAbsoluteScrollMargins().ToInclusive(); - const auto marginsSet = margins.Bottom > margins.Top; - const auto cursorInMargins = currentCursorY <= margins.Bottom && currentCursorY >= margins.Top; COORD clampedPos = { cursor.GetPosition().X, cursor.GetPosition().Y + lines }; // Make sure the cursor doesn't move outside the viewport. screenInfo.GetViewport().Clamp(clampedPos); // Make sure the cursor stays inside the margins, but only if it started there - if (marginsSet && cursorInMargins) + if (screenInfo.AreMarginsSet() && screenInfo.IsCursorInMargins(cursor.GetPosition())) { try { + const auto margins = screenInfo.GetAbsoluteScrollMargins().ToInclusive(); const auto v = clampedPos.Y; const auto lo = margins.Top; const auto hi = margins.Bottom; @@ -2037,8 +2031,7 @@ void DoSrvPrivateModifyLinesImpl(const unsigned int count, const bool insert) auto& screenInfo = ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer().GetActiveBuffer(); auto& textBuffer = screenInfo.GetTextBuffer(); const auto cursorPosition = textBuffer.GetCursor().GetPosition(); - const auto margins = screenInfo.GetAbsoluteScrollMargins(); - if (margins.IsInBounds(cursorPosition)) + if (screenInfo.IsCursorInMargins(cursorPosition)) { const auto screenEdges = screenInfo.GetBufferSize().ToInclusive(); // Rectangle to cut out of the existing buffer diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 83cb0809741..c8bee162166 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -2899,6 +2899,24 @@ bool SCREEN_INFORMATION::AreMarginsSet() const noexcept return _scrollMargins.BottomInclusive() > _scrollMargins.Top(); } +// Routine Description: +// - Determines whether a cursor position is within the vertical bounds of the +// scroll margins, or the margins aren't set. +// Parameters: +// - cursorPosition - The cursor position to test +// Return value: +// - true iff the position is in bounds. +bool SCREEN_INFORMATION::IsCursorInMargins(const COORD cursorPosition) const noexcept +{ + // If the margins aren't set, then any position is considered in bounds. + if (!AreMarginsSet()) + { + return true; + } + const auto margins = GetAbsoluteScrollMargins().ToInclusive(); + return cursorPosition.Y <= margins.Bottom && cursorPosition.Y >= margins.Top; +} + // Method Description: // - Gets the region of the buffer that should be used for scrolling within the // scroll margins. If the scroll margins aren't set, it returns the entire diff --git a/src/host/screenInfo.hpp b/src/host/screenInfo.hpp index 0a617008e8c..d44a16e05d6 100644 --- a/src/host/screenInfo.hpp +++ b/src/host/screenInfo.hpp @@ -201,6 +201,7 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console Microsoft::Console::Types::Viewport GetAbsoluteScrollMargins() const; void SetScrollMargins(const Microsoft::Console::Types::Viewport margins); bool AreMarginsSet() const noexcept; + bool IsCursorInMargins(const COORD cursorPosition) const noexcept; Microsoft::Console::Types::Viewport GetScrollingRegion() const noexcept; [[nodiscard]] NTSTATUS UseAlternateScreenBuffer(); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index d218d76974b..aa56b1b4806 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -163,6 +163,9 @@ class ScreenBufferTests TEST_METHOD(ScrollUpInMargins); TEST_METHOD(ScrollDownInMargins); + TEST_METHOD(InsertLinesInMargins); + TEST_METHOD(DeleteLinesInMargins); + TEST_METHOD(ReverseLineFeedInMargins); TEST_METHOD(SetOriginMode); }; @@ -3220,6 +3223,232 @@ void ScreenBufferTests::ScrollDownInMargins() } } +void ScreenBufferTests::InsertLinesInMargins() +{ + Log::Comment( + L"Does the common scrolling setup, then inserts two lines inside the " + L"margin boundaries, and verifies the rows have what we'd expect."); + + _CommonScrollingSetup(); + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + // Move to column 5 of line 3 + stateMachine.ProcessString(L"\x1b[3;5H"); + // Insert 2 lines + stateMachine.ProcessString(L"\x1b[2L"); + + Log::Comment(NoThrowString().Format( + L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); + Log::Comment(NoThrowString().Format( + L"viewport=%s", VerifyOutputTraits::ToString(si.GetViewport().ToInclusive()).GetBuffer())); + + VERIFY_ARE_EQUAL(4, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(2, cursor.GetPosition().Y); + { + auto iter0 = tbi.GetCellDataAt({ 0, 0 }); + auto iter1 = tbi.GetCellDataAt({ 0, 1 }); + auto iter2 = tbi.GetCellDataAt({ 0, 2 }); + auto iter3 = tbi.GetCellDataAt({ 0, 3 }); + auto iter4 = tbi.GetCellDataAt({ 0, 4 }); + auto iter5 = tbi.GetCellDataAt({ 0, 5 }); + VERIFY_ARE_EQUAL(L"A", iter0->Chars()); + VERIFY_ARE_EQUAL(L"5", iter1->Chars()); + VERIFY_ARE_EQUAL(L"\x20", iter2->Chars()); + VERIFY_ARE_EQUAL(L"\x20", iter3->Chars()); + VERIFY_ARE_EQUAL(L"6", iter4->Chars()); + VERIFY_ARE_EQUAL(L"B", iter5->Chars()); + } + + Log::Comment( + L"Does the common scrolling setup, then inserts one line with no " + L"margins set, and verifies the rows have what we'd expect."); + + _CommonScrollingSetup(); + // Clear the scroll margins + stateMachine.ProcessString(L"\x1b[r"); + // Move to column 5 of line 2 + stateMachine.ProcessString(L"\x1b[2;5H"); + // Insert 1 line + stateMachine.ProcessString(L"\x1b[L"); + + Log::Comment(NoThrowString().Format( + L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); + Log::Comment(NoThrowString().Format( + L"viewport=%s", VerifyOutputTraits::ToString(si.GetViewport().ToInclusive()).GetBuffer())); + + VERIFY_ARE_EQUAL(4, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + { + auto iter0 = tbi.GetCellDataAt({ 0, 0 }); + auto iter1 = tbi.GetCellDataAt({ 0, 1 }); + auto iter2 = tbi.GetCellDataAt({ 0, 2 }); + auto iter3 = tbi.GetCellDataAt({ 0, 3 }); + auto iter4 = tbi.GetCellDataAt({ 0, 4 }); + auto iter5 = tbi.GetCellDataAt({ 0, 5 }); + VERIFY_ARE_EQUAL(L"A", iter0->Chars()); + VERIFY_ARE_EQUAL(L"\x20", iter1->Chars()); + VERIFY_ARE_EQUAL(L"5", iter2->Chars()); + VERIFY_ARE_EQUAL(L"6", iter3->Chars()); + VERIFY_ARE_EQUAL(L"7", iter4->Chars()); + VERIFY_ARE_EQUAL(L"\x20", iter5->Chars()); + } +} + +void ScreenBufferTests::DeleteLinesInMargins() +{ + Log::Comment( + L"Does the common scrolling setup, then deletes two lines inside the " + L"margin boundaries, and verifies the rows have what we'd expect."); + + _CommonScrollingSetup(); + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + // Move to column 5 of line 3 + stateMachine.ProcessString(L"\x1b[3;5H"); + // Delete 2 lines + stateMachine.ProcessString(L"\x1b[2M"); + + Log::Comment(NoThrowString().Format( + L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); + Log::Comment(NoThrowString().Format( + L"viewport=%s", VerifyOutputTraits::ToString(si.GetViewport().ToInclusive()).GetBuffer())); + + VERIFY_ARE_EQUAL(4, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(2, cursor.GetPosition().Y); + { + auto iter0 = tbi.GetCellDataAt({ 0, 0 }); + auto iter1 = tbi.GetCellDataAt({ 0, 1 }); + auto iter2 = tbi.GetCellDataAt({ 0, 2 }); + auto iter3 = tbi.GetCellDataAt({ 0, 3 }); + auto iter4 = tbi.GetCellDataAt({ 0, 4 }); + auto iter5 = tbi.GetCellDataAt({ 0, 5 }); + VERIFY_ARE_EQUAL(L"A", iter0->Chars()); + VERIFY_ARE_EQUAL(L"5", iter1->Chars()); + VERIFY_ARE_EQUAL(L"\x20", iter2->Chars()); + VERIFY_ARE_EQUAL(L"\x20", iter3->Chars()); + VERIFY_ARE_EQUAL(L"\x20", iter4->Chars()); + VERIFY_ARE_EQUAL(L"B", iter5->Chars()); + } + + Log::Comment( + L"Does the common scrolling setup, then deletes one line with no " + L"margins set, and verifies the rows have what we'd expect."); + + _CommonScrollingSetup(); + // Clear the scroll margins + stateMachine.ProcessString(L"\x1b[r"); + // Move to column 5 of line 2 + stateMachine.ProcessString(L"\x1b[2;5H"); + // Delete 1 line + stateMachine.ProcessString(L"\x1b[M"); + + Log::Comment(NoThrowString().Format( + L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); + Log::Comment(NoThrowString().Format( + L"viewport=%s", VerifyOutputTraits::ToString(si.GetViewport().ToInclusive()).GetBuffer())); + + VERIFY_ARE_EQUAL(4, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + { + auto iter0 = tbi.GetCellDataAt({ 0, 0 }); + auto iter1 = tbi.GetCellDataAt({ 0, 1 }); + auto iter2 = tbi.GetCellDataAt({ 0, 2 }); + auto iter3 = tbi.GetCellDataAt({ 0, 3 }); + auto iter4 = tbi.GetCellDataAt({ 0, 4 }); + auto iter5 = tbi.GetCellDataAt({ 0, 5 }); + VERIFY_ARE_EQUAL(L"A", iter0->Chars()); + VERIFY_ARE_EQUAL(L"6", iter1->Chars()); + VERIFY_ARE_EQUAL(L"7", iter2->Chars()); + VERIFY_ARE_EQUAL(L"\x20", iter3->Chars()); + VERIFY_ARE_EQUAL(L"B", iter4->Chars()); + VERIFY_ARE_EQUAL(L"\x20", iter5->Chars()); + } +} + +void ScreenBufferTests::ReverseLineFeedInMargins() +{ + Log::Comment( + L"Does the common scrolling setup, then executes a reverse line feed " + L"below the top margin, and verifies the rows have what we'd expect."); + + _CommonScrollingSetup(); + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + // Move to column 5 of line 2, the top margin + stateMachine.ProcessString(L"\x1b[2;5H"); + // Execute a reverse line feed (RI) + stateMachine.ProcessString(L"\x1bM"); + + Log::Comment(NoThrowString().Format( + L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); + Log::Comment(NoThrowString().Format( + L"viewport=%s", VerifyOutputTraits::ToString(si.GetViewport().ToInclusive()).GetBuffer())); + + VERIFY_ARE_EQUAL(4, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + { + auto iter0 = tbi.GetCellDataAt({ 0, 0 }); + auto iter1 = tbi.GetCellDataAt({ 0, 1 }); + auto iter2 = tbi.GetCellDataAt({ 0, 2 }); + auto iter3 = tbi.GetCellDataAt({ 0, 3 }); + auto iter4 = tbi.GetCellDataAt({ 0, 4 }); + auto iter5 = tbi.GetCellDataAt({ 0, 5 }); + VERIFY_ARE_EQUAL(L"A", iter0->Chars()); + VERIFY_ARE_EQUAL(L"\x20", iter1->Chars()); + VERIFY_ARE_EQUAL(L"5", iter2->Chars()); + VERIFY_ARE_EQUAL(L"6", iter3->Chars()); + VERIFY_ARE_EQUAL(L"7", iter4->Chars()); + VERIFY_ARE_EQUAL(L"B", iter5->Chars()); + } + + Log::Comment( + L"Does the common scrolling setup, then executes a reverse line feed " + L"with the top margin at the top of the screen, and verifies the rows " + L"have what we'd expect."); + + _CommonScrollingSetup(); + // Set the top scroll margin to the top of the screen + stateMachine.ProcessString(L"\x1b[1;5r"); + // Move to column 5 of line 1, the top of the screen + stateMachine.ProcessString(L"\x1b[1;5H"); + // Execute a reverse line feed (RI) + stateMachine.ProcessString(L"\x1bM"); + + Log::Comment(NoThrowString().Format( + L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); + Log::Comment(NoThrowString().Format( + L"viewport=%s", VerifyOutputTraits::ToString(si.GetViewport().ToInclusive()).GetBuffer())); + + VERIFY_ARE_EQUAL(4, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y); + { + auto iter0 = tbi.GetCellDataAt({ 0, 0 }); + auto iter1 = tbi.GetCellDataAt({ 0, 1 }); + auto iter2 = tbi.GetCellDataAt({ 0, 2 }); + auto iter3 = tbi.GetCellDataAt({ 0, 3 }); + auto iter4 = tbi.GetCellDataAt({ 0, 4 }); + auto iter5 = tbi.GetCellDataAt({ 0, 5 }); + VERIFY_ARE_EQUAL(L"\x20", iter0->Chars()); + VERIFY_ARE_EQUAL(L"A", iter1->Chars()); + VERIFY_ARE_EQUAL(L"5", iter2->Chars()); + VERIFY_ARE_EQUAL(L"6", iter3->Chars()); + VERIFY_ARE_EQUAL(L"7", iter4->Chars()); + VERIFY_ARE_EQUAL(L"B", iter5->Chars()); + } +} + void ScreenBufferTests::SetOriginMode() { auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();