Initialize the text buffer with the default attributes on a resize

src/cascadia/TerminalCore/Terminal.cpp

@@ -245,8 +245,14 @@ void Terminal::UpdateAppearance(const ICoreAppearance& appearance)
     std::unique_ptr<TextBuffer> newTextBuffer;
     try
     {
+        // GH#3848 - Stash away the current attributes the old text buffer is
+        // using. We'll initialize the new buffer with the default attributes,
+        // but after the resize, we'll want to make sure that the new buffer's
+        // current attributes (the ones used for printing new text) match the
+        // old buffer's.
+        const auto oldBufferAttributes = _buffer->GetCurrentAttributes();
         newTextBuffer = std::make_unique<TextBuffer>(bufferSize,
-                                                     _buffer->GetCurrentAttributes(),
+                                                     TextAttribute{},
                                                      0, // temporarily set size to 0 so it won't render.
                                                      _buffer->GetRenderTarget());
 
@@ -274,6 +280,9 @@ void Terminal::UpdateAppearance(const ICoreAppearance& appearance)
 
         newViewportTop = oldRows.mutableViewportTop;
         newVisibleTop = oldRows.visibleViewportTop;
+
+        // Restore the active text attributes
+        newTextBuffer->SetCurrentAttributes(oldBufferAttributes);
     }
     CATCH_RETURN();
 

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -216,6 +216,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
 
     TEST_METHOD(HyperlinkIdConsistency);
 
+    TEST_METHOD(ResizeInitializeBufferWithDefaultAttrs);
+
 private:
     bool _writeCallback(const char* const pch, size_t const cch);
     void _flushFirstFrame();
@@ -2816,7 +2818,7 @@ void ConptyRoundtripTests::TestResizeWithCookedRead()
     // Don't let the cooked read pollute other tests
     BEGIN_TEST_METHOD_PROPERTIES()
         TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method")
-        TEST_METHOD_PROPERTY(L"Data:dx", L"{-10, -1, 0, 1, -10}")
+        TEST_METHOD_PROPERTY(L"Data:dx", L"{-10, -1, 0, 1, 10}")
         TEST_METHOD_PROPERTY(L"Data:dy", L"{-10, -1, 0, 1, 10}")
     END_TEST_METHOD_PROPERTIES()
 
@@ -2855,6 +2857,153 @@ void ConptyRoundtripTests::TestResizeWithCookedRead()
     // By simply reaching the end of this test, we know that we didn't crash. Hooray!
 }
 
+void ConptyRoundtripTests::ResizeInitializeBufferWithDefaultAttrs()
+{
+    // See https://github.com/microsoft/terminal/issues/3848
+    Log::Comment(L"This test checks that the attributes in the text buffer are "
+                 L"initialized to a sensible value during a resize. The entire "
+                 L"buffer shouldn't be filled with _whatever the current "
+                 L"attributes are_, it should be filled with the default "
+                 L"attributes (however the application defines that). Then, "
+                 L"after the resize, we should still be able to print to the "
+                 L"buffer with the old \"current attributes\"");
+
+    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"{-1, 0, 1}")
+        TEST_METHOD_PROPERTY(L"Data:leaveTrailingChar", L"{false, true}")
+    END_TEST_METHOD_PROPERTIES()
+
+    INIT_TEST_PROPERTY(int, dx, L"The change in width of the buffer");
+    INIT_TEST_PROPERTY(int, dy, L"The change in height of the buffer");
+    INIT_TEST_PROPERTY(bool, leaveTrailingChar, L"If true, we'll print one additional '#' on row 3");
+
+    // Do nothing if the resize would just be a no-op.
+    if (dx == 0 && dy == 0)
+    {
+        return;
+    }
+
+    auto& g = ServiceLocator::LocateGlobals();
+    auto& renderer = *g.pRender;
+    auto& gci = g.getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& sm = si.GetStateMachine();
+    auto* hostTb = &si.GetTextBuffer();
+    auto* termTb = term->_buffer.get();
+
+    _flushFirstFrame();
+
+    _checkConptyOutput = false;
+    _logConpty = true;
+
+    auto defaultAttrs = si.GetAttributes();
+    auto conhostGreenAttrs = TextAttribute();
+
+    // Conhost and Terminal store attributes in different bits.
+    // conhostGreenAttrs.SetIndexedAttributes(std::nullopt,
+    //                                        { static_cast<BYTE>(FOREGROUND_GREEN) });
+    conhostGreenAttrs.SetIndexedBackground(FOREGROUND_GREEN);
+    auto terminalGreenAttrs = TextAttribute();
+    // terminalGreenAttrs.SetIndexedAttributes(std::nullopt,
+    //                                         { static_cast<BYTE>(XTERM_GREEN_ATTR) });
+    terminalGreenAttrs.SetIndexedBackground(XTERM_GREEN_ATTR);
+
+    const size_t width = static_cast<size_t>(TerminalViewWidth);
+
+    // Use an initial ^[[m to start printing with default-on-default
+    sm.ProcessString(L"\x1b[m");
+
+    // Print three lines with "# #", where the first "# " are in
+    // default-on-green.
+    for (int i = 0; i < 3; i++)
+    {
+        sm.ProcessString(L"\x1b[42m");
+        sm.ProcessString(L"# ");
+        sm.ProcessString(L"\x1b[m");
+        sm.ProcessString(L"#");
+        sm.ProcessString(L"\r\n");
+    }
+
+    // Now, leave the active attributes as default-on-green. When we resize the
+    // buffers, we don't want them initialized with default-on-green, we want
+    // them to use whatever the set default attributes are.
+    sm.ProcessString(L"\x1b[42m");
+
+    // If leaveTrailingChar is true, we'll leave one default-on-green '#' on row
+    // 3. This will force conpty to change the Terminal's colors to
+    // default-on-green, so we can check that not only conhost initialize the
+    // buffer colors correctly, but so does the Terminal.
+    if (leaveTrailingChar)
+    {
+        sm.ProcessString(L"#");
+    }
+
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rectangle viewport, const bool isTerminal, const bool afterResize) {
+        const auto width = viewport.width<short>();
+
+        // Conhost and Terminal store attributes in different bits.
+        const auto greenAttrs = isTerminal ? terminalGreenAttrs : conhostGreenAttrs;
+
+        for (short row = 0; row < tb.GetSize().Height(); row++)
+        {
+            Log::Comment(NoThrowString().Format(L"Checking row %d...", row));
+
+            VERIFY_IS_FALSE(tb.GetRowByOffset(row).WasWrapForced());
+
+            const bool hasChar = row < 3;
+            const auto actualDefaultAttrs = isTerminal ? TextAttribute() : defaultAttrs;
+
+            if (hasChar)
+            {
+                auto iter = TestUtils::VerifyLineContains(tb, { 0, row }, L'#', greenAttrs, 1u);
+                TestUtils::VerifyLineContains(iter, L' ', greenAttrs, 1u);
+                TestUtils::VerifyLineContains(iter, L'#', TextAttribute(), 1u);
+                // After the resize, the default attrs of the last char will
+                // extend to fill the rest of the row. This is GH#32. If that
+                // bug ever gets fixed, this test will break, but that's
+                // ABSOLUTELY OKAY.
+                TestUtils::VerifyLineContains(iter, L' ', (afterResize ? TextAttribute() : actualDefaultAttrs), static_cast<size_t>(width - 3));
+            }
+            else if (leaveTrailingChar && row == 3)
+            {
+                auto iter = TestUtils::VerifyLineContains(tb, { 0, row }, L'#', greenAttrs, 1u);
+                TestUtils::VerifyLineContains(iter, L' ', (afterResize ? greenAttrs : actualDefaultAttrs), static_cast<size_t>(width - 1));
+            }
+            else
+            {
+                TestUtils::VerifyLineContains(tb, { 0, row }, L' ', actualDefaultAttrs, viewport.width<size_t>());
+            }
+        }
+    };
+
+    Log::Comment(L"========== Checking the host buffer state (before) ==========");
+    verifyBuffer(*hostTb, si.GetViewport().ToInclusive(), false, false);
+
+    Log::Comment(L"Painting the frame");
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    Log::Comment(L"========== Checking the terminal buffer state (before) ==========");
+    verifyBuffer(*termTb, term->_mutableViewport.ToInclusive(), true, false);
+
+    // After we resize, make sure to get the new textBuffers
+    std::tie(hostTb, termTb) = _performResize({ TerminalViewWidth + dx,
+                                                TerminalViewHeight + dy });
+
+    Log::Comment(L"Painting the frame");
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    Log::Comment(L"========== Checking the host buffer state (after) ==========");
+    verifyBuffer(*hostTb, si.GetViewport().ToInclusive(), false, true);
+
+    Log::Comment(L"Painting the frame");
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    Log::Comment(L"========== Checking the terminal buffer state (after) ==========");
+    verifyBuffer(*termTb, term->_mutableViewport.ToInclusive(), true, true);
+}
+
 void ConptyRoundtripTests::NewLinesAtBottomWithBackground()
 {
     BEGIN_TEST_METHOD_PROPERTIES()

src/host/screenInfo.cpp

@@ -1412,10 +1412,16 @@ bool SCREEN_INFORMATION::IsMaximizedY() const
 
     // First allocate a new text buffer to take the place of the current one.
     std::unique_ptr<TextBuffer> newTextBuffer;
+
+    // GH#3848 - Stash away the current attributes the old text buffer is using.
+    // We'll initialize the new buffer with the default attributes, but after
+    // the resize, we'll want to make sure that the new buffer's current
+    // attributes (the ones used for printing new text) match the old buffer's.
+    const auto oldPrimaryAttributes = _textBuffer->GetCurrentAttributes();
     try
     {
         newTextBuffer = std::make_unique<TextBuffer>(coordNewScreenSize,
-                                                     GetAttributes(),
+                                                     TextAttribute{},
                                                      0,
                                                      _renderTarget); // temporarily set size to 0 so it won't render.
     }
@@ -1444,6 +1450,8 @@ bool SCREEN_INFORMATION::IsMaximizedY() const
         coordCursorHeightDiff.Y = sCursorHeightInViewportAfter - sCursorHeightInViewportBefore;
         LOG_IF_FAILED(SetViewportOrigin(false, coordCursorHeightDiff, true));
 
+        _textBuffer->SetCurrentAttributes(oldPrimaryAttributes);
+
         _textBuffer.swap(newTextBuffer);
     }