Don't reflow the alt buffer on resize (#12719)

VTE only rewraps the contents of the (normal screen + its scrollback
buffer) on a resize event. It doesn't rewrap the contents of the
alternate screen. The alternate screen is used by applications which
repaint it after a resize event. So, it doesn't really matter. However,
in that short time window, after resizing the terminal but before the
application catches up, this prevents vertical lines

It was really hard to get a gif of this where it happened and was small
enough to upload to GH, but there is one in #12719.

There's something in this branch that fixes a scrolling issue in the
parent PR. I'm partially filing this so I can look at the diffs here and
try and figure out what that is. I kinda want to just take all 3 alt
buffer PRs as a single atomic unit, but splitting them up made sense
from a review standpoint.

Closes #3493

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;
     }
 }