diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 07f246b68ff..d5aee1d8b06 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/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 " diff --git a/src/host/ut_host/ConptyOutputTests.cpp b/src/host/ut_host/ConptyOutputTests.cpp index 29e0e28f20f..1ebff63cac2 100644 --- a/src/host/ut_host/ConptyOutputTests.cpp +++ b/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()); +} diff --git a/src/inc/TestUtils.h b/src/inc/TestUtils.h index de8a94f35e8..d6e8f146593 100644 --- a/src/inc/TestUtils.h +++ b/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; diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index 42a7a2496d1..e1555ea18fb 100644 --- a/src/renderer/vt/paint.cpp +++ b/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) :