Don't reflow the alt buffer on resize

src/cascadia/TerminalCore/Terminal.cpp

@@ -228,12 +228,38 @@ void Terminal::UpdateAppearance(const ICoreAppearance& appearance)
 //      appropriate HRESULT for failing to resize.
 [[nodiscard]] HRESULT Terminal::UserResize(const COORD viewportSize) noexcept
 {
-    const auto oldDimensions = _mutableViewport.Dimensions();
+    const auto oldDimensions = _GetMutableViewport().Dimensions();
     if (viewportSize == oldDimensions)
     {
         return S_FALSE;
     }
 
+    // Shortcut: if we're in the alt buffer, just resize the
+    // alt buffer and put off resizing the main buffer till we switch back. Fortunately, this is easy. We don't need to
+    // worry about the viewport and scrollback at all! The alt buffer never has
+    // any scrollback, so we just need to resize it and presto, we're done.
+    if (_inAltBuffer())
+    {
+        // stash this resize for the future.
+        _deferredResize = til::size{ viewportSize };
+
+        _altBuffer->GetCursor().StartDeferDrawing();
+        // we're capturing `this` here because when we exit, we want to EndDefer on the (newly created) active buffer.
+        auto endDefer = wil::scope_exit([this]() noexcept { _altBuffer->GetCursor().EndDeferDrawing(); });
+
+        // GH#3494: We don't need to reflow the alt buffer. Apps that use the
+        // alt buffer will redraw themselves. This prevents graphical artifacts.
+        //
+        // This is consistent with VTE
+        RETURN_IF_FAILED(_altBuffer->ResizeTraditional(viewportSize));
+
+        // Since the _mutableViewport is no longer the size of the actual
+        // viewport, then update our _altBufferSize tracker we're using to help
+        // us out here.
+        _altBufferSize = til::size{ viewportSize };
+        return S_OK;
+    }
+
     const auto dx = ::base::ClampSub(viewportSize.X, oldDimensions.X);
     const short newBufferHeight = ::base::ClampAdd(viewportSize.Y, _scrollbackLines);
 
@@ -415,48 +441,6 @@ void Terminal::UpdateAppearance(const ICoreAppearance& appearance)
     // before, and shouldn't be now either.
     _scrollOffset = originalOffsetWasZero ? 0 : static_cast<int>(::base::ClampSub(_mutableViewport.Top(), newVisibleTop));
 
-    // Now that we've finished the hard work of resizing the main buffer and
-    // getting the viewport back into the right spot, we need to ALSO resize the
-    // alt buffer, if one exists. Fortunately, this is easy. We don't need to
-    // worry about the viewport and scrollback at all! The alt buffer never has
-    // any scrollback, so we just need to resize it and presto, we're done.
-    if (_inAltBuffer())
-    {
-        _altBuffer->GetCursor().StartDeferDrawing();
-        // we're capturing `this` here because when we exit, we want to EndDefer on the (newly created) active buffer.
-        auto endDefer = wil::scope_exit([this]() noexcept { _altBuffer->GetCursor().EndDeferDrawing(); });
-
-        // First allocate a new text buffer to take the place of the current one.
-        std::unique_ptr<TextBuffer> newTextBuffer;
-        try
-        {
-            // GH#3848 - Stash away the current attributes
-            const auto oldBufferAttributes = _altBuffer->GetCurrentAttributes();
-            newTextBuffer = std::make_unique<TextBuffer>(bufferSize,
-                                                         TextAttribute{},
-                                                         0, // temporarily set size to 0 so it won't render.
-                                                         true,
-                                                         _mainBuffer->GetRenderer());
-
-            // start defer drawing on the new buffer
-            newTextBuffer->GetCursor().StartDeferDrawing();
-
-            // We don't need any fancy position information. We're just gonna
-            // resize the buffer, it's gonna be in exactly the place it is now.
-            // There's no scrollback to worry about!
-
-            RETURN_IF_FAILED(TextBuffer::Reflow(*_altBuffer.get(),
-                                                *newTextBuffer.get(),
-                                                _GetMutableViewport(),
-                                                std::nullopt));
-
-            // Restore the active text attributes
-            newTextBuffer->SetCurrentAttributes(oldBufferAttributes);
-            _altBuffer.swap(newTextBuffer);
-        }
-        CATCH_RETURN();
-    }
-
     // GH#5029 - make sure to InvalidateAll here, so that we'll paint the entire visible viewport.
     try
     {
@@ -723,7 +707,7 @@ bool Terminal::SendMouseEvent(const COORD viewportPos, const unsigned int uiButt
     // clamp them to be within the range [(0, 0), (W, H)].
 #pragma warning(suppress : 26496) // analysis can't tell we're assigning through a reference below
     auto clampedPos{ viewportPos };
-    _mutableViewport.ToOrigin().Clamp(clampedPos);
+    _GetMutableViewport().ToOrigin().Clamp(clampedPos);
     return _terminalInput->HandleMouse(clampedPos, uiButton, GET_KEYSTATE_WPARAM(states.Value()), wheelDelta, state);
 }
 
@@ -951,7 +935,10 @@ WORD Terminal::_TakeVirtualKeyFromLastKeyEvent(const WORD scanCode) noexcept
 
 Viewport Terminal::_GetMutableViewport() const noexcept
 {
-    return _inAltBuffer() ? Viewport::FromDimensions(_mutableViewport.Dimensions()) :
+    // GH#3493: if we're in the alt buffer, then it's possible that the mutable
+    // viewport's size hasn't been updated yet. In that case, use the
+    // temporarily stashed _altBufferSize instead.
+    return _inAltBuffer() ? Viewport::FromDimensions(_altBufferSize.to_win32_coord()) :
                             _mutableViewport;
 }
 
@@ -968,7 +955,7 @@ int Terminal::ViewStartIndex() const noexcept
 
 int Terminal::ViewEndIndex() const noexcept
 {
-    return _inAltBuffer() ? _mutableViewport.Height() - 1 : _mutableViewport.BottomInclusive();
+    return _inAltBuffer() ? _altBufferSize.height : _mutableViewport.BottomInclusive();
 }
 
 // _VisibleStartIndex is the first visible line of the buffer
@@ -986,9 +973,14 @@ int Terminal::_VisibleEndIndex() const noexcept
 
 Viewport Terminal::_GetVisibleViewport() const noexcept
 {
+    // GH#3493: if we're in the alt buffer, then it's possible that the mutable
+    // viewport's size hasn't been updated yet. In that case, use the
+    // temporarily stashed _altBufferSize instead.
     const COORD origin{ 0, gsl::narrow<short>(_VisibleStartIndex()) };
+    const COORD size{ _inAltBuffer() ? _altBufferSize.to_win32_coord() :
+                                       _mutableViewport.Dimensions() };
     return Viewport::FromDimensions(origin,
-                                    _mutableViewport.Dimensions());
+                                    size);
 }
 
 // Writes a string of text to the buffer, then moves the cursor (and viewport)
@@ -1121,46 +1113,50 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition)
     cursor.SetPosition(proposedCursorPosition);
 
     // Move the viewport down if the cursor moved below the viewport.
-    bool updatedViewport = false;
-    const auto scrollAmount = std::max(0, proposedCursorPosition.Y - _mutableViewport.BottomInclusive());
-    if (scrollAmount > 0)
+    // Obviously, don't need to do this in the alt buffer.
+    if (!_inAltBuffer())
     {
-        const auto newViewTop = std::max(0, proposedCursorPosition.Y - (_mutableViewport.Height() - 1));
-        // In the alt buffer, we never need to adjust _mutableViewport, which is the viewport of the main buffer.
-        if (!_inAltBuffer() && newViewTop != _mutableViewport.Top())
+        bool updatedViewport = false;
+        const auto scrollAmount = std::max(0, proposedCursorPosition.Y - _mutableViewport.BottomInclusive());
+        if (scrollAmount > 0)
         {
-            _mutableViewport = Viewport::FromDimensions({ 0, gsl::narrow<short>(newViewTop) },
-                                                        _mutableViewport.Dimensions());
-            updatedViewport = true;
+            const auto newViewTop = std::max(0, proposedCursorPosition.Y - (_mutableViewport.Height() - 1));
+            // In the alt buffer, we never need to adjust _mutableViewport, which is the viewport of the main buffer.
+            if (newViewTop != _mutableViewport.Top())
+            {
+                _mutableViewport = Viewport::FromDimensions({ 0, gsl::narrow<short>(newViewTop) },
+                                                            _mutableViewport.Dimensions());
+                updatedViewport = true;
+            }
         }
-    }
 
-    // If the viewport moved, or we circled the buffer, we might need to update
-    // our _scrollOffset
-    if (!_inAltBuffer() && (updatedViewport || newRows != 0))
-    {
-        const auto oldScrollOffset = _scrollOffset;
+        // If the viewport moved, or we circled the buffer, we might need to update
+        // our _scrollOffset
+        if (updatedViewport || newRows != 0)
+        {
+            const auto oldScrollOffset = _scrollOffset;
 
-        // scroll if...
-        //   - no selection is active
-        //   - viewport is already at the bottom
-        const bool scrollToOutput = !IsSelectionActive() && _scrollOffset == 0;
+            // scroll if...
+            //   - no selection is active
+            //   - viewport is already at the bottom
+            const bool scrollToOutput = !IsSelectionActive() && _scrollOffset == 0;
 
-        _scrollOffset = scrollToOutput ? 0 : _scrollOffset + scrollAmount + newRows;
+            _scrollOffset = scrollToOutput ? 0 : _scrollOffset + scrollAmount + newRows;
 
-        // Clamp the range to make sure that we don't scroll way off the top of the buffer
-        _scrollOffset = std::clamp(_scrollOffset,
-                                   0,
-                                   _activeBuffer().GetSize().Height() - _mutableViewport.Height());
+            // Clamp the range to make sure that we don't scroll way off the top of the buffer
+            _scrollOffset = std::clamp(_scrollOffset,
+                                       0,
+                                       _activeBuffer().GetSize().Height() - _mutableViewport.Height());
 
-        // If the new scroll offset is different, then we'll still want to raise a scroll event
-        updatedViewport = updatedViewport || (oldScrollOffset != _scrollOffset);
-    }
+            // If the new scroll offset is different, then we'll still want to raise a scroll event
+            updatedViewport = updatedViewport || (oldScrollOffset != _scrollOffset);
+        }
 
-    // If the viewport moved, then send a scrolling notification.
-    if (updatedViewport)
-    {
-        _NotifyScrollEvent();
+        // If the viewport moved, then send a scrolling notification.
+        if (updatedViewport)
+        {
+            _NotifyScrollEvent();
+        }
     }
 
     if (rowsPushedOffTopOfBuffer != 0)

src/cascadia/TerminalCore/Terminal.hpp

@@ -328,6 +328,9 @@ class Microsoft::Terminal::Core::Terminal final :
     SHORT _scrollbackLines;
     bool _detectURLs{ false };
 
+    til::size _altBufferSize;
+    std::optional<til::size> _deferredResize{ std::nullopt };
+
     // _scrollOffset is the number of lines above the viewport that are currently visible
     // If _scrollOffset is 0, then the visible region of the buffer is the viewport.
     int _scrollOffset;

src/cascadia/TerminalCore/TerminalApi.cpp

@@ -598,14 +598,15 @@ void Terminal::PopGraphicsRendition()
 void Terminal::UseAlternateScreenBuffer()
 {
     // the new alt buffer is exactly the size of the viewport.
-    const COORD bufferSize{ _mutableViewport.Dimensions() };
+    _altBufferSize = til::size{ _mutableViewport.Dimensions() };
+
     const auto cursorSize = _mainBuffer->GetCursor().GetSize();
 
     ClearSelection();
     _mainBuffer->ClearPatternRecognizers();
 
     // Create a new alt buffer
-    _altBuffer = std::make_unique<TextBuffer>(bufferSize,
+    _altBuffer = std::make_unique<TextBuffer>(_altBufferSize.to_win32_coord(),
                                               TextAttribute{},
                                               cursorSize,
                                               true,
@@ -674,6 +675,12 @@ void Terminal::UseMainScreenBuffer()
     // destroy the alt buffer
     _altBuffer = nullptr;
 
+    if (_deferredResize.has_value())
+    {
+        LOG_IF_FAILED(UserResize(_deferredResize.value().to_win32_coord()));
+        _deferredResize = std::nullopt;
+    }
+
     // update all the hyperlinks on the screen
     _mainBuffer->ClearPatternRecognizers();
     _updateUrlDetection();

src/cascadia/TerminalCore/TerminalSelection.cpp

@@ -426,15 +426,15 @@ void Terminal::_MoveByViewport(SelectionDirection direction, COORD& pos)
         break;
     case SelectionDirection::Up:
     {
-        const auto viewportHeight{ _mutableViewport.Height() };
+        const auto viewportHeight{ _GetMutableViewport().Height() };
         const auto newY{ base::ClampSub<short, short>(pos.Y, viewportHeight) };
         pos = newY < bufferSize.Top() ? bufferSize.Origin() : COORD{ pos.X, newY };
         break;
     }
     case SelectionDirection::Down:
     {
-        const auto viewportHeight{ _mutableViewport.Height() };
-        const auto mutableBottom{ _mutableViewport.BottomInclusive() };
+        const auto viewportHeight{ _GetMutableViewport().Height() };
+        const auto mutableBottom{ _GetMutableViewport().BottomInclusive() };
         const auto newY{ base::ClampAdd<short, short>(pos.Y, viewportHeight) };
         pos = newY > mutableBottom ? COORD{ bufferSize.RightInclusive(), mutableBottom } : COORD{ pos.X, newY };
         break;
@@ -453,7 +453,7 @@ void Terminal::_MoveByBuffer(SelectionDirection direction, COORD& pos)
         break;
     case SelectionDirection::Right:
     case SelectionDirection::Down:
-        pos = { bufferSize.RightInclusive(), _mutableViewport.BottomInclusive() };
+        pos = { bufferSize.RightInclusive(), _GetMutableViewport().BottomInclusive() };
         break;
     }
 }