Fix conpty not emitting colored spaces on the VERY FIRST frame (#13665)

This bug arose from a "race condition" in the first frame handling of conpty. We'd try to optimize out spaces if we've cleared the entire frame (which always happens on the first frame). However, doing that even for colored spaces meant that things like powerline prompts could be emitted to conhost during the first frame, and we'd optimize the spaces out. That's silly.

This is hard to repro naturally, but this comment has another repro I used
#8341 (comment)

Modified to facilitate simpler testing, it looks like:

#### Before
![image](https://user-images.githubusercontent.com/18356694/182680119-bb22179c-a328-43f3-b64a-0d1d5773b813.png)

#### After
![image](https://user-images.githubusercontent.com/18356694/182680159-805964c5-c4cc-411a-8865-3866fca8d6e9.png)

* [x] Closes #8341
* [x] Tests added

Co-authored by @DHowett

(cherry picked from commit 210a98e)
Service-Card-Id: 84836597
Service-Version: 1.15

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -227,6 +227,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
     TEST_METHOD(SimpleAltBufferTest);
     TEST_METHOD(AltBufferToAltBufferTest);
 
+    TEST_METHOD(TestPowerLineFirstFrame);
+
     TEST_METHOD(AltBufferResizeCrash);
 
     TEST_METHOD(TestNoExtendedAttrsOptimization);
@@ -4111,6 +4113,77 @@ void ConptyRoundtripTests::AltBufferToAltBufferTest()
     verifyBuffer(*termAltTb, term->_GetMutableViewport().ToExclusive(), Frame::StillInAltBuffer);
 }
 
+void ConptyRoundtripTests::TestPowerLineFirstFrame()
+{
+    Log::Comment(L"This is a test for GH#8341. If we received colored spaces "
+                 L"BEFORE the first frame, we should still emit them!");
+
+    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->_mainBuffer.get();
+
+    _checkConptyOutput = false;
+
+    TextAttribute whiteOnGreen{};
+    whiteOnGreen.SetIndexedForeground(TextColor::DARK_WHITE);
+    whiteOnGreen.SetIndexedBackground(TextColor::BRIGHT_GREEN);
+
+    TextAttribute greenOnBlack{};
+    greenOnBlack.SetIndexedForeground(TextColor::BRIGHT_GREEN);
+    greenOnBlack.SetIndexedBackground(TextColor::BRIGHT_BLACK);
+
+    TextAttribute whiteOnBlack{};
+    whiteOnBlack.SetIndexedForeground(TextColor::DARK_WHITE);
+    whiteOnBlack.SetIndexedBackground(TextColor::BRIGHT_BLACK);
+
+    TextAttribute blackOnDefault{};
+    blackOnDefault.SetIndexedForeground(TextColor::BRIGHT_BLACK);
+
+    TextAttribute defaultOnDefault{};
+
+    Log::Comment(L"========== Fill test content ==========");
+
+    // As a pwsh one-liner:
+    //
+    //  "`e[37m`e[102m foo\bar `e[92m`e[100m▶ `e[37mBar `e[90m`e[49m▶ `e[m"
+    //
+    // Generally taken from
+    // https://github.com/microsoft/terminal/issues/8341#issuecomment-731310022,
+    // but minimized for easier testing.
+
+    sm.ProcessString(L"\x1b[37m\x1b[102m" // dark white on bright green
+                     L" foo\\bar ");
+    sm.ProcessString(L"\x1b[92m\x1b[100m" // bright green on bright black
+                     L"▶ ");
+    sm.ProcessString(L"\x1b[37m" // dark white on bright black
+                     L"Bar ");
+    sm.ProcessString(L"\x1b[90m\x1b[49m" // bright black on default
+                     L"▶ ");
+    sm.ProcessString(L"\x1b[m\n"); // default on default
+
+    auto verifyBuffer = [&](const TextBuffer& tb) {
+        // If this test fails on character 8, then it's because we didn't emit the space, we just moved ahead.
+        auto iter0 = TestUtils::VerifyLineContains(tb, { 0, 0 }, whiteOnGreen, 9u);
+        TestUtils::VerifyLineContains(iter0, OutputCellIterator{ greenOnBlack, 2u });
+        TestUtils::VerifyLineContains(iter0, OutputCellIterator{ whiteOnBlack, 4u });
+        TestUtils::VerifyLineContains(iter0, OutputCellIterator{ blackOnDefault, 2u });
+    };
+
+    Log::Comment(L"========== Check host buffer ==========");
+    verifyBuffer(*hostTb);
+
+    Log::Comment(L"========== Paint first frame ==========");
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    Log::Comment(L"========== Check terminal buffer ==========");
+    verifyBuffer(*termTb);
+}
+
 void ConptyRoundtripTests::AltBufferResizeCrash()
 {
     Log::Comment(L"During the review for GH#12719, it was noticed that this "

src/host/ut_host/ConptyOutputTests.cpp

@@ -121,6 +121,7 @@ class ConptyOutputTests
     TEST_METHOD(WriteAFewSimpleLines);
     TEST_METHOD(InvalidateUntilOneBeforeEnd);
     TEST_METHOD(SetConsoleTitleWithControlChars);
+    TEST_METHOD(IncludeBackgroundColorChangesInFirstFrame);
 
 private:
     bool _writeCallback(const char* const pch, const size_t cch);
@@ -371,6 +372,7 @@ void ConptyOutputTests::SetConsoleTitleWithControlChars()
 {
     BEGIN_TEST_METHOD_PROPERTIES()
         TEST_METHOD_PROPERTY(L"Data:control", L"{0x00, 0x0A, 0x1B, 0x80, 0x9B, 0x9C}")
+        TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method")
     END_TEST_METHOD_PROPERTIES()
 
     int control;
@@ -400,3 +402,29 @@ void ConptyOutputTests::SetConsoleTitleWithControlChars()
 
     VERIFY_SUCCEEDED(renderer.PaintFrame());
 }
+
+void ConptyOutputTests::IncludeBackgroundColorChangesInFirstFrame()
+{
+    auto& g = ServiceLocator::LocateGlobals();
+    auto& renderer = *g.pRender;
+    auto& gci = g.getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& sm = si.GetStateMachine();
+
+    sm.ProcessString(L"\x1b[41mRun 1     \x1b[42mRun 2     \x1b[43mRun 3     \x1b[m");
+
+    expectedOutput.push_back("\x1b[2J"); // standard init sequence for the first frame
+    expectedOutput.push_back("\x1b[m"); // standard init sequence for the first frame
+    expectedOutput.push_back("\x1b[41m");
+    expectedOutput.push_back("\x1b[H"); // standard init sequence for the first frame
+    expectedOutput.push_back("Run 1     ");
+    expectedOutput.push_back("\x1b[42m");
+    expectedOutput.push_back("Run 2     ");
+    expectedOutput.push_back("\x1b[43m");
+    expectedOutput.push_back("Run 3     ");
+
+    // This is also part of the standard init sequence.
+    expectedOutput.push_back("\x1b[?25h");
+
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+}

src/inc/TestUtils.h

@@ -168,15 +168,19 @@ class TerminalCoreUnitTests::TestUtils
             auto actualAttrs = actual->TextAttr();
             auto expectedAttrs = expected->TextAttr();
 
-            auto mismatched = (actualChars != expectedChars || actualAttrs != expectedAttrs);
+            auto mismatched = ((!expectedChars.empty() && actualChars != expectedChars) || actualAttrs != expectedAttrs);
             if (mismatched)
             {
                 WEX::Logging::Log::Comment(WEX::Common::NoThrowString().Format(
                     L"Character or attribute at index %d was mismatched", charsProcessed));
             }
 
-            VERIFY_ARE_EQUAL(expectedChars, actualChars);
+            if (!expectedChars.empty())
+            {
+                VERIFY_ARE_EQUAL(expectedChars, actualChars);
+            }
             VERIFY_ARE_EQUAL(expectedAttrs, actualAttrs);
+
             if (mismatched)
             {
                 return false;

src/renderer/vt/paint.cpp

@@ -453,10 +453,13 @@ using namespace Microsoft::Console::Types;
     // the lines _wrapped_. It doesn't care to manually break the lines, but if
     // we trimmed the spaces off here, we'd print all the "~"s one after another
     // on the same line.
-    const auto removeSpaces = !lineWrapped && (useEraseChar ||
-                                               _clearedAllThisFrame ||
-                                               (_newBottomLine && printingBottomLine && bgMatched));
-    const auto cchActual = removeSpaces ? nonSpaceLength : cchLine;
+    static const TextAttribute defaultAttrs{};
+    const bool removeSpaces = !lineWrapped && (useEraseChar // we determined earlier that ECH is optimal
+                                               || (_clearedAllThisFrame && _lastTextAttributes == defaultAttrs) // OR we cleared the last frame to the default attributes (specifically)
+                                               || (_newBottomLine && printingBottomLine && bgMatched)); // OR we just scrolled a new line onto the bottom of the screen with the correct attributes
+    const size_t cchActual = removeSpaces ?
+                                 (cchLine - numSpaces) :
+                                 cchLine;
 
     const auto columnsActual = removeSpaces ?
                                    (totalWidth - numSpaces) :