Fix VT scroll margins leaking between pages

src/buffer/out/textBuffer.cpp

@@ -275,6 +275,7 @@ ROW& TextBuffer::GetScratchpadRow(const TextAttribute& attributes)
 void TextBuffer::CopyProperties(const TextBuffer& OtherBuffer) noexcept
 {
     GetCursor().CopyProperties(OtherBuffer.GetCursor());
+    _scrollMargins = OtherBuffer._scrollMargins;
 }
 
 // Routine Description:
@@ -906,6 +907,16 @@ void TextBuffer::SetCurrentAttributes(const TextAttribute& currentAttributes) no
     _currentAttributes = currentAttributes;
 }
 
+til::inclusive_rect TextBuffer::GetScrollMargins() const noexcept
+{
+    return _scrollMargins;
+}
+
+void TextBuffer::SetScrollMargins(const til::inclusive_rect& scrollMargins) noexcept
+{
+    _scrollMargins = scrollMargins;
+}
+
 void TextBuffer::SetWrapForced(const til::CoordType y, bool wrap)
 {
     GetMutableRowByOffset(y).SetWrapForced(wrap);

src/buffer/out/textBuffer.hpp

@@ -143,6 +143,9 @@ class TextBuffer final
 
     void SetCurrentAttributes(const TextAttribute& currentAttributes) noexcept;
 
+    til::inclusive_rect GetScrollMargins() const noexcept;
+    void SetScrollMargins(const til::inclusive_rect& scrollMargins) noexcept;
+
     void SetWrapForced(til::CoordType y, bool wrap);
     void SetCurrentLineRendition(const LineRendition lineRendition, const TextAttribute& fillAttributes);
     void ResetLineRenditionRange(const til::CoordType startRow, const til::CoordType endRow);
@@ -405,6 +408,7 @@ class TextBuffer final
     uint16_t _height = 0;
 
     TextAttribute _currentAttributes;
+    til::inclusive_rect _scrollMargins;
     til::CoordType _firstRow = 0; // indexes top row (not necessarily 0)
     uint64_t _lastMutationId = 0;
 

src/host/ut_host/ScreenBufferTests.cpp

@@ -124,6 +124,7 @@ class ScreenBufferTests
 
     TEST_METHOD(VtSoftResetCursorPosition);
     TEST_METHOD(VtSoftResetAltBufferCursorState);
+    TEST_METHOD(VtScrollMarginsAltBufferInheritance);
 
     TEST_METHOD(VtScrollMarginsNewlineColor);
 
@@ -1535,6 +1536,25 @@ void ScreenBufferTests::VtSoftResetAltBufferCursorState()
     VERIFY_ARE_EQUAL(til::point(6, 3), gci.GetActiveOutputBuffer().GetTextBuffer().GetCursor().GetPosition());
 }
 
+void ScreenBufferTests::VtScrollMarginsAltBufferInheritance()
+{
+    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    gci.LockConsole();
+    auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); });
+
+    auto& stateMachine = gci.GetActiveOutputBuffer().GetStateMachine();
+
+    Log::Comment(L"Set margins on the main buffer and enter the alternate buffer.");
+    stateMachine.ProcessString(L"\x1b[1;15r\x1b[?1049h\x1b[999B");
+    VERIFY_IS_TRUE(gci.GetActiveOutputBuffer()._IsAltBuffer());
+    VERIFY_ARE_EQUAL(til::point(0, 14), gci.GetActiveOutputBuffer().GetTextBuffer().GetCursor().GetPosition());
+
+    Log::Comment(L"Change margins on the alternate buffer and return to the main buffer.");
+    stateMachine.ProcessString(L"\x1b[1;10r\x1b[?1049l\x1b[999B");
+    VERIFY_IS_FALSE(gci.GetActiveOutputBuffer()._IsAltBuffer());
+    VERIFY_ARE_EQUAL(til::point(0, 9), gci.GetActiveOutputBuffer().GetTextBuffer().GetCursor().GetPosition());
+}
+
 void ScreenBufferTests::VtScrollMarginsNewlineColor()
 {
     auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();

src/host/ut_host/TextBufferTests.cpp

@@ -369,12 +369,16 @@ void TextBufferTests::TestCopyProperties()
     testTextBuffer->GetCursor().SetIsDouble(false);
     otherTbi.GetCursor().SetIsDouble(true);
 
+    testTextBuffer->SetScrollMargins({});
+    otherTbi.SetScrollMargins({ 5, 3, 70, 20 });
+
     // run copy
     testTextBuffer->CopyProperties(otherTbi);
 
     // test that new now contains values from other
     VERIFY_IS_TRUE(testTextBuffer->GetCursor().IsVisible());
     VERIFY_IS_TRUE(testTextBuffer->GetCursor().IsDouble());
+    VERIFY_ARE_EQUAL(otherTbi.GetScrollMargins(), testTextBuffer->GetScrollMargins());
 }
 
 void TextBufferTests::TestLastNonSpace(const til::CoordType cursorPosY)

src/terminal/adapter/PageManager.cpp

@@ -8,6 +8,24 @@
 
 using namespace Microsoft::Console::VirtualTerminal;
 
+namespace
+{
+    til::inclusive_rect getEffectiveMargins(const TextBuffer& buffer, const til::size pageSize) noexcept
+    {
+        const auto scrollMargins = buffer.GetScrollMargins();
+        const auto rightmostColumn = buffer.GetSize().Width() - 1;
+        const auto bottommostRow = pageSize.height - 1;
+        const auto horizontalMarginsSet = scrollMargins.left < scrollMargins.right && scrollMargins.left < rightmostColumn;
+        const auto verticalMarginsSet = scrollMargins.top < scrollMargins.bottom && scrollMargins.top < bottommostRow;
+        return {
+            horizontalMarginsSet ? scrollMargins.left : 0,
+            verticalMarginsSet ? scrollMargins.top : 0,
+            horizontalMarginsSet ? std::min(scrollMargins.right, rightmostColumn) : rightmostColumn,
+            verticalMarginsSet ? std::min(scrollMargins.bottom, bottommostRow) : bottommostRow,
+        };
+    }
+}
+
 Page::Page(TextBuffer& buffer, const til::rect& viewport, const til::CoordType number) noexcept :
     _buffer{ buffer },
     _viewport{ viewport },
@@ -149,7 +167,7 @@ Page PageManager::VisiblePage() const
     return Get(_visiblePageNumber);
 }
 
-void PageManager::MoveTo(const til::CoordType pageNumber, const bool makeVisible)
+void PageManager::MoveTo(const til::CoordType pageNumber, const bool makeVisible, const bool originMode)
 {
     auto [visibleBuffer, visibleViewport, isMainBuffer] = _api.GetBufferAndViewport();
     if (!isMainBuffer)
@@ -161,6 +179,8 @@ void PageManager::MoveTo(const til::CoordType pageNumber, const bool makeVisible
     const auto visibleTop = visibleViewport.top;
     const auto wasVisible = _activePageNumber == _visiblePageNumber;
     const auto newPageNumber = std::min(std::max(pageNumber, 1), MAX_PAGES);
+    // This must be captured before a visible page swap replaces the margins in visibleBuffer.
+    const auto oldMargins = getEffectiveMargins(wasVisible ? visibleBuffer : _getBuffer(_activePageNumber, pageSize), pageSize);
     auto redrawRequired = false;
 
     // If we're changing the visible page, what we do is swap out the current
@@ -179,6 +199,8 @@ void PageManager::MoveTo(const til::CoordType pageNumber, const bool makeVisible
         {
             newBuffer.CopyRow(i, visibleTop + i, visibleBuffer);
         }
+        saveBuffer.SetScrollMargins(visibleBuffer.GetScrollMargins());
+        visibleBuffer.SetScrollMargins(newBuffer.GetScrollMargins());
         _visiblePageNumber = newPageNumber;
         redrawRequired = true;
     }
@@ -187,24 +209,17 @@ void PageManager::MoveTo(const til::CoordType pageNumber, const bool makeVisible
     // there is no need to update any buffer properties, because we'll have
     // been using the main buffer in both cases.
     const auto isVisible = newPageNumber == _visiblePageNumber;
+    auto& oldBuffer = wasVisible ? visibleBuffer : _getBuffer(_activePageNumber, pageSize);
+    auto& newBuffer = isVisible ? visibleBuffer : _getBuffer(newPageNumber, pageSize);
     if (!wasVisible || !isVisible)
     {
         // Otherwise we need to copy the properties from the old buffer to the
         // new, so we retain the current attributes and cursor position. This
         // is only needed if they are actually different.
-        auto& oldBuffer = wasVisible ? visibleBuffer : _getBuffer(_activePageNumber, pageSize);
-        auto& newBuffer = isVisible ? visibleBuffer : _getBuffer(newPageNumber, pageSize);
         if (&oldBuffer != &newBuffer)
         {
-            // When copying the cursor position, we need to adjust the y
-            // coordinate to account for scrollback in the visible buffer.
-            const auto oldTop = wasVisible ? visibleTop : 0;
-            const auto newTop = isVisible ? visibleTop : 0;
-            auto position = oldBuffer.GetCursor().GetPosition();
-            position.y = position.y - oldTop + newTop;
             newBuffer.SetCurrentAttributes(oldBuffer.GetCurrentAttributes());
-            newBuffer.CopyProperties(oldBuffer);
-            newBuffer.GetCursor().SetPosition(position);
+            newBuffer.GetCursor().CopyProperties(oldBuffer.GetCursor());
         }
         // If we moved from the visible buffer to a background buffer we need
         // to hide the cursor in the visible buffer. This is because the page
@@ -217,23 +232,42 @@ void PageManager::MoveTo(const til::CoordType pageNumber, const bool makeVisible
         }
     }
 
+    // Adjust the cursor when changing the active page, or when transferring an
+    // unchanged active page from its backing buffer into the visible buffer.
+    if (_activePageNumber != newPageNumber || (makeVisible && !wasVisible))
+    {
+        const auto oldTop = wasVisible ? visibleTop : 0;
+        const auto newTop = isVisible ? visibleTop : 0;
+        auto position = oldBuffer.GetCursor().GetPosition();
+        position.y = position.y - oldTop + newTop;
+        if (originMode)
+        {
+            const auto newMargins = getEffectiveMargins(newBuffer, pageSize);
+            position.x += newMargins.left - oldMargins.left;
+            position.y += newMargins.top - oldMargins.top;
+            position.x = std::min(position.x, newMargins.right);
+            position.y = std::min(position.y, newTop + newMargins.bottom);
+        }
+        newBuffer.GetCursor().SetPosition(position);
+    }
+
     _activePageNumber = newPageNumber;
     if (redrawRequired && _renderer)
     {
         _renderer->TriggerRedrawAll();
     }
 }
 
-void PageManager::MoveRelative(const til::CoordType pageCount, const bool makeVisible)
+void PageManager::MoveRelative(const til::CoordType pageCount, const bool makeVisible, const bool originMode)
 {
-    MoveTo(_activePageNumber + pageCount, makeVisible);
+    MoveTo(_activePageNumber + pageCount, makeVisible, originMode);
 }
 
-void PageManager::MakeActivePageVisible()
+void PageManager::MakeActivePageVisible(const bool originMode)
 {
     if (_activePageNumber != _visiblePageNumber)
     {
-        MoveTo(_activePageNumber, true);
+        MoveTo(_activePageNumber, true, originMode);
     }
 }
 

src/terminal/adapter/PageManager.hpp

@@ -52,9 +52,9 @@ namespace Microsoft::Console::VirtualTerminal
         Page Get(const til::CoordType pageNumber) const;
         Page ActivePage() const;
         Page VisiblePage() const;
-        void MoveTo(const til::CoordType pageNumber, const bool makeVisible);
-        void MoveRelative(const til::CoordType pageCount, const bool makeVisible);
-        void MakeActivePageVisible();
+        void MoveTo(const til::CoordType pageNumber, const bool makeVisible, const bool originMode);
+        void MoveRelative(const til::CoordType pageCount, const bool makeVisible, const bool originMode);
+        void MakeActivePageVisible(const bool originMode);
 
     private:
         TextBuffer& _getBuffer(const til::CoordType pageNumber, const til::size pageSize) const;

src/terminal/adapter/SixelParser.cpp

@@ -284,7 +284,7 @@ bool SixelParser::_initTextBufferBoundaries()
         // be inside the horizontal margins and above the bottom margin, else
         // nothing will be rendered.
         const auto [topMargin, bottomMargin] = _dispatcher._GetVerticalMargins(page, true);
-        const auto [leftMargin, rightMargin] = _dispatcher._GetHorizontalMargins(page.Width());
+        const auto [leftMargin, rightMargin] = _dispatcher._GetHorizontalMargins(page, page.Width());
         _textMargins = til::rect{ leftMargin, topMargin, rightMargin + 1, bottomMargin + 1 };
         _textCursor = page.Cursor().GetPosition();
         _availablePixelWidth = (_textMargins.right - _textCursor.x) * _cellSize.width;