diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index 203242c7f8a..54bb9d9123b 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -357,11 +357,6 @@ void TextAttribute::SetReverseVideo(bool isReversed) noexcept WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO, isReversed); } -ExtendedAttributes TextAttribute::GetExtendedAttributes() const noexcept -{ - return _extendedAttrs; -} - // Routine Description: // - swaps foreground and background color void TextAttribute::Invert() noexcept diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index b85417f2def..f43d5846169 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -109,7 +109,10 @@ class TextAttribute final void SetOverlined(bool isOverlined) noexcept; void SetReverseVideo(bool isReversed) noexcept; - ExtendedAttributes GetExtendedAttributes() const noexcept; + constexpr ExtendedAttributes GetExtendedAttributes() const noexcept + { + return _extendedAttrs; + } bool IsHyperlink() const noexcept; @@ -161,6 +164,13 @@ class TextAttribute final { return WI_IsAnyFlagSet(_wAttrLegacy, COMMON_LVB_GRID_HORIZONTAL | COMMON_LVB_GRID_LVERTICAL | COMMON_LVB_GRID_RVERTICAL | COMMON_LVB_UNDERSCORE); } + constexpr bool HasAnyExtendedAttributes() const noexcept + { + return GetExtendedAttributes() != ExtendedAttributes::Normal || + IsAnyGridLineEnabled() || + GetHyperlinkId() != 0 || + IsReverseVideo(); + } private: static std::array s_legacyForegroundColorMap; diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index a497a826597..07f246b68ff 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -229,6 +229,9 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final TEST_METHOD(AltBufferResizeCrash); + TEST_METHOD(TestNoExtendedAttrsOptimization); + TEST_METHOD(TestNoBackgroundAttrsOptimization); + private: bool _writeCallback(const char* const pch, const size_t cch); void _flushFirstFrame(); @@ -4114,6 +4117,11 @@ void ConptyRoundtripTests::AltBufferResizeCrash() L"particular combination of resizing could crash the terminal." L" This test makes sure we don't."); + // Anything that resizes the buffer needs IsolationLevel:Method + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method") + END_TEST_METHOD_PROPERTIES() + auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; auto& gci = g.getConsoleInformation(); @@ -4154,3 +4162,99 @@ void ConptyRoundtripTests::AltBufferResizeCrash() Log::Comment(L"Painting the frame"); VERIFY_SUCCEEDED(renderer.PaintFrame()); } + +void ConptyRoundtripTests::TestNoExtendedAttrsOptimization() +{ + Log::Comment(L"We don't want conpty to optimize out runs of spaces that DO " + L"have extended attrs, because EL / ECH don't fill space with " + L"those attributes"); + + 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(); + + gci.LockConsole(); // Lock must be taken to manipulate alt/main buffer state. + auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); }); + + _flushFirstFrame(); + + _checkConptyOutput = false; + + TextAttribute reverseAttrs{}; + reverseAttrs.SetReverseVideo(true); + + auto verifyBuffer = [&](const TextBuffer& tb) { + auto iter0 = TestUtils::VerifyLineContains(tb, { 0, 0 }, L' ', reverseAttrs, 9u); + TestUtils::VerifyExpectedString(L"test", iter0); + TestUtils::VerifyLineContains(iter0, L' ', reverseAttrs, 9u); + + TestUtils::VerifyLineContains(tb, { 0, 1 }, L' ', reverseAttrs, static_cast(TerminalViewWidth)); + }; + + Log::Comment(L"========== Fill test content =========="); + sm.ProcessString(L"\x1b[7m test \x1b[m\n"); + sm.ProcessString(L"\x1b[7m"); + sm.ProcessString(std::wstring(TerminalViewWidth, L' ')); + sm.ProcessString(L"\x1b[m\n"); + + Log::Comment(L"========== Check host buffer =========="); + verifyBuffer(*hostTb); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Check terminal buffer =========="); + verifyBuffer(*termTb); +} + +void ConptyRoundtripTests::TestNoBackgroundAttrsOptimization() +{ + Log::Comment(L"Same as above, with BG attrs"); + + 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(); + + gci.LockConsole(); // Lock must be taken to manipulate alt/main buffer state. + auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); }); + + _flushFirstFrame(); + + _checkConptyOutput = false; + + TextAttribute bgAttrs{}; + bgAttrs.SetIndexedBackground(TextColor::DARK_WHITE); + + auto verifyBuffer = [&](const TextBuffer& tb) { + auto iter0 = TestUtils::VerifyLineContains(tb, { 0, 0 }, L' ', bgAttrs, 9u); + TestUtils::VerifyExpectedString(L"test", iter0); + TestUtils::VerifyLineContains(iter0, L' ', bgAttrs, 9u); + + TestUtils::VerifyLineContains(tb, { 0, 1 }, L' ', bgAttrs, static_cast(TerminalViewWidth)); + }; + + Log::Comment(L"========== Fill test content =========="); + sm.ProcessString(L"\x1b[47m test \x1b[m\n"); + sm.ProcessString(L"\x1b[47m"); + sm.ProcessString(std::wstring(TerminalViewWidth, L' ')); + sm.ProcessString(L"\x1b[m\n"); + + Log::Comment(L"========== Check host buffer =========="); + verifyBuffer(*hostTb); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Check terminal buffer =========="); + verifyBuffer(*termTb); +} diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index fade9487eb8..42a7a2496d1 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -424,10 +424,15 @@ using namespace Microsoft::Console::Types; // the inbox telnet client doesn't understand the Erase Character sequence, // and it uses xterm-ascii. This ensures that xterm and -256color consumers // get the enhancements, and telnet isn't broken. + // + // GH#13229: ECH and EL don't fill the space with "meta" attributes like + // underline, reverse video, hyperlinks, etc. If these spaces had those + // attrs, then don't try and optimize them out. const auto optimalToUseECH = numSpaces > ERASE_CHARACTER_STRING_LENGTH; const auto useEraseChar = (optimalToUseECH) && (!_newBottomLine) && - (!_clearedAllThisFrame); + (!_clearedAllThisFrame) && + (!_lastTextAttributes.HasAnyExtendedAttributes()); const auto printingBottomLine = coord.Y == _lastViewport.BottomInclusive(); // GH#5502 - If the background color of the "new bottom line" is different