Make the RIS command clear the display and scrollback correctly (#2367)

When the scrollback buffer is empty, the RIS escape sequence (Reset to Initial
State) will fail to clear the screen, or reset any of the state. And when there
is something in the scrollback, it doesn't get cleared completely, and the
screen may get filled with the wrong background color (it should use the
default color, but it actually uses the previously active background color).
This commit attempts to fix those issues.

The initial failure is caused by the `SCREEN_INFORMATION::WriteRect` method
throwing an exception when passed an empty viewport. And the reason it's passed
an empty viewport is because that's what the `Viewport::Subtract` method
returns when the result of the subtraction is nothing.  The PR fixes the
problem by making the `Viewport::Subtract` method actually return nothing in
that situation. 

This is a change in the defined behavior that also required the associated
viewport tests to be updated. However, it does seem a sensible change, since
the `Subtract` method never returns empty viewports under any other
circumstances. And the only place the method seems to be used is in the
`ScrollRegion` implementation, where the previous behavior is guaranteed to
throw an exception.

The other issues are fixed simply by changing the order in which things are
reset in the `AdaptDispatch::HardReset` method. The call to `SoftReset` needed
to be made first, so that the SGR attributes would be reset before the screen
was cleared, thus making sure that the default background color would be used.
And the screen needed to be cleared before the scrollback was erased, otherwise
the last view of the screen would be retained in the scrollback buffer.

These changes also required existing adapter tests to be updated, but not
because of a change in the expected behaviour. It's just that certain tests
relied on the `SoftReset` happening later in the order, so weren't expecting it
to be called if say the scrollback erase had failed. It doesn't seem like the
tests were deliberately trying to verify that the SoftReset _hadn't_ been
called.

In addition to the updates to existing tests, this PR also add a new screen
buffer test which verifies the display and scrollback are correctly cleared
under the conditions that were previously failing.

Fixes #2307.

src/host/ut_host/ScreenBufferTests.cpp

@@ -168,6 +168,8 @@ class ScreenBufferTests
     TEST_METHOD(ReverseLineFeedInMargins);
 
     TEST_METHOD(SetOriginMode);
+
+    TEST_METHOD(HardResetBuffer);
 };
 
 void ScreenBufferTests::SingleAlternateBufferCreationTest()
@@ -3516,3 +3518,66 @@ void ScreenBufferTests::SetOriginMode()
     // Reset DECOM so we don't affect future tests
     stateMachine.ProcessString(L"\x1B[?6l");
 }
+
+void ScreenBufferTests::HardResetBuffer()
+{
+    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer().GetActiveBuffer();
+    auto& stateMachine = si.GetStateMachine();
+    const auto& viewport = si.GetViewport();
+    const auto& cursor = si.GetTextBuffer().GetCursor();
+    WI_SetFlag(si.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING);
+
+    auto isBufferClear = [&]() {
+        auto offset = 0;
+        auto width = si.GetBufferSize().Width();
+        for (auto iter = si.GetCellDataAt({}); iter; ++iter, ++offset)
+        {
+            if (iter->Chars() != L" " || iter->TextAttr() != TextAttribute{})
+            {
+                Log::Comment(NoThrowString().Format(
+                    L"Buffer not clear at (X:%d, Y:%d)",
+                    offset % width,
+                    offset / width));
+                return false;
+            }
+        }
+        return true;
+    };
+
+    const auto resetToInitialState = L"\033c";
+
+    Log::Comment(L"Start with a clear buffer, viewport and cursor at 0,0");
+    si.SetAttributes(TextAttribute());
+    si.ClearTextData();
+    VERIFY_SUCCEEDED(si.SetViewportOrigin(true, { 0, 0 }, true));
+    VERIFY_SUCCEEDED(si.SetCursorPosition({ 0, 0 }, true));
+    VERIFY_IS_TRUE(isBufferClear());
+
+    Log::Comment(L"Write a single line of text to the buffer");
+    stateMachine.ProcessString(L"Hello World!\n");
+    VERIFY_IS_FALSE(isBufferClear());
+    VERIFY_ARE_EQUAL(COORD({ 0, 1 }), cursor.GetPosition());
+
+    Log::Comment(L"After a reset, buffer should be clear, with cursor at 0,0");
+    stateMachine.ProcessString(resetToInitialState);
+    VERIFY_IS_TRUE(isBufferClear());
+    VERIFY_ARE_EQUAL(COORD({ 0, 0 }), cursor.GetPosition());
+
+    Log::Comment(L"Set the background color to red");
+    stateMachine.ProcessString(L"\x1b[41m");
+    Log::Comment(L"Write multiple pages of text to the buffer");
+    for (auto i = 0; i < viewport.Height() * 2; i++)
+    {
+        stateMachine.ProcessString(L"Hello World!\n");
+    }
+    VERIFY_IS_FALSE(isBufferClear());
+    VERIFY_IS_GREATER_THAN(viewport.Top(), viewport.Height());
+    VERIFY_IS_GREATER_THAN(cursor.GetPosition().Y, viewport.Height());
+
+    Log::Comment(L"After a reset, buffer should be clear, with viewport and cursor at 0,0");
+    stateMachine.ProcessString(resetToInitialState);
+    VERIFY_IS_TRUE(isBufferClear());
+    VERIFY_ARE_EQUAL(COORD({ 0, 0 }), viewport.Origin());
+    VERIFY_ARE_EQUAL(COORD({ 0, 0 }), cursor.GetPosition());
+}

src/host/ut_host/ViewportTests.cpp

@@ -1044,18 +1044,8 @@ class ViewportTests
         const auto original = Viewport::FromInclusive(srOriginal);
         const auto remove = original;
 
-        std::vector<Viewport> expected;
-        expected.emplace_back(Viewport::FromDimensions(original.Origin(), { 0, 0 }));
-
         const auto actual = Viewport::Subtract(original, remove);
 
-        VERIFY_ARE_EQUAL(expected.size(), actual.size(), L"Same number of viewports in expected and actual");
-        Log::Comment(L"Now validate that each viewport has the expected area.");
-        for (size_t i = 0; i < expected.size(); i++)
-        {
-            const auto& exp = expected.at(i);
-            const auto& act = actual.at(i);
-            VERIFY_ARE_EQUAL(exp, act);
-        }
+        VERIFY_ARE_EQUAL(0u, actual.size(), L"There should be no viewports returned");
     }
 };

src/terminal/adapter/adaptDispatch.cpp

@@ -1601,17 +1601,18 @@ bool AdaptDispatch::SoftReset()
 // True if handled successfully. False otherwise.
 bool AdaptDispatch::HardReset()
 {
+    // Sets the SGR state to normal - this must be done before EraseInDisplay
+    //      to ensure that it clears with the default background color.
+    bool fSuccess = SoftReset();
+
     // Clears the screen - Needs to be done in two operations.
-    bool fSuccess = _EraseScrollback();
     if (fSuccess)
     {
         fSuccess = EraseInDisplay(DispatchTypes::EraseType::All);
     }
-
-    // Sets the SGR state to normal.
     if (fSuccess)
     {
-        fSuccess = SoftReset();
+        fSuccess = _EraseScrollback();
     }
 
     // Cursor to 1,1 - the Soft Reset guarantees this is absolute

src/terminal/adapter/ut_adapter/adapterTest.cpp

@@ -3426,49 +3426,55 @@ class AdapterTest
         // The cursor will be moved to the same relative location in the new viewport with origin @ 0, 0
         const COORD coordRelativeCursor = { _testGetSet->_coordCursorPos.X - _testGetSet->_srViewport.Left,
                                             _testGetSet->_coordCursorPos.Y - _testGetSet->_srViewport.Top };
-
-        // Cursor to 1,1
-        _testGetSet->_coordExpectedCursorPos = { 0, 0 };
-        _testGetSet->_fSetConsoleCursorPositionResult = true;
-        _testGetSet->_fPrivateSetLegacyAttributesResult = true;
-        _testGetSet->_fPrivateSetDefaultAttributesResult = true;
-        _testGetSet->_fPrivateBoldTextResult = true;
-        _testGetSet->_fExpectedForeground = true;
-        _testGetSet->_fExpectedBackground = true;
-        _testGetSet->_fExpectedMeta = true;
-        _testGetSet->_fExpectedIsBold = false;
-        _testGetSet->_expectedShowCursor = true;
-        _testGetSet->_privateShowCursorResult = true;
         const COORD coordExpectedCursorPos = { 0, 0 };
 
-        // We're expecting _SetDefaultColorHelper to call
-        //      PrivateSetLegacyAttributes with 0 as the wAttr param.
-        _testGetSet->_wExpectedAttribute = 0;
+        auto prepExpectedParameters = [&]() {
+            // Cursor to 1,1
+            _testGetSet->_coordExpectedCursorPos = { 0, 0 };
+            _testGetSet->_fSetConsoleCursorPositionResult = true;
+            _testGetSet->_fPrivateSetLegacyAttributesResult = true;
+            _testGetSet->_fPrivateSetDefaultAttributesResult = true;
+            _testGetSet->_fPrivateBoldTextResult = true;
+            _testGetSet->_fExpectedForeground = true;
+            _testGetSet->_fExpectedBackground = true;
+            _testGetSet->_fExpectedMeta = true;
+            _testGetSet->_fExpectedIsBold = false;
+            _testGetSet->_expectedShowCursor = true;
+            _testGetSet->_privateShowCursorResult = true;
+
+            // We're expecting _SetDefaultColorHelper to call
+            //      PrivateSetLegacyAttributes with 0 as the wAttr param.
+            _testGetSet->_wExpectedAttribute = 0;
 
-        // Prepare the results of SoftReset api calls
-        _testGetSet->_fPrivateSetCursorKeysModeResult = true;
-        _testGetSet->_fPrivateSetKeypadModeResult = true;
-        _testGetSet->_fGetConsoleScreenBufferInfoExResult = true;
-        _testGetSet->_fPrivateSetScrollingRegionResult = true;
+            // Prepare the results of SoftReset api calls
+            _testGetSet->_fPrivateSetCursorKeysModeResult = true;
+            _testGetSet->_fPrivateSetKeypadModeResult = true;
+            _testGetSet->_fGetConsoleScreenBufferInfoExResult = true;
+            _testGetSet->_fPrivateSetScrollingRegionResult = true;
+        };
+        prepExpectedParameters();
 
         VERIFY_IS_TRUE(_pDispatch->HardReset());
         VERIFY_ARE_EQUAL(_testGetSet->_coordCursorPos, coordExpectedCursorPos);
         VERIFY_ARE_EQUAL(_testGetSet->_fUsingRgbColor, false);
 
         Log::Comment(L"Test 2: Gracefully fail when getting console information fails.");
         _testGetSet->PrepData();
+        prepExpectedParameters();
         _testGetSet->_fGetConsoleScreenBufferInfoExResult = false;
 
         VERIFY_IS_FALSE(_pDispatch->HardReset());
 
         Log::Comment(L"Test 3: Gracefully fail when filling the rectangle fails.");
         _testGetSet->PrepData();
+        prepExpectedParameters();
         _testGetSet->_fFillConsoleOutputCharacterWResult = false;
 
         VERIFY_IS_FALSE(_pDispatch->HardReset());
 
         Log::Comment(L"Test 4: Gracefully fail when setting the window fails.");
         _testGetSet->PrepData();
+        prepExpectedParameters();
         _testGetSet->_fSetConsoleWindowInfoResult = false;
 
         VERIFY_IS_FALSE(_pDispatch->HardReset());

src/types/viewport.cpp

@@ -909,7 +909,8 @@ Viewport Viewport::ToOrigin() const noexcept
         // Just put the original rectangle into the results and return early.
         result.viewports.at(result.used++) = original;
     }
-    else
+    // If the original rectangle matches the intersection, there is nothing to return.
+    else if (original != intersection)
     {
         // Generate our potential four viewports that represent the region of the original that falls outside of the remove area.
         // We will bias toward generating wide rectangles over tall rectangles (if possible) so that optimizations that apply
@@ -922,8 +923,8 @@ Viewport Viewport::ToOrigin() const noexcept
         // |                          |             |                          |
         // |                          |             |                          |
         // |                          |             |                          |
-        // |                          |    ======>  |        intersect         |  ======>  early return of 0x0 Viewport
-        // |                          |             |                          |           at Original's origin
+        // |                          |    ======>  |        intersect         |  ======>  early return of nothing
+        // |                          |             |                          |
         // |                          |             |                          |
         // |                          |             |                          |
         // |---------removeMe---------|             |--------------------------|
@@ -992,39 +993,32 @@ Viewport Viewport::ToOrigin() const noexcept
         //         | removeMe      |
         //         |---------------|
 
-        if (original == intersection)
+        // We generate these rectangles by the original and intersection points, but some of them might be empty when the intersection
+        // lines up with the edge of the original. That's OK. That just means that the subtraction didn't leave anything behind.
+        // We will filter those out below when adding them to the result.
+        const auto top = Viewport({ original.Left(), original.Top(), original.RightInclusive(), intersection.Top() - 1 });
+        const auto bottom = Viewport({ original.Left(), intersection.BottomExclusive(), original.RightInclusive(), original.BottomInclusive() });
+        const auto left = Viewport({ original.Left(), intersection.Top(), intersection.Left() - 1, intersection.BottomInclusive() });
+        const auto right = Viewport({ intersection.RightExclusive(), intersection.Top(), original.RightInclusive(), intersection.BottomInclusive() });
+
+        if (top.IsValid())
         {
-            result.viewports.at(result.used++) = Viewport::FromDimensions(original.Origin(), { 0, 0 });
+            result.viewports.at(result.used++) = top;
         }
-        else
-        {
-            // We generate these rectangles by the original and intersection points, but some of them might be empty when the intersection
-            // lines up with the edge of the original. That's OK. That just means that the subtraction didn't leave anything behind.
-            // We will filter those out below when adding them to the result.
-            const auto top = Viewport({ original.Left(), original.Top(), original.RightInclusive(), intersection.Top() - 1 });
-            const auto bottom = Viewport({ original.Left(), intersection.BottomExclusive(), original.RightInclusive(), original.BottomInclusive() });
-            const auto left = Viewport({ original.Left(), intersection.Top(), intersection.Left() - 1, intersection.BottomInclusive() });
-            const auto right = Viewport({ intersection.RightExclusive(), intersection.Top(), original.RightInclusive(), intersection.BottomInclusive() });
-
-            if (top.IsValid())
-            {
-                result.viewports.at(result.used++) = top;
-            }
 
-            if (bottom.IsValid())
-            {
-                result.viewports.at(result.used++) = bottom;
-            }
+        if (bottom.IsValid())
+        {
+            result.viewports.at(result.used++) = bottom;
+        }
 
-            if (left.IsValid())
-            {
-                result.viewports.at(result.used++) = left;
-            }
+        if (left.IsValid())
+        {
+            result.viewports.at(result.used++) = left;
+        }
 
-            if (right.IsValid())
-            {
-                result.viewports.at(result.used++) = right;
-            }
+        if (right.IsValid())
+        {
+            result.viewports.at(result.used++) = right;
         }
     }