I thought this might fix the problems I noticed in #4354, but I don't think that's the case after all. Gonna step back and try again.

This reverts commit 4248c87a7e, reversing
changes made to 8c61481213.

src/buffer/out/textBuffer.cpp

@@ -1572,11 +1572,15 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer)
 
     short const cOldRowsTotal = cOldLastChar.Y + 1;
     short const cOldColsTotal = oldBuffer.GetSize().Width();
+    short const newCols = newBuffer.GetSize().Width();
 
     COORD cNewCursorPos = { 0 };
     bool fFoundCursorPos = false;
 
     HRESULT hr = S_OK;
+
+    std::optional<short> firstWrapChangedRow{ std::nullopt };
+
     // Loop through all the rows of the old buffer and reprint them into the new buffer
     for (short iOldRow = 0; iOldRow < cOldRowsTotal; iOldRow++)
     {
@@ -1609,6 +1613,13 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer)
             }
         }
 
+        if (!firstWrapChangedRow.has_value() &&
+            ((iRight > newCols) ||
+             charRow.WasWrapForced()))
+        {
+            firstWrapChangedRow = newCursor.GetPosition().Y;
+        }
+
         // Loop through every character in the current row (up to
         // the "right" boundary, which is one past the final valid
         // character)
@@ -1635,6 +1646,9 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer)
             }
             CATCH_RETURN();
         }
+
+        // bool manuallyNewlined = false;
+
         if (SUCCEEDED(hr))
         {
             // If we didn't have a full row to copy, insert a new
@@ -1656,6 +1670,7 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer)
                 if (iOldRow < cOldRowsTotal - 1)
                 {
                     hr = newBuffer.NewlineCursor() ? hr : E_OUTOFMEMORY;
+                    // manuallyNewlined = true;
                 }
                 else
                 {
@@ -1689,11 +1704,20 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer)
                         if (newBuffer.GetRowByOffset(gsl::narrow_cast<size_t>(coordNewCursor.Y) - 1).GetCharRow().WasWrapForced())
                         {
                             hr = newBuffer.NewlineCursor() ? hr : E_OUTOFMEMORY;
+                            // manuallyNewlined = true;
                         }
                     }
                 }
             }
         }
+
+        // if (SUCCEEDED(hr))
+        // {
+        //     if (manuallyNewlined && charRow.WasWrapForced() && !firstWrapChangedRow)
+        //     {
+
+        //     }
+        // }
     }
     if (SUCCEEDED(hr))
     {
@@ -1762,6 +1786,16 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer)
         newCursor.SetSize(ulSize);
     }
 
+    if (SUCCEEDED(hr) && firstWrapChangedRow.has_value())
+    {
+        const short invalidTop = firstWrapChangedRow.value();
+        const COORD newLastChar = newBuffer.GetLastNonSpaceCharacter();
+        const short newRowsTotal = newLastChar.Y + 1;
+
+        newBuffer._renderTarget.TriggerRedraw(
+            Viewport::FromInclusive({ 0, invalidTop, newCols, newRowsTotal }));
+    }
+
     newCursor.EndDeferDrawing();
     oldCursor.EndDeferDrawing();
 

src/cascadia/TerminalCore/Terminal.cpp

@@ -176,14 +176,40 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
         return S_FALSE;
     }
 
+    const auto dy = viewportSize.Y - oldDimensions.Y;
+
+    // We're going to attempt to "stick to the top" of where the old viewport was.
     const auto oldTop = _mutableViewport.Top();
 
     const short newBufferHeight = viewportSize.Y + _scrollbackLines;
     COORD bufferSize{ viewportSize.X, newBufferHeight };
     RETURN_IF_FAILED(_buffer->ResizeTraditional(bufferSize));
 
-    auto proposedTop = oldTop;
-    const auto newView = Viewport::FromDimensions({ 0, proposedTop }, viewportSize);
+    // However conpty resizes a little oddly - if the height decreased, and
+    // there were blank lines at the bottom, those lines will get trimmed.
+    // If there's not blank lines, then the top will get "shifted down",
+    // moving the top line into scrollback.
+    // See GH#3490 for more details.
+
+    // If the final position in the buffer is on the bottom row of the new
+    // viewport, then we're going to need to move the top down. Otherwise,
+    // move the bottom up.
+    const COORD cOldCursorPos = _buffer->GetCursor().GetPosition();
+    COORD cOldLastChar = cOldCursorPos;
+    try
+    {
+        cOldLastChar = _buffer->GetLastNonSpaceCharacter();
+    }
+    CATCH_LOG();
+
+    const auto maxRow = std::max(cOldLastChar.Y, cOldCursorPos.Y);
+
+    const bool beforeLastRow = maxRow < bufferSize.Y - 1;
+    const auto adjustment = beforeLastRow ? 0 : std::max(0, -dy);
+
+    auto proposedTop = oldTop + adjustment;
+
+    const auto newView = Viewport::FromDimensions({ 0, gsl::narrow_cast<short>(proposedTop) }, viewportSize);
     const auto proposedBottom = newView.BottomExclusive();
     // If the new bottom would be below the bottom of the buffer, then slide the
     // top up so that we'll still fit within the buffer.
@@ -192,7 +218,8 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
         proposedTop -= (proposedBottom - bufferSize.Y);
     }
 
-    _mutableViewport = Viewport::FromDimensions({ 0, proposedTop }, viewportSize);
+    _mutableViewport = Viewport::FromDimensions({ 0, gsl::narrow_cast<short>(proposedTop) }, viewportSize);
+
     _scrollOffset = 0;
     _NotifyScrollEvent();
 

src/cascadia/TerminalCore/Terminal.hpp

@@ -30,7 +30,11 @@ namespace Microsoft::Terminal::Core
 
 // fwdecl unittest classes
 #ifdef UNIT_TESTING
-class ConptyRoundtripTests;
+namespace TerminalCoreUnitTests
+{
+    class TerminalBufferTests;
+    class ConptyRoundtripTests;
+};
 #endif
 
 class Microsoft::Terminal::Core::Terminal final :
@@ -252,6 +256,7 @@ private:
 #pragma endregion
 
 #ifdef UNIT_TESTING
-    friend class ::ConptyRoundtripTests;
+    friend class TerminalCoreUnitTests::TerminalBufferTests;
+    friend class TerminalCoreUnitTests::ConptyRoundtripTests;
 #endif
 };

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -40,8 +40,17 @@ using namespace Microsoft::Console::Types;
 
 using namespace Microsoft::Terminal::Core;
 
-class ConptyRoundtripTests
+namespace TerminalCoreUnitTests
 {
+    class TerminalBufferTests;
+};
+using namespace TerminalCoreUnitTests;
+
+class TerminalCoreUnitTests::ConptyRoundtripTests final
+{
+    static const SHORT TerminalViewWidth = 80;
+    static const SHORT TerminalViewHeight = 32;
+
     TEST_CLASS(ConptyRoundtripTests);
 
     TEST_CLASS_SETUP(ClassSetup)
@@ -50,7 +59,7 @@ class ConptyRoundtripTests
 
         m_state->InitEvents();
         m_state->PrepareGlobalFont();
-        m_state->PrepareGlobalScreenBuffer();
+        m_state->PrepareGlobalScreenBuffer(TerminalViewWidth, TerminalViewHeight, TerminalViewWidth, TerminalViewHeight);
         m_state->PrepareGlobalInputBuffer();
 
         return true;
@@ -71,18 +80,19 @@ class ConptyRoundtripTests
     {
         // STEP 1: Set up the Terminal
         term = std::make_unique<Terminal>();
-        term->Create({ CommonState::s_csBufferWidth, CommonState::s_csBufferHeight }, 0, emptyRT);
+        term->Create({ TerminalViewWidth, TerminalViewHeight }, 100, emptyRT);
 
         // STEP 2: Set up the Conpty
 
         // Set up some sane defaults
         auto& g = ServiceLocator::LocateGlobals();
         auto& gci = g.getConsoleInformation();
+
         gci.SetDefaultForegroundColor(INVALID_COLOR);
         gci.SetDefaultBackgroundColor(INVALID_COLOR);
         gci.SetFillAttribute(0x07); // DARK_WHITE on DARK_BLACK
 
-        m_state->PrepareNewTextBufferInfo(true);
+        m_state->PrepareNewTextBufferInfo(true, TerminalViewWidth, TerminalViewHeight);
         auto& currentBuffer = gci.GetActiveOutputBuffer();
         // Make sure a test hasn't left us in the alt buffer on accident
         VERIFY_IS_FALSE(currentBuffer._IsAltBuffer());
@@ -106,6 +116,13 @@ class ConptyRoundtripTests
         g.pRender->AddRenderEngine(_pVtRenderEngine.get());
         gci.GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get());
 
+        _pConApi = std::make_unique<ConhostInternalGetSet>(gci);
+
+        // Manually set the console into conpty mode. We're not actually going
+        // to set up the pipes for conpty, but we want the console to behave
+        // like it would in conpty mode.
+        g.EnableConptyModeForTests();
+
         expectedOutput.clear();
 
         return true;
@@ -129,13 +146,20 @@ class ConptyRoundtripTests
     TEST_METHOD(SimpleWriteOutputTest);
     TEST_METHOD(WriteTwoLinesUsesNewline);
     TEST_METHOD(WriteAFewSimpleLines);
+    TEST_METHOD(TestResizeHeight);
 
 private:
     bool _writeCallback(const char* const pch, size_t const cch);
     void _flushFirstFrame();
+    void _resizeConpty(const unsigned short sx, const unsigned short sy);
     std::deque<std::string> expectedOutput;
     std::unique_ptr<Microsoft::Console::Render::VtEngine> _pVtRenderEngine;
     std::unique_ptr<CommonState> m_state;
+    std::unique_ptr<Microsoft::Console::VirtualTerminal::ConGetSet> _pConApi;
+
+    // Tests can set these variables how they link to configure the behavior of the test harness.
+    bool _checkConptyOutput{ true }; // If true, the test class will check that the output from conpty was expected
+    bool _logConpty{ false }; // If true, the test class will log all the output from conpty. Helpful for debugging.
 
     DummyRenderTarget emptyRT;
     std::unique_ptr<Terminal> term;
@@ -144,18 +168,27 @@ private:
 bool ConptyRoundtripTests::_writeCallback(const char* const pch, size_t const cch)
 {
     std::string actualString = std::string(pch, cch);
-    VERIFY_IS_GREATER_THAN(expectedOutput.size(),
-                           static_cast<size_t>(0),
-                           NoThrowString().Format(L"writing=\"%hs\", expecting %u strings", actualString.c_str(), expectedOutput.size()));
 
-    std::string first = expectedOutput.front();
-    expectedOutput.pop_front();
+    if (_checkConptyOutput)
+    {
+        VERIFY_IS_GREATER_THAN(expectedOutput.size(),
+                               static_cast<size_t>(0),
+                               NoThrowString().Format(L"writing=\"%hs\", expecting %u strings", actualString.c_str(), expectedOutput.size()));
 
-    Log::Comment(NoThrowString().Format(L"Expected =\t\"%hs\"", first.c_str()));
-    Log::Comment(NoThrowString().Format(L"Actual =\t\"%hs\"", actualString.c_str()));
+        std::string first = expectedOutput.front();
+        expectedOutput.pop_front();
 
-    VERIFY_ARE_EQUAL(first.length(), cch);
-    VERIFY_ARE_EQUAL(first, actualString);
+        Log::Comment(NoThrowString().Format(L"Expected =\t\"%hs\"", first.c_str()));
+        Log::Comment(NoThrowString().Format(L"Actual =\t\"%hs\"", actualString.c_str()));
+
+        VERIFY_ARE_EQUAL(first.length(), cch);
+        VERIFY_ARE_EQUAL(first, actualString);
+    }
+    else if (_logConpty)
+    {
+        Log::Comment(NoThrowString().Format(
+            L"Writing \"%hs\" to Terminal", actualString.c_str()));
+    }
 
     // Write the string back to our Terminal
     const auto converted = ConvertToW(CP_UTF8, actualString);
@@ -177,6 +210,19 @@ void ConptyRoundtripTests::_flushFirstFrame()
     VERIFY_SUCCEEDED(renderer.PaintFrame());
 }
 
+void ConptyRoundtripTests::_resizeConpty(const unsigned short sx,
+                                         const unsigned short sy)
+{
+    // Largely taken from implementation in PtySignalInputThread::_InputThread
+    if (DispatchCommon::s_ResizeWindow(*_pConApi, sx, sy))
+    {
+        // Instead of going through the VtIo to suppress the resize repaint,
+        // just call the method directly on the renderer. This is implemented in
+        // VtIo::SuppressResizeRepaint
+        VERIFY_SUCCEEDED(_pVtRenderEngine->SuppressResizeRepaint());
+    }
+}
+
 // Function Description:
 // - Helper function to validate that a number of characters in a row are all
 //   the same. Validates that the next end-start characters are all equal to the
@@ -364,3 +410,208 @@ void ConptyRoundtripTests::WriteAFewSimpleLines()
 
     verifyData(termTb);
 }
+
+void ConptyRoundtripTests::TestResizeHeight()
+{
+    // This test class is _60_ tests to ensure that resizing the terminal works
+    // with conpty correctly. There's a lot of min/maxing in expressions here,
+    // to account for the sheer number of cases here, and that we have to handle
+    // both resizing larger and smaller all in one test.
+
+    BEGIN_TEST_METHOD_PROPERTIES()
+        TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method")
+        TEST_METHOD_PROPERTY(L"Data:dx", L"{-1, 0, 1}")
+        TEST_METHOD_PROPERTY(L"Data:dy", L"{-10, -1, 0, 1, 10}")
+        TEST_METHOD_PROPERTY(L"Data:printedRows", L"{1, 10, 50, 200}")
+    END_TEST_METHOD_PROPERTIES()
+    int dx, dy;
+    int printedRows;
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"dx", dx), L"change in width of buffer");
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"dy", dy), L"change in height of buffer");
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"printedRows", printedRows), L"Number of rows of text to print");
+
+    _checkConptyOutput = false;
+
+    auto& g = ServiceLocator::LocateGlobals();
+    auto& renderer = *g.pRender;
+    auto& gci = g.getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& hostSm = si.GetStateMachine();
+    auto* hostTb = &si.GetTextBuffer();
+    auto* termTb = term->_buffer.get();
+    const auto initialHostView = si.GetViewport();
+    const auto initialTermView = term->GetViewport();
+    const auto initialTerminalBufferHeight = term->GetTextBuffer().GetSize().Height();
+
+    VERIFY_ARE_EQUAL(0, initialHostView.Top());
+    VERIFY_ARE_EQUAL(TerminalViewHeight, initialHostView.BottomExclusive());
+    VERIFY_ARE_EQUAL(0, initialTermView.Top());
+    VERIFY_ARE_EQUAL(TerminalViewHeight, initialTermView.BottomExclusive());
+
+    Log::Comment(NoThrowString().Format(
+        L"Print %d lines of output, which will scroll the viewport", printedRows));
+
+    for (auto i = 0; i < printedRows; i++)
+    {
+        // This looks insane, but this expression is carefully crafted to give
+        // us only printable characters, starting with `!` (0n33).
+        // Similar statements are used elsewhere throughout this test.
+        auto wstr = std::wstring(1, static_cast<wchar_t>((i) % 93) + 33);
+        hostSm.ProcessString(wstr);
+        hostSm.ProcessString(L"\r\n");
+    }
+
+    // Conpty doesn't have a scrollback, it's view's origin is always 0,0
+    const auto secondHostView = si.GetViewport();
+    VERIFY_ARE_EQUAL(0, secondHostView.Top());
+    VERIFY_ARE_EQUAL(TerminalViewHeight, secondHostView.BottomExclusive());
+
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    const auto secondTermView = term->GetViewport();
+    // If we've printed more lines than the height of the buffer, then we're
+    // expecting the viewport to have moved down. Otherwise, the terminal's
+    // viewport will stay at 0,0.
+    const auto expectedTerminalViewBottom = std::max(std::min(gsl::narrow_cast<short>(printedRows + 1),
+                                                              term->GetBufferHeight()),
+                                                     term->GetViewport().Height());
+
+    VERIFY_ARE_EQUAL(expectedTerminalViewBottom, secondTermView.BottomExclusive());
+    VERIFY_ARE_EQUAL(expectedTerminalViewBottom - initialTermView.Height(), secondTermView.Top());
+
+    auto verifyTermData = [&expectedTerminalViewBottom, &printedRows, this, &initialTerminalBufferHeight](TextBuffer& termTb, const int resizeDy = 0) {
+        // Some number of lines of text were lost from the scrollback. The
+        // number of lines lost will be determined by whichever of the initial
+        // or current buffer is smaller.
+        const auto numLostRows = std::max(0,
+                                          printedRows - std::min(term->GetTextBuffer().GetSize().Height(), initialTerminalBufferHeight) + 1);
+
+        const auto rowsWithText = std::min(gsl::narrow_cast<short>(printedRows),
+                                           expectedTerminalViewBottom) -
+                                  1 + std::min(resizeDy, 0);
+
+        for (short row = 0; row < rowsWithText; row++)
+        {
+            SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures);
+            auto iter = termTb.GetCellDataAt({ 0, row });
+            const wchar_t expectedChar = static_cast<wchar_t>((row + numLostRows) % 93) + 33;
+
+            auto expectedString = std::wstring(1, expectedChar);
+
+            if (iter->Chars() != expectedString)
+            {
+                Log::Comment(NoThrowString().Format(L"row [%d] was mismatched", row));
+            }
+            VERIFY_ARE_EQUAL(expectedString, (iter++)->Chars());
+            VERIFY_ARE_EQUAL(L" ", (iter)->Chars());
+        }
+    };
+    auto verifyHostData = [&si, &initialHostView, &printedRows](TextBuffer& hostTb, const int resizeDy = 0) {
+        const auto hostView = si.GetViewport();
+
+        // In the host, there are two regions we're interested in:
+
+        // 1. the first section of the buffer with the output in it. Before
+        //    we're resized, this will be filled with one character on each row.
+        // 2. The second area below the first that's empty (filled with spaces).
+        //    Initially, this is only one row.
+        // After we resize, different things will happen.
+        // * If we decrease the height of the buffer, the characters in the
+        //   buffer will all move _up_ the same number of rows. We'll want to
+        //   only check the first initialView+dy rows for characters.
+        // * If we increase the height, rows will be added at the bottom. We'll
+        //   want to check the initial viewport height for the original
+        //   characters, but then we'll want to look for more blank rows at the
+        //   bottom. The characters in the initial viewport won't have moved.
+
+        const short originalViewHeight = gsl::narrow_cast<short>(resizeDy < 0 ?
+                                                                     initialHostView.Height() + resizeDy :
+                                                                     initialHostView.Height());
+        const auto rowsWithText = std::min(originalViewHeight - 1, printedRows);
+        const bool scrolled = printedRows > initialHostView.Height();
+        // The last row of the viewport should be empty
+        // The second last row will have '0'+50
+        // The third last row will have '0'+49
+        // ...
+        // The <height> last row will have '0'+(50-height+1)
+        const auto firstChar = static_cast<wchar_t>(scrolled ?
+                                                        (printedRows - originalViewHeight + 1) :
+                                                        0);
+
+        short row = 0;
+        // Don't include the last row of the viewport in this check, since it'll
+        // be blank. We'll check it in the below loop.
+        for (; row < rowsWithText; row++)
+        {
+            SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures);
+            auto iter = hostTb.GetCellDataAt({ 0, row });
+
+            const auto expectedChar = static_cast<wchar_t>(((firstChar + row) % 93) + 33);
+            auto expectedString = std::wstring(1, static_cast<wchar_t>(expectedChar));
+
+            if (iter->Chars() != expectedString)
+            {
+                Log::Comment(NoThrowString().Format(L"row [%d] was mismatched", row));
+            }
+            VERIFY_ARE_EQUAL(expectedString, (iter++)->Chars(), NoThrowString().Format(L"%s", expectedString.data()));
+            VERIFY_ARE_EQUAL(L" ", (iter)->Chars());
+        }
+
+        // Check that the remaining rows in the viewport are empty.
+        for (; row < hostView.Height(); row++)
+        {
+            SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures);
+            auto iter = hostTb.GetCellDataAt({ 0, row });
+            VERIFY_ARE_EQUAL(L" ", (iter)->Chars());
+        }
+    };
+
+    verifyHostData(*hostTb);
+    verifyTermData(*termTb);
+
+    const COORD newViewportSize{
+        gsl::narrow_cast<short>(TerminalViewWidth + dx),
+        gsl::narrow_cast<short>(TerminalViewHeight + dy)
+    };
+
+    Log::Comment(NoThrowString().Format(L"Resize the Terminal and conpty here"));
+    auto resizeResult = term->UserResize(newViewportSize);
+    VERIFY_SUCCEEDED(resizeResult);
+    _resizeConpty(newViewportSize.X, newViewportSize.Y);
+
+    // After we resize, make sure to get the new textBuffers
+    hostTb = &si.GetTextBuffer();
+    termTb = term->_buffer.get();
+
+    // Conpty's doesn't have a scrollback, it's view's origin is always 0,0
+    const auto thirdHostView = si.GetViewport();
+    VERIFY_ARE_EQUAL(0, thirdHostView.Top());
+    VERIFY_ARE_EQUAL(newViewportSize.Y, thirdHostView.BottomExclusive());
+
+    // The Terminal should be stuck to the top of the viewport.
+    const auto thirdTermView = term->GetViewport();
+
+    VERIFY_ARE_EQUAL(secondTermView.Top(), thirdTermView.Top());
+    VERIFY_ARE_EQUAL(expectedTerminalViewBottom + dy, thirdTermView.BottomExclusive());
+
+    verifyHostData(*hostTb, dy);
+    // Note that at this point, nothing should have changed with the Terminal.
+    verifyTermData(*termTb, dy);
+
+    Log::Comment(NoThrowString().Format(L"Paint a frame to update the Terminal"));
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    // Conpty's doesn't have a scrollback, it's view's origin is always 0,0
+    const auto fourthHostView = si.GetViewport();
+    VERIFY_ARE_EQUAL(0, fourthHostView.Top());
+    VERIFY_ARE_EQUAL(newViewportSize.Y, fourthHostView.BottomExclusive());
+
+    // The Terminal should be stuck to the top of the viewport.
+    const auto fourthTermView = term->GetViewport();
+
+    VERIFY_ARE_EQUAL(secondTermView.Top(), fourthTermView.Top());
+    VERIFY_ARE_EQUAL(expectedTerminalViewBottom + dy, fourthTermView.BottomExclusive());
+
+    verifyHostData(*hostTb, dy);
+    verifyTermData(*termTb, dy);
+}

src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp

@@ -0,0 +1,108 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#include "precomp.h"
+#include <WexTestClass.h>
+
+#include "../renderer/inc/DummyRenderTarget.hpp"
+#include "../cascadia/TerminalCore/Terminal.hpp"
+#include "MockTermSettings.h"
+#include "consoletaeftemplates.hpp"
+
+using namespace winrt::Microsoft::Terminal::Settings;
+using namespace Microsoft::Terminal::Core;
+
+using namespace WEX::Common;
+using namespace WEX::Logging;
+using namespace WEX::TestExecution;
+
+namespace TerminalCoreUnitTests
+{
+    class TerminalBufferTests;
+};
+using namespace TerminalCoreUnitTests;
+
+class TerminalCoreUnitTests::TerminalBufferTests final
+{
+    TEST_CLASS(TerminalBufferTests);
+
+    TEST_METHOD(TestResizeHeight);
+
+    TEST_METHOD_SETUP(MethodSetup)
+    {
+        // STEP 1: Set up the Terminal
+        term = std::make_unique<Terminal>();
+        term->Create({ 80, 32 }, 100, emptyRT);
+        return true;
+    }
+
+    TEST_METHOD_CLEANUP(MethodCleanup)
+    {
+        term = nullptr;
+        return true;
+    }
+
+private:
+    DummyRenderTarget emptyRT;
+    std::unique_ptr<Terminal> term;
+};
+
+void TerminalBufferTests::TestResizeHeight()
+{
+    BEGIN_TEST_METHOD_PROPERTIES()
+        TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method")
+        TEST_METHOD_PROPERTY(L"Data:dy", L"{-10, -1, 0, 1, 10}")
+    END_TEST_METHOD_PROPERTIES()
+    int dy;
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"dy", dy), L"change in height of buffer");
+
+    auto& termTb = *term->_buffer;
+    auto& termSm = *term->_stateMachine;
+    const auto initialView = term->GetViewport();
+
+    VERIFY_ARE_EQUAL(0, initialView.Top());
+    VERIFY_ARE_EQUAL(32, initialView.BottomExclusive());
+
+    Log::Comment(NoThrowString().Format(
+        L"Print 50 lines of output, which will scroll the viewport"));
+
+    for (auto i = 0; i < 50; i++)
+    {
+        auto wstr = std::wstring(1, static_cast<wchar_t>(L'0' + i));
+        termSm.ProcessString(wstr);
+        termSm.ProcessString(L"\r\n");
+    }
+
+    const auto secondView = term->GetViewport();
+
+    VERIFY_ARE_EQUAL(50 - initialView.Height() + 1, secondView.Top());
+    VERIFY_ARE_EQUAL(50, secondView.BottomInclusive());
+
+    auto verifyBufferContents = [&termTb]() {
+        for (short row = 0; row < 50; row++)
+        {
+            SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures);
+            auto iter = termTb.GetCellDataAt({ 0, row });
+            auto expectedString = std::wstring(1, static_cast<wchar_t>(L'0' + row));
+
+            if (iter->Chars() != expectedString)
+            {
+                Log::Comment(NoThrowString().Format(L"row [%d] was mismatched", row));
+            }
+            VERIFY_ARE_EQUAL(expectedString, (iter++)->Chars());
+            VERIFY_ARE_EQUAL(L" ", (iter)->Chars());
+        }
+    };
+    verifyBufferContents();
+
+    auto resizeResult = term->UserResize({ 80, gsl::narrow_cast<short>(32 + dy) });
+    VERIFY_SUCCEEDED(resizeResult);
+
+    const auto thirdView = term->GetViewport();
+
+    VERIFY_ARE_EQUAL(secondView.Top(), thirdView.Top());
+    VERIFY_ARE_EQUAL(50 + dy, thirdView.BottomInclusive());
+    // VERIFY_ARE_EQUAL(50 - thirdView.Height() + 1, thirdView.Top());
+
+    verifyBufferContents();
+}

src/cascadia/UnitTests_TerminalCore/UnitTests.vcxproj

@@ -11,6 +11,7 @@
   <Import Project="$(SolutionDir)\common.openconsole.props" Condition="'$(OpenConsoleDir)'==''" />
   <Import Project="$(SolutionDir)\src\common.build.pre.props" />
   <ItemGroup>
+    <ClCompile Include="TerminalBufferTests.cpp" />
     <ClCompile Include="ScreenSizeLimitsTest.cpp" />
     <ClCompile Include="SelectionTest.cpp" />
     <ClCompile Include="InputTest.cpp" />

src/host/ConsoleArguments.cpp

@@ -638,3 +638,17 @@ void ConsoleArguments::SetExpectedSize(COORD dimensions) noexcept
         _recievedEarlySizeChange = true;
     }
 }
+
+#ifdef UNIT_TESTING
+// Method Description:
+// - This is a test helper method. It can be used to trick us into thinking
+//   we're headless (in conpty mode), even without parsing any arguments.
+// Arguments:
+// - <none>
+// Return Value:
+// - <none>
+void ConsoleArguments::EnableConptyModeForTests()
+{
+    _headless = true;
+}
+#endif

src/host/ConsoleArguments.hpp

@@ -53,6 +53,10 @@ public:
 
     void SetExpectedSize(COORD dimensions) noexcept;
 
+#ifdef UNIT_TESTING
+    void EnableConptyModeForTests();
+#endif
+
     static const std::wstring_view VT_MODE_ARG;
     static const std::wstring_view HEADLESS_ARG;
     static const std::wstring_view SERVER_HANDLE_ARG;

src/host/VtIo.cpp

@@ -206,6 +206,21 @@ bool VtIo::IsUsingVt() const
     return _objectsCreated;
 }
 
+#ifdef UNIT_TESTING
+// Method Description:
+// - This is a test helper method. It can be used to trick VtIo into responding
+//   true to `IsUsingVt`, which will cause the console host to act in conpty
+//   mode.
+// Arguments:
+// - <none>
+// Return Value:
+// - <none>
+void VtIo::EnableConptyModeForTests()
+{
+    _objectsCreated = true;
+}
+#endif
+
 // Routine Description:
 //  Potentially starts this VtIo's input thread and render engine.
 //      If the VtIo hasn't yet been given pipes, then this function will

src/host/VtIo.hpp

@@ -39,6 +39,10 @@ namespace Microsoft::Console::VirtualTerminal
         void BeginResize();
         void EndResize();
 
+#ifdef UNIT_TESTING
+        void EnableConptyModeForTests();
+#endif
+
     private:
         // After CreateIoHandlers is called, these will be invalid.
         wil::unique_hfile _hInput;

src/host/getset.cpp

@@ -776,7 +776,15 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
         {
             // TODO: MSFT: 9574827 - shouldn't we be looking at or at least logging the failure codes here? (Or making them non-void?)
             context.PostUpdateWindowSize();
-            WriteToScreen(context, context.GetViewport());
+
+            // Use WriteToScreen to invalidate the viewport with the renderer.
+            // GH#3490 - If we're in conpty mode, don't invalidate the entire
+            // viewport. In conpty mode, the VtEngine will later decide what
+            // part of the buffer actually needs to be re-sent to the terminal.
+            if (!g.getConsoleInformation().IsInVtIoMode())
+            {
+                WriteToScreen(context, context.GetViewport());
+            }
         }
         return S_OK;
     }

src/host/globals.cpp

@@ -16,3 +16,19 @@ bool Globals::IsHeadless() const
 {
     return launchArgs.IsHeadless();
 }
+
+#ifdef UNIT_TESTING
+// Method Description:
+// - This is a test helper method. It can be used to trick us into responding
+//   true to `IsHeadless`, which will cause the console host to act in conpty
+//   mode.
+// Arguments:
+// - <none>
+// Return Value:
+// - <none>
+void Globals::EnableConptyModeForTests()
+{
+    launchArgs.EnableConptyModeForTests();
+    getConsoleInformation().GetVtIo()->EnableConptyModeForTests();
+}
+#endif

src/host/globals.h

@@ -69,6 +69,10 @@ public:
 
     ApiRoutines api;
 
+#ifdef UNIT_TESTING
+    void EnableConptyModeForTests();
+#endif
+
 private:
     CONSOLE_INFORMATION ciConsoleInformation;
 };

src/host/screenInfo.cpp

@@ -1423,22 +1423,51 @@ bool SCREEN_INFORMATION::IsMaximizedY() const
     }
 
     // Save cursor's relative height versus the viewport
-    SHORT const sCursorHeightInViewportBefore = _textBuffer->GetCursor().GetPosition().Y - _viewport.Top();
+    short cursorHeightInViewportBefore = _textBuffer->GetCursor().GetPosition().Y - _viewport.Top();
 
+    // auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    // const bool isConpty = gci.IsInVtIoMode();
     HRESULT hr = TextBuffer::Reflow(*_textBuffer.get(), *newTextBuffer.get());
+    const bool widthChanged = coordNewScreenSize.X != _textBuffer->GetSize().Width();
 
     if (SUCCEEDED(hr))
     {
         Cursor& newCursor = newTextBuffer->GetCursor();
         // Adjust the viewport so the cursor doesn't wildly fly off up or down.
-        SHORT const sCursorHeightInViewportAfter = newCursor.GetPosition().Y - _viewport.Top();
+        const short cursorHeightInViewportAfter = newCursor.GetPosition().Y - _viewport.Top();
         COORD coordCursorHeightDiff = { 0 };
-        coordCursorHeightDiff.Y = sCursorHeightInViewportAfter - sCursorHeightInViewportBefore;
+        coordCursorHeightDiff.Y = cursorHeightInViewportAfter - cursorHeightInViewportBefore;
         LOG_IF_FAILED(SetViewportOrigin(false, coordCursorHeightDiff, true));
 
         _textBuffer.swap(newTextBuffer);
     }
 
+    // // GH#3490 - In conpty mode, We want to invalidate all of the viewport that
+    // // might have been below any wrapped lines, up until the last character of
+    // // the buffer. Lines that were wrapped may have been re-wrapped during this
+    // // resize, so we want them repainted to the terminal. We don't want to just
+    // // invalidate everything though - if there were blank lines at the bottom,
+    // // those can just be ignored.
+    // if (isConpty && widthChanged)
+    // {
+    //     // Loop through all the rows of the old buffer and reprint them into the new buffer
+    //     const auto bottom = std::max(_textBuffer->GetCursor().GetPosition().Y,
+    //                                  std::min(_viewport.BottomInclusive(),
+    //                                           _textBuffer->GetLastNonSpaceCharacter().Y));
+    //     bool foundWrappedLine = false;
+    //     for (short y = _viewport.Top(); y <= bottom; y++)
+    //     {
+    //         // Fetch the row and its "right" which is the last printable character.
+    //         const ROW& row = _textBuffer->GetRowByOffset(y);
+    //         const CharRow& charRow = row.GetCharRow();
+    //         if (foundWrappedLine || charRow.WasWrapForced())
+    //         {
+    //             foundWrappedLine = true;
+    //             _renderTarget.TriggerRedraw(Viewport::FromDimensions({ 0, y }, _viewport.Width(), 1));
+    //         }
+    //     }
+    // }
+
     return NTSTATUS_FROM_HRESULT(hr);
 }
 
@@ -2176,8 +2205,13 @@ void SCREEN_INFORMATION::SetDefaultAttributes(const TextAttribute& attributes,
         commandLine.UpdatePopups(attributes, popupAttributes, oldPrimaryAttributes, oldPopupAttributes);
     }
 
-    // force repaint of entire viewport
-    GetRenderTarget().TriggerRedrawAll();
+    // Force repaint of entire viewport, unless we're in conpty mode. In that
+    // case, we don't really need to force a redraw of the entire screen just
+    // because the text attributes changed.
+    if (!gci.IsInVtIoMode())
+    {
+        GetRenderTarget().TriggerRedrawAll();
+    }
 
     gci.ConsoleIme.RefreshAreaAttributes();
 

src/host/screenInfo.hpp

@@ -50,6 +50,14 @@ Revision History:
 #include "../types/IConsoleWindow.hpp"
 class ConversionAreaInfo; // forward decl window. circular reference
 
+// fwdecl unittest classes
+#ifdef UNIT_TESTING
+namespace TerminalCoreUnitTests
+{
+    class ConptyRoundtripTests;
+};
+#endif
+
 class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console::IIoProvider
 {
 public:
@@ -308,6 +316,6 @@ private:
     friend class ScreenBufferTests;
     friend class CommonState;
     friend class ConptyOutputTests;
-    friend class ConptyRoundtripTests;
+    friend class TerminalCoreUnitTests::ConptyRoundtripTests;
 #endif
 };

src/host/ut_host/ConptyOutputTests.cpp

@@ -85,6 +85,11 @@ class ConptyOutputTests
         g.pRender->AddRenderEngine(_pVtRenderEngine.get());
         gci.GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get());
 
+        // Manually set the console into conpty mode. We're not actually going
+        // to set up the pipes for conpty, but we want the console to behave
+        // like it would in conpty mode.
+        g.EnableConptyModeForTests();
+
         expectedOutput.clear();
 
         return true;

src/host/ut_host/VtRendererTests.cpp

@@ -1343,15 +1343,15 @@ void VtRendererTest::TestResize()
         VERIFY_IS_FALSE(engine->_suppressResizeRepaint);
     });
 
-    // Resize the viewport to 120x30
-    // Everything should be invalidated, and a resize message sent.
+    // Resize the viewport to 120x30. A resize message should be sent.
+    // GH#3490: Not everything will be invalidated here. The console host will
+    //   determine which lines need to be invalidated for us.
     const auto newView = Viewport::FromDimensions({ 0, 0 }, { 120, 30 });
     qExpectedInput.push_back("\x1b[8;30;120t");
 
     VERIFY_SUCCEEDED(engine->UpdateViewport(newView.ToInclusive()));
 
     TestPaint(*engine, [&]() {
-        VERIFY_ARE_EQUAL(newView, engine->_invalidRect);
         VERIFY_IS_FALSE(engine->_firstPaint);
         VERIFY_IS_FALSE(engine->_suppressResizeRepaint);
     });

src/inc/test/CommonState.hpp

@@ -81,16 +81,19 @@ public:
         }
     }
 
-    void PrepareGlobalScreenBuffer()
+    void PrepareGlobalScreenBuffer(const short viewWidth = s_csWindowWidth,
+                                   const short viewHeight = s_csWindowHeight,
+                                   const short bufferWidth = s_csBufferWidth,
+                                   const short bufferHeight = s_csBufferHeight)
     {
         CONSOLE_INFORMATION& gci = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals().getConsoleInformation();
         COORD coordWindowSize;
-        coordWindowSize.X = s_csWindowWidth;
-        coordWindowSize.Y = s_csWindowHeight;
+        coordWindowSize.X = viewWidth;
+        coordWindowSize.Y = viewHeight;
 
         COORD coordScreenBufferSize;
-        coordScreenBufferSize.X = s_csBufferWidth;
-        coordScreenBufferSize.Y = s_csBufferHeight;
+        coordScreenBufferSize.X = bufferWidth;
+        coordScreenBufferSize.Y = bufferHeight;
 
         UINT uiCursorSize = 12;
 
@@ -143,12 +146,14 @@ public:
         gci.SetCookedReadData(nullptr);
     }
 
-    void PrepareNewTextBufferInfo(const bool useDefaultAttributes = false)
+    void PrepareNewTextBufferInfo(const bool useDefaultAttributes = false,
+                                  const short bufferWidth = s_csBufferWidth,
+                                  const short bufferHeight = s_csBufferHeight)
     {
         CONSOLE_INFORMATION& gci = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals().getConsoleInformation();
         COORD coordScreenBufferSize;
-        coordScreenBufferSize.X = s_csBufferWidth;
-        coordScreenBufferSize.Y = s_csBufferHeight;
+        coordScreenBufferSize.X = bufferWidth;
+        coordScreenBufferSize.Y = bufferHeight;
 
         UINT uiCursorSize = 12;
 

src/renderer/vt/state.cpp

@@ -37,7 +37,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
     _lastViewport(initialViewport),
     _invalidRect(Viewport::Empty()),
     _fInvalidRectUsed(false),
-    _lastRealCursor({ 0 }),
     _lastText({ 0 }),
     _scrollDelta({ 0 }),
     _quickReturn(false),
@@ -278,38 +277,12 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
     //      lead to the first _actual_ resize being suppressed.
     _suppressResizeRepaint = false;
 
-    if (SUCCEEDED(hr))
-    {
-        // Viewport is smaller now - just update it all.
-        if (oldView.Height() > newView.Height() || oldView.Width() > newView.Width())
-        {
-            hr = InvalidateAll();
-        }
-        else
-        {
-            // At least one of the directions grew.
-            // First try and add everything to the right of the old viewport,
-            //      then everything below where the old viewport ended.
-            if (oldView.Width() < newView.Width())
-            {
-                short left = oldView.RightExclusive();
-                short top = 0;
-                short right = newView.RightInclusive();
-                short bottom = oldView.BottomInclusive();
-                Viewport rightOfOldViewport = Viewport::FromInclusive({ left, top, right, bottom });
-                hr = _InvalidCombine(rightOfOldViewport);
-            }
-            if (SUCCEEDED(hr) && oldView.Height() < newView.Height())
-            {
-                short left = 0;
-                short top = oldView.BottomExclusive();
-                short right = newView.RightInclusive();
-                short bottom = newView.BottomInclusive();
-                Viewport belowOldViewport = Viewport::FromInclusive({ left, top, right, bottom });
-                hr = _InvalidCombine(belowOldViewport);
-            }
-        }
-    }
+    // GH#3490 - When the viewport width changed, don't do anything extra here.
+    // If the buffer had areas that were invalid due to the resize, then the
+    // buffer will have triggered it's own invalidations for what it knows is
+    // invalid. Previously, we'd invalidate everything if the width changed,
+    // because we couldn't be sure if lines were reflowed.
+
     _resized = true;
     return hr;
 }

src/renderer/vt/vtrenderer.hpp

@@ -26,7 +26,10 @@ Author(s):
 
 // fwdecl unittest classes
 #ifdef UNIT_TESTING
-class ConptyRoundtripTests;
+namespace TerminalCoreUnitTests
+{
+    class ConptyRoundtripTests;
+};
 #endif
 
 namespace Microsoft::Console::Render
@@ -116,7 +119,6 @@ namespace Microsoft::Console::Render
         Microsoft::Console::Types::Viewport _invalidRect;
 
         bool _fInvalidRectUsed;
-        COORD _lastRealCursor;
         COORD _lastText;
         COORD _scrollDelta;
 
@@ -226,7 +228,7 @@ namespace Microsoft::Console::Render
 
         friend class VtRendererTest;
         friend class ConptyOutputTests;
-        friend class ConptyRoundtripTests;
+        friend class TerminalCoreUnitTests::ConptyRoundtripTests;
 #endif
 
         void SetTestCallback(_In_ std::function<bool(const char* const, size_t const)> pfn);