From dec5c11e198e57c98ace463817a2cc2731d99a15 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 4 Oct 2019 15:53:54 -0500 Subject: [PATCH] =?UTF-8?q?Add=20support=20for=20passing=20through=20exten?= =?UTF-8?q?ded=20text=20attributes,=20like=E2=80=A6=20(#2917)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request Adds support for Italics, Blinking, Invisible, CrossedOut text, THROUGH CONPTY. This does **NOT** add support for those styles to conhost or the terminal. We will store these "Extended Text Attributes" in a `TextAttribute`. When we go to render a line, we'll see if the state has changed from our previous state, and if so, we'll appropriately toggle that state with VT. Boldness has been moved from a `bool` to a single bit in these flags. Technically, now that these are stored in the buffer, we only need to make changes to the renderers to be able to support them. That's not being done as a part of this PR however. ## References See also #2915 and #2916, which are some follow-up tasks from this fix. I thought them too risky for 20H1. ## PR Checklist * [x] Closes #2554 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated
* store text with extended attributes too * Plumb attributes through all the renderers * parse extended attrs, though we're not renderering them right * Render these states correctly * Add a very extensive test * Cleanup for PR * a block of PR feedback * add 512 test cases * Fix the build * Fix @carlos-zamora's suggestions * @miniksa's PR feedback --- src/buffer/out/TextAttribute.cpp | 14 +- src/buffer/out/TextAttribute.hpp | 36 +- .../TerminalCore/TerminalDispatchGraphics.cpp | 4 +- src/host/getset.cpp | 33 ++ src/host/getset.h | 3 + src/host/outputStream.cpp | 26 ++ src/host/outputStream.hpp | 2 + src/host/ut_host/ScreenBufferTests.cpp | 333 ++++++++++++++++++ src/host/ut_host/VtRendererTests.cpp | 142 ++++++-- src/inc/conattrs.hpp | 15 + src/interactivity/onecore/BgfxEngine.cpp | 2 +- src/interactivity/onecore/BgfxEngine.hpp | 2 +- src/renderer/base/renderer.cpp | 4 +- src/renderer/dx/DxRenderer.cpp | 4 +- src/renderer/dx/DxRenderer.hpp | 2 +- src/renderer/gdi/gdirenderer.hpp | 2 +- src/renderer/gdi/state.cpp | 4 +- src/renderer/inc/IRenderEngine.hpp | 2 +- src/renderer/vt/VtSequences.cpp | 88 +++++ src/renderer/vt/WinTelnetEngine.cpp | 9 +- src/renderer/vt/WinTelnetEngine.hpp | 2 +- src/renderer/vt/Xterm256Engine.cpp | 67 +++- src/renderer/vt/Xterm256Engine.hpp | 8 +- src/renderer/vt/XtermEngine.cpp | 11 +- src/renderer/vt/XtermEngine.hpp | 2 +- src/renderer/vt/vtrenderer.hpp | 15 +- src/renderer/wddmcon/WddmConRenderer.cpp | 2 +- src/renderer/wddmcon/WddmConRenderer.hpp | 2 +- src/terminal/adapter/DispatchTypes.hpp | 21 +- src/terminal/adapter/adaptDispatch.cpp | 20 +- src/terminal/adapter/adaptDispatch.hpp | 2 + .../adapter/adaptDispatchGraphics.cpp | 134 ++++++- src/terminal/adapter/conGetSet.hpp | 2 + .../adapter/ut_adapter/adapterTest.cpp | 22 +- src/types/inc/utils.hpp | 13 + 35 files changed, 938 insertions(+), 112 deletions(-) diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index 0c964b848..fb498d251 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -44,7 +44,7 @@ COLORREF TextAttribute::CalculateRgbBackground(std::basic_string_view COLORREF TextAttribute::_GetRgbForeground(std::basic_string_view colorTable, COLORREF defaultColor) const noexcept { - return _foreground.GetColor(colorTable, defaultColor, _isBold); + return _foreground.GetColor(colorTable, defaultColor, IsBold()); } // Routine Description: @@ -155,11 +155,6 @@ void TextAttribute::SetColor(const COLORREF rgbColor, const bool fIsForeground) } } -bool TextAttribute::IsBold() const noexcept -{ - return _isBold; -} - bool TextAttribute::_IsReverseVideo() const noexcept { return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO); @@ -215,6 +210,11 @@ void TextAttribute::Debolden() noexcept _SetBoldness(false); } +void TextAttribute::SetExtendedAttributes(const ExtendedAttributes attrs) noexcept +{ + _extendedAttrs = attrs; +} + // Routine Description: // - swaps foreground and background color void TextAttribute::Invert() noexcept @@ -224,7 +224,7 @@ void TextAttribute::Invert() noexcept void TextAttribute::_SetBoldness(const bool isBold) noexcept { - _isBold = isBold; + WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Bold, isBold); } void TextAttribute::SetDefaultForeground() noexcept diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index eb8cc6111..213c908e8 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -27,6 +27,8 @@ Revision History: #include "WexTestClass.h" #endif +#pragma pack(push, 1) + class TextAttribute final { public: @@ -34,7 +36,7 @@ public: _wAttrLegacy{ 0 }, _foreground{}, _background{}, - _isBold{ false } + _extendedAttrs{ ExtendedAttributes::Normal } { } @@ -42,7 +44,7 @@ public: _wAttrLegacy{ gsl::narrow_cast(wLegacyAttr & META_ATTRS) }, _foreground{ gsl::narrow_cast(wLegacyAttr & FG_ATTRS) }, _background{ gsl::narrow_cast((wLegacyAttr & BG_ATTRS) >> 4) }, - _isBold{ false } + _extendedAttrs{ ExtendedAttributes::Normal } { // If we're given lead/trailing byte information with the legacy color, strip it. WI_ClearAllFlags(_wAttrLegacy, COMMON_LVB_SBCSDBCS); @@ -53,7 +55,7 @@ public: _wAttrLegacy{ 0 }, _foreground{ rgbForeground }, _background{ rgbBackground }, - _isBold{ false } + _extendedAttrs{ ExtendedAttributes::Normal } { } @@ -62,7 +64,7 @@ public: const BYTE fg = (_foreground.GetIndex() & FG_ATTRS); const BYTE bg = (_background.GetIndex() << 4) & BG_ATTRS; const WORD meta = (_wAttrLegacy & META_ATTRS); - return (fg | bg | meta) | (_isBold ? FOREGROUND_INTENSITY : 0); + return (fg | bg | meta) | (IsBold() ? FOREGROUND_INTENSITY : 0); } // Method Description: @@ -85,7 +87,7 @@ public: const BYTE fg = (fgIndex & FG_ATTRS); const BYTE bg = (bgIndex << 4) & BG_ATTRS; const WORD meta = (_wAttrLegacy & META_ATTRS); - return (fg | bg | meta) | (_isBold ? FOREGROUND_INTENSITY : 0); + return (fg | bg | meta) | (IsBold() ? FOREGROUND_INTENSITY : 0); } COLORREF CalculateRgbForeground(std::basic_string_view colorTable, @@ -131,7 +133,18 @@ public: friend constexpr bool operator!=(const WORD& legacyAttr, const TextAttribute& attr) noexcept; bool IsLegacy() const noexcept; - bool IsBold() const noexcept; + + constexpr bool IsBold() const noexcept + { + return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Bold); + } + + constexpr ExtendedAttributes GetExtendedAttributes() const noexcept + { + return _extendedAttrs; + } + + void SetExtendedAttributes(const ExtendedAttributes attrs) noexcept; void SetForeground(const COLORREF rgbForeground) noexcept; void SetBackground(const COLORREF rgbBackground) noexcept; @@ -159,7 +172,7 @@ private: WORD _wAttrLegacy; TextColor _foreground; TextColor _background; - bool _isBold; + ExtendedAttributes _extendedAttrs; #ifdef UNIT_TESTING friend class TextBufferTests; @@ -169,6 +182,13 @@ private: #endif }; +#pragma pack(pop) +// 2 for _wAttrLegacy +// 4 for _foreground +// 4 for _background +// 1 for _extendedAttrs +static_assert(sizeof(TextAttribute) <= 11 * sizeof(BYTE), "We should only need 11B for an entire TextColor. Any more than that is just waste"); + enum class TextAttributeBehavior { Stored, // use contained text attribute @@ -181,7 +201,7 @@ constexpr bool operator==(const TextAttribute& a, const TextAttribute& b) noexce return a._wAttrLegacy == b._wAttrLegacy && a._foreground == b._foreground && a._background == b._background && - a._isBold == b._isBold; + a._extendedAttrs == b._extendedAttrs; } constexpr bool operator!=(const TextAttribute& a, const TextAttribute& b) noexcept diff --git a/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp b/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp index 03dab6c84..b73b9adee 100644 --- a/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp +++ b/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp @@ -103,7 +103,7 @@ bool TerminalDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTy isForeground = false; } - if (typeOpt == DispatchTypes::GraphicsOptions::RGBColor && cOptions >= 5) + if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint && cOptions >= 5) { *pcOptionsConsumed = 5; // ensure that each value fits in a byte @@ -115,7 +115,7 @@ bool TerminalDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTy fSuccess = _terminalApi.SetTextRgbColor(color, isForeground); } - else if (typeOpt == DispatchTypes::GraphicsOptions::Xterm256Index && cOptions >= 3) + else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index && cOptions >= 3) { *pcOptionsConsumed = 3; if (rgOptions[2] <= 255) // ensure that the provided index is on the table diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 77fb4d9c4..6d979633b 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -981,6 +981,39 @@ void DoSrvPrivateBoldText(SCREEN_INFORMATION& screenInfo, const bool bolded) buffer.SetAttributes(attrs); } +// Method Description: +// - Retrieves the active ExtendedAttributes (italic, underline, etc.) of the +// given screen buffer. Text written to this buffer will be written with these +// attributes. +// Arguments: +// - screenInfo: The buffer to get the extended attrs from. +// Return Value: +// - the currently active ExtendedAttributes. +ExtendedAttributes DoSrvPrivateGetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo) +{ + auto& buffer = screenInfo.GetActiveBuffer(); + auto attrs = buffer.GetAttributes(); + return attrs.GetExtendedAttributes(); +} + +// Method Description: +// - Sets the active ExtendedAttributes (italic, underline, etc.) of the given +// screen buffer. Text written to this buffer will be written with these +// attributes. +// Arguments: +// - screenInfo: The buffer to set the extended attrs for. +// - extendedAttrs: The new ExtendedAttributes to use +// Return Value: +// - +void DoSrvPrivateSetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo, + const ExtendedAttributes extendedAttrs) +{ + auto& buffer = screenInfo.GetActiveBuffer(); + auto attrs = buffer.GetAttributes(); + attrs.SetExtendedAttributes(extendedAttrs); + buffer.SetAttributes(attrs); +} + // Routine Description: // - Sets the codepage used for translating text when calling A versions of functions affecting the output buffer. // Arguments: diff --git a/src/host/getset.h b/src/host/getset.h index 129bb8cd1..349c3c6b8 100644 --- a/src/host/getset.h +++ b/src/host/getset.h @@ -60,6 +60,9 @@ void DoSrvPrivateSetConsoleRGBTextAttribute(SCREEN_INFORMATION& screenInfo, void DoSrvPrivateBoldText(SCREEN_INFORMATION& screenInfo, const bool bolded); +ExtendedAttributes DoSrvPrivateGetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo); +void DoSrvPrivateSetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo, const ExtendedAttributes attrs); + [[nodiscard]] NTSTATUS DoSrvPrivateEraseAll(SCREEN_INFORMATION& screenInfo); void DoSrvSetCursorStyle(SCREEN_INFORMATION& screenInfo, diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index 9f1211ce9..f45fc8b1d 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -273,6 +273,32 @@ BOOL ConhostInternalGetSet::PrivateBoldText(const bool bolded) return TRUE; } +// Method Description: +// - Retrieves the currently active ExtendedAttributes. See also +// DoSrvPrivateGetExtendedTextAttributes +// Arguments: +// - pAttrs: Recieves the ExtendedAttributes value. +// Return Value: +// - TRUE if successful (see DoSrvPrivateGetExtendedTextAttributes). FALSE otherwise. +BOOL ConhostInternalGetSet::PrivateGetExtendedTextAttributes(ExtendedAttributes* const pAttrs) +{ + *pAttrs = DoSrvPrivateGetExtendedTextAttributes(_io.GetActiveOutputBuffer()); + return TRUE; +} + +// Method Description: +// - Sets the active ExtendedAttributes of the active screen buffer. Text +// written to this buffer will be written with these attributes. +// Arguments: +// - extendedAttrs: The new ExtendedAttributes to use +// Return Value: +// - TRUE if successful (see DoSrvPrivateSetExtendedTextAttributes). FALSE otherwise. +BOOL ConhostInternalGetSet::PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs) +{ + DoSrvPrivateSetExtendedTextAttributes(_io.GetActiveOutputBuffer(), attrs); + return TRUE; +} + // Routine Description: // - Connects the WriteConsoleInput API call directly into our Driver Message servicing call inside Conhost.exe // Arguments: diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index cc626e054..5cfb7bf8b 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -88,6 +88,8 @@ public: const bool fIsForeground) override; BOOL PrivateBoldText(const bool bolded) override; + BOOL PrivateGetExtendedTextAttributes(ExtendedAttributes* const pAttrs) override; + BOOL PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs) override; BOOL PrivateWriteConsoleInputW(_Inout_ std::deque>& events, _Out_ size_t& eventsWritten) override; diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 7abe51fe8..3281c0854 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -187,6 +187,9 @@ class ScreenBufferTests TEST_METHOD(InitializeTabStopsInVTMode); + TEST_METHOD(TestExtendedTextAttributes); + TEST_METHOD(TestExtendedTextAttributesWithColors); + TEST_METHOD(CursorUpDownAcrossMargins); TEST_METHOD(CursorUpDownOutsideMargins); TEST_METHOD(CursorUpDownExactlyAtMargins); @@ -4546,6 +4549,336 @@ void ScreenBufferTests::InitializeTabStopsInVTMode() VERIFY_IS_TRUE(gci.GetActiveOutputBuffer().AreTabsSet()); } +void ScreenBufferTests::TestExtendedTextAttributes() +{ + // This is a test for microsoft/terminal#2554. Refer to that issue for more + // context. + + // We're going to set every possible combination of extended attributes via + // VT, then disable them, and make sure that they are all always represented + // internally correctly. + + // Run this test for each and every possible combination of states. + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:bold", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:italics", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:blink", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:invisible", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:crossedOut", L"{false, true}") + END_TEST_METHOD_PROPERTIES() + + bool bold, italics, blink, invisible, crossedOut; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"bold", bold)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"italics", italics)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"blink", blink)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"invisible", invisible)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"crossedOut", crossedOut)); + + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = tbi.GetCursor(); + + ExtendedAttributes expectedAttrs{ ExtendedAttributes::Normal }; + std::wstring vtSeq = L""; + + // Collect up a VT sequence to set the state given the method properties + if (bold) + { + WI_SetFlag(expectedAttrs, ExtendedAttributes::Bold); + vtSeq += L"\x1b[1m"; + } + if (italics) + { + WI_SetFlag(expectedAttrs, ExtendedAttributes::Italics); + vtSeq += L"\x1b[3m"; + } + if (blink) + { + WI_SetFlag(expectedAttrs, ExtendedAttributes::Blinking); + vtSeq += L"\x1b[5m"; + } + if (invisible) + { + WI_SetFlag(expectedAttrs, ExtendedAttributes::Invisible); + vtSeq += L"\x1b[8m"; + } + if (crossedOut) + { + WI_SetFlag(expectedAttrs, ExtendedAttributes::CrossedOut); + vtSeq += L"\x1b[9m"; + } + + // Helper lambda to write a VT sequence, then an "X", then check that the + // attributes of the "X" match what we think they should be. + auto validate = [&](const ExtendedAttributes expectedAttrs, + const std::wstring& vtSequence) { + auto cursorPos = cursor.GetPosition(); + + // Convert the vtSequence to something printable. Lets not set these + // attrs on the test console + std::wstring debugString = vtSequence; + { + size_t start_pos = 0; + while ((start_pos = debugString.find(L"\x1b", start_pos)) != std::string::npos) + { + debugString.replace(start_pos, 1, L"\\x1b"); + start_pos += 4; + } + } + + Log::Comment(NoThrowString().Format( + L"Testing string:\"%s\"", debugString.c_str())); + Log::Comment(NoThrowString().Format( + L"Expecting attrs:0x%02x", expectedAttrs)); + + stateMachine.ProcessString(vtSequence); + stateMachine.ProcessString(L"X"); + + auto iter = tbi.GetCellDataAt(cursorPos); + auto currentExtendedAttrs = iter->TextAttr().GetExtendedAttributes(); + VERIFY_ARE_EQUAL(expectedAttrs, currentExtendedAttrs); + }; + + // Check setting all the states collected above + validate(expectedAttrs, vtSeq); + + // One-by-one, turn off each of these states with VT, then check that the + // state matched. + if (bold) + { + WI_ClearFlag(expectedAttrs, ExtendedAttributes::Bold); + vtSeq = L"\x1b[22m"; + validate(expectedAttrs, vtSeq); + } + if (italics) + { + WI_ClearFlag(expectedAttrs, ExtendedAttributes::Italics); + vtSeq = L"\x1b[23m"; + validate(expectedAttrs, vtSeq); + } + if (blink) + { + WI_ClearFlag(expectedAttrs, ExtendedAttributes::Blinking); + vtSeq = L"\x1b[25m"; + validate(expectedAttrs, vtSeq); + } + if (invisible) + { + WI_ClearFlag(expectedAttrs, ExtendedAttributes::Invisible); + vtSeq = L"\x1b[28m"; + validate(expectedAttrs, vtSeq); + } + if (crossedOut) + { + WI_ClearFlag(expectedAttrs, ExtendedAttributes::CrossedOut); + vtSeq = L"\x1b[29m"; + validate(expectedAttrs, vtSeq); + } + + stateMachine.ProcessString(L"\x1b[0m"); +} + +void ScreenBufferTests::TestExtendedTextAttributesWithColors() +{ + // This is a test for microsoft/terminal#2554. Refer to that issue for more + // context. + + // We're going to set every possible combination of extended attributes via + // VT, then set assorted colors, then disable extended attrs, then reset + // colors, in various ways, and make sure that they are all always + // represented internally correctly. + + // Run this test for each and every possible combination of states. + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:bold", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:italics", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:blink", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:invisible", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:crossedOut", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:setForegroundType", L"{0, 1, 2, 3}") + TEST_METHOD_PROPERTY(L"Data:setBackgroundType", L"{0, 1, 2, 3}") + END_TEST_METHOD_PROPERTIES() + + // colorStyle will be used to control whether we use a color from the 16 + // color table, a color from the 256 color table, or a pure RGB color. + const int UseDefault = 0; + const int Use16Color = 1; + const int Use256Color = 2; + const int UseRGBColor = 3; + + bool bold, italics, blink, invisible, crossedOut; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"bold", bold)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"italics", italics)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"blink", blink)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"invisible", invisible)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"crossedOut", crossedOut)); + + int setForegroundType, setBackgroundType; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"setForegroundType", setForegroundType), L"controls whether to use the 16 color table, 256 table, or RGB colors"); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"setBackgroundType", setBackgroundType), L"controls whether to use the 16 color table, 256 table, or RGB colors"); + + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = tbi.GetCursor(); + + TextAttribute expectedAttr{ si.GetAttributes() }; + ExtendedAttributes expectedExtendedAttrs{ ExtendedAttributes::Normal }; + std::wstring vtSeq = L""; + + // Collect up a VT sequence to set the state given the method properties + if (bold) + { + WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Bold); + vtSeq += L"\x1b[1m"; + } + if (italics) + { + WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Italics); + vtSeq += L"\x1b[3m"; + } + if (blink) + { + WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Blinking); + vtSeq += L"\x1b[5m"; + } + if (invisible) + { + WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Invisible); + vtSeq += L"\x1b[8m"; + } + if (crossedOut) + { + WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::CrossedOut); + vtSeq += L"\x1b[9m"; + } + + // Prepare the foreground attributes + if (setForegroundType == UseDefault) + { + expectedAttr.SetDefaultForeground(); + vtSeq += L"\x1b[39m"; + } + else if (setForegroundType == Use16Color) + { + expectedAttr.SetIndexedAttributes({ static_cast(2) }, std::nullopt); + vtSeq += L"\x1b[32m"; + } + else if (setForegroundType == Use256Color) + { + expectedAttr.SetForeground(gci.GetColorTableEntry(20)); + vtSeq += L"\x1b[38;5;20m"; + } + else if (setForegroundType == UseRGBColor) + { + expectedAttr.SetForeground(RGB(1, 2, 3)); + vtSeq += L"\x1b[38;2;1;2;3m"; + } + + // Prepare the background attributes + if (setBackgroundType == UseDefault) + { + expectedAttr.SetDefaultBackground(); + vtSeq += L"\x1b[49m"; + } + else if (setBackgroundType == Use16Color) + { + expectedAttr.SetIndexedAttributes(std::nullopt, { static_cast(2) }); + vtSeq += L"\x1b[42m"; + } + else if (setBackgroundType == Use256Color) + { + expectedAttr.SetBackground(gci.GetColorTableEntry(20)); + vtSeq += L"\x1b[48;5;20m"; + } + else if (setBackgroundType == UseRGBColor) + { + expectedAttr.SetBackground(RGB(1, 2, 3)); + vtSeq += L"\x1b[48;2;1;2;3m"; + } + + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + + // Helper lambda to write a VT sequence, then an "X", then check that the + // attributes of the "X" match what we think they should be. + auto validate = [&](const TextAttribute attr, + const std::wstring& vtSequence) { + auto cursorPos = cursor.GetPosition(); + + // Convert the vtSequence to something printable. Lets not set these + // attrs on the test console + std::wstring debugString = vtSequence; + { + size_t start_pos = 0; + while ((start_pos = debugString.find(L"\x1b", start_pos)) != std::string::npos) + { + debugString.replace(start_pos, 1, L"\\x1b"); + start_pos += 4; + } + } + + Log::Comment(NoThrowString().Format( + L"Testing string:\"%s\"", debugString.c_str())); + Log::Comment(NoThrowString().Format( + L"Expecting attrs:0x%02x", VerifyOutputTraits::ToString(attr).GetBuffer())); + + stateMachine.ProcessString(vtSequence); + stateMachine.ProcessString(L"X"); + + auto iter = tbi.GetCellDataAt(cursorPos); + const TextAttribute currentAttrs = iter->TextAttr(); + VERIFY_ARE_EQUAL(attr, currentAttrs); + }; + + // Check setting all the states collected above + validate(expectedAttr, vtSeq); + + // One-by-one, turn off each of these states with VT, then check that the + // state matched. + if (bold) + { + WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Bold); + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + vtSeq = L"\x1b[22m"; + validate(expectedAttr, vtSeq); + } + if (italics) + { + WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Italics); + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + vtSeq = L"\x1b[23m"; + validate(expectedAttr, vtSeq); + } + if (blink) + { + WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Blinking); + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + vtSeq = L"\x1b[25m"; + validate(expectedAttr, vtSeq); + } + if (invisible) + { + WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Invisible); + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + vtSeq = L"\x1b[28m"; + validate(expectedAttr, vtSeq); + } + if (crossedOut) + { + WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::CrossedOut); + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + vtSeq = L"\x1b[29m"; + validate(expectedAttr, vtSeq); + } + + stateMachine.ProcessString(L"\x1b[0m"); +} + void ScreenBufferTests::CursorUpDownAcrossMargins() { // Test inspired by: https://github.com/microsoft/terminal/issues/2929 diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index 0fc75e6b3..437338cf5 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -390,25 +390,41 @@ void VtRendererTest::Xterm256TestColors() L"These values were picked for ease of formatting raw COLORREF values.")); qExpectedInput.push_back("\x1b[38;2;1;2;3m"); qExpectedInput.push_back("\x1b[48;2;5;6;7m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x00030201, 0x00070605, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x00030201, + 0x00070605, + 0, + ExtendedAttributes::Normal, + false)); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"----Change only the BG----")); qExpectedInput.push_back("\x1b[48;2;7;8;9m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x00030201, 0x00090807, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x00030201, + 0x00090807, + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the FG----")); qExpectedInput.push_back("\x1b[38;2;10;11;12m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x000c0b0a, 0x00090807, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x000c0b0a, + 0x00090807, + 0, + ExtendedAttributes::Normal, + false)); }); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"Make sure that color setting persists across EndPaint/StartPaint")); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x000c0b0a, 0x00090807, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x000c0b0a, + 0x00090807, + 0, + ExtendedAttributes::Normal, + false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); @@ -420,41 +436,69 @@ void VtRendererTest::Xterm256TestColors() L"Begin by setting the default colors - FG,BG = BRIGHT_WHITE,DARK_BLACK")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"----Change only the BG----")); qExpectedInput.push_back("\x1b[41m"); // Background DARK_RED - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the FG----")); qExpectedInput.push_back("\x1b[37m"); // Foreground DARK_WHITE - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to something not in the table----")); qExpectedInput.push_back("\x1b[48;2;1;1;1m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], 0x010101, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + 0x010101, + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to the 'Default' background----")); qExpectedInput.push_back("\x1b[49m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Back to defaults----")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); }); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"Make sure that color setting persists across EndPaint/StartPaint")); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); } @@ -725,41 +769,65 @@ void VtRendererTest::XtermTestColors() L"Begin by setting the default colors - FG,BG = BRIGHT_WHITE,DARK_BLACK")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"----Change only the BG----")); qExpectedInput.push_back("\x1b[41m"); // Background DARK_RED - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the FG----")); qExpectedInput.push_back("\x1b[37m"); // Foreground DARK_WHITE - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to something not in the table----")); qExpectedInput.push_back("\x1b[40m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], 0x010101, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], 0x010101, 0, ExtendedAttributes::Normal, false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to the 'Default' background----")); qExpectedInput.push_back("\x1b[40m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Back to defaults----")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); }); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"Make sure that color setting persists across EndPaint/StartPaint")); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); } @@ -961,40 +1029,64 @@ void VtRendererTest::WinTelnetTestColors() L"Begin by setting the default colors - FG,BG = BRIGHT_WHITE,DARK_BLACK")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"----Change only the BG----")); qExpectedInput.push_back("\x1b[41m"); // Background DARK_RED - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the FG----")); qExpectedInput.push_back("\x1b[37m"); // Foreground DARK_WHITE - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to something not in the table----")); qExpectedInput.push_back("\x1b[40m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], 0x010101, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], 0x010101, 0, ExtendedAttributes::Normal, false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to the 'Default' background----")); qExpectedInput.push_back("\x1b[40m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Back to defaults----")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); }); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"Make sure that color setting persists across EndPaint/StartPaint")); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); } diff --git a/src/inc/conattrs.hpp b/src/inc/conattrs.hpp index 5e5325190..a168b173c 100644 --- a/src/inc/conattrs.hpp +++ b/src/inc/conattrs.hpp @@ -8,6 +8,21 @@ Licensed under the MIT license. #define BG_ATTRS (BACKGROUND_BLUE | BACKGROUND_GREEN | BACKGROUND_RED | BACKGROUND_INTENSITY) #define META_ATTRS (COMMON_LVB_LEADING_BYTE | COMMON_LVB_TRAILING_BYTE | COMMON_LVB_GRID_HORIZONTAL | COMMON_LVB_GRID_LVERTICAL | COMMON_LVB_GRID_RVERTICAL | COMMON_LVB_REVERSE_VIDEO | COMMON_LVB_UNDERSCORE) +enum class ExtendedAttributes : BYTE +{ + Normal = 0x00, + Bold = 0x01, + Italics = 0x02, + Blinking = 0x04, + Invisible = 0x08, + CrossedOut = 0x10, + // TODO:GH#2916 add support for these to the parser as well. + Underlined = 0x20, // _technically_ different from LVB_UNDERSCORE, see TODO:GH#2915 + DoublyUnderlined = 0x40, // Included for completeness, but not currently supported. + Faint = 0x80, // Included for completeness, but not currently supported. +}; +DEFINE_ENUM_FLAG_OPERATORS(ExtendedAttributes); + WORD FindNearestTableIndex(const COLORREF Color, _In_reads_(cColorTable) const COLORREF* const ColorTable, const WORD cColorTable); diff --git a/src/interactivity/onecore/BgfxEngine.cpp b/src/interactivity/onecore/BgfxEngine.cpp index 02eb42c57..b73c769c7 100644 --- a/src/interactivity/onecore/BgfxEngine.cpp +++ b/src/interactivity/onecore/BgfxEngine.cpp @@ -196,7 +196,7 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid [[nodiscard]] HRESULT BgfxEngine::UpdateDrawingBrushes(COLORREF const /*colorForeground*/, COLORREF const /*colorBackground*/, const WORD legacyColorAttribute, - const bool /*isBold*/, + const ExtendedAttributes /*extendedAttrs*/, bool const /*isSettingDefaultBrushes*/) noexcept { _currentLegacyColorAttribute = legacyColorAttribute; diff --git a/src/interactivity/onecore/BgfxEngine.hpp b/src/interactivity/onecore/BgfxEngine.hpp index 19a255503..9ce46e357 100644 --- a/src/interactivity/onecore/BgfxEngine.hpp +++ b/src/interactivity/onecore/BgfxEngine.hpp @@ -60,7 +60,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(COLORREF const colorForeground, COLORREF const colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, bool const isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo) noexcept override; [[nodiscard]] HRESULT UpdateDpi(int const iDpi) noexcept override; diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index e434c5d07..64e12c32e 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -872,11 +872,11 @@ void Renderer::_PaintSelection(_In_ IRenderEngine* const pEngine) const COLORREF rgbForeground = _pData->GetForegroundColor(textAttributes); const COLORREF rgbBackground = _pData->GetBackgroundColor(textAttributes); const WORD legacyAttributes = textAttributes.GetLegacyAttributes(); - const bool isBold = textAttributes.IsBold(); + const auto extendedAttrs = textAttributes.GetExtendedAttributes(); // The last color needs to be each engine's responsibility. If it's local to this function, // then on the next engine we might not update the color. - RETURN_IF_FAILED(pEngine->UpdateDrawingBrushes(rgbForeground, rgbBackground, legacyAttributes, isBold, isSettingDefaultBrushes)); + RETURN_IF_FAILED(pEngine->UpdateDrawingBrushes(rgbForeground, rgbBackground, legacyAttributes, extendedAttrs, isSettingDefaultBrushes)); return S_OK; } diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index b27ed99b9..505ab354c 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1218,14 +1218,14 @@ enum class CursorPaintType // - colorForeground - Foreground brush color // - colorBackground - Background brush color // - legacyColorAttribute - -// - isBold - +// - extendedAttrs - // - isSettingDefaultBrushes - Lets us know that these are the default brushes to paint the swapchain background or selection // Return Value: // - S_OK or relevant DirectX error. [[nodiscard]] HRESULT DxEngine::UpdateDrawingBrushes(COLORREF const colorForeground, COLORREF const colorBackground, const WORD /*legacyColorAttribute*/, - const bool /*isBold*/, + const ExtendedAttributes /*extendedAttrs*/, bool const isSettingDefaultBrushes) noexcept { _foregroundColor = _ColorFFromColorRef(colorForeground); diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index ebe6846cf..4960d1f72 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -80,7 +80,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(COLORREF const colorForeground, COLORREF const colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, bool const isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo) noexcept override; [[nodiscard]] HRESULT UpdateDpi(int const iDpi) noexcept override; diff --git a/src/renderer/gdi/gdirenderer.hpp b/src/renderer/gdi/gdirenderer.hpp index 079c099dd..4840a1819 100644 --- a/src/renderer/gdi/gdirenderer.hpp +++ b/src/renderer/gdi/gdirenderer.hpp @@ -56,7 +56,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& FontInfoDesired, _Out_ FontInfo& FontInfo) noexcept override; diff --git a/src/renderer/gdi/state.cpp b/src/renderer/gdi/state.cpp index b55dc12f0..8ab7d3b8e 100644 --- a/src/renderer/gdi/state.cpp +++ b/src/renderer/gdi/state.cpp @@ -176,7 +176,7 @@ GdiEngine::~GdiEngine() // - colorForeground - Foreground Color // - colorBackground - Background colo // - legacyColorAttribute - -// - isBold - +// - extendedAttrs - // - isSettingDefaultBrushes - Lets us know that the default brushes are being set so we can update the DC background // and the hung app background painting color // Return Value: @@ -184,7 +184,7 @@ GdiEngine::~GdiEngine() [[nodiscard]] HRESULT GdiEngine::UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD /*legacyColorAttribute*/, - const bool /*isBold*/, + const ExtendedAttributes /*extendedAttrs*/, const bool isSettingDefaultBrushes) noexcept { RETURN_IF_FAILED(_FlushBufferLines()); diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp index 2d3213813..aff56a5bb 100644 --- a/src/renderer/inc/IRenderEngine.hpp +++ b/src/renderer/inc/IRenderEngine.hpp @@ -105,7 +105,7 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept = 0; [[nodiscard]] virtual HRESULT UpdateFont(const FontInfoDesired& FontInfoDesired, _Out_ FontInfo& FontInfo) noexcept = 0; diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index 2990331a2..a629c697b 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -345,3 +345,91 @@ using namespace Microsoft::Console::Render; { return _Write("\x1b[24m"); } + +// Method Description: +// - Writes a sequence to tell the terminal to start italicizing text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_BeginItalics() noexcept +{ + return _Write("\x1b[3m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to stop italicizing text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_EndItalics() noexcept +{ + return _Write("\x1b[23m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to start blinking text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_BeginBlink() noexcept +{ + return _Write("\x1b[5m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to stop blinking text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_EndBlink() noexcept +{ + return _Write("\x1b[25m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to start marking text as invisible +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_BeginInvisible() noexcept +{ + return _Write("\x1b[8m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to stop marking text as invisible +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_EndInvisible() noexcept +{ + return _Write("\x1b[28m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to start crossing-out text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_BeginCrossedOut() noexcept +{ + return _Write("\x1b[9m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to stop crossing-out text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_EndCrossedOut() noexcept +{ + return _Write("\x1b[29m"); +} diff --git a/src/renderer/vt/WinTelnetEngine.cpp b/src/renderer/vt/WinTelnetEngine.cpp index 614c66a3b..c273232fd 100644 --- a/src/renderer/vt/WinTelnetEngine.cpp +++ b/src/renderer/vt/WinTelnetEngine.cpp @@ -29,6 +29,7 @@ WinTelnetEngine::WinTelnetEngine(_In_ wil::unique_hfile hPipe, // - colorBackground: The RGB Color to use to paint the background of the text. // - legacyColorAttribute: A console attributes bit field specifying the brush // colors we should use. +// - extendedAttrs - extended text attributes (italic, underline, etc.) to use. // - isSettingDefaultBrushes: indicates if we should change the background color of // the window. Unused for VT // Return Value: @@ -36,10 +37,14 @@ WinTelnetEngine::WinTelnetEngine(_In_ wil::unique_hfile hPipe, [[nodiscard]] HRESULT WinTelnetEngine::UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD /*legacyColorAttribute*/, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool /*isSettingDefaultBrushes*/) noexcept { - return VtEngine::_16ColorUpdateDrawingBrushes(colorForeground, colorBackground, isBold, _ColorTable, _cColorTable); + return VtEngine::_16ColorUpdateDrawingBrushes(colorForeground, + colorBackground, + WI_IsFlagSet(extendedAttrs, ExtendedAttributes::Bold), + _ColorTable, + _cColorTable); } // Routine Description: diff --git a/src/renderer/vt/WinTelnetEngine.hpp b/src/renderer/vt/WinTelnetEngine.hpp index d025d985c..8c27c2218 100644 --- a/src/renderer/vt/WinTelnetEngine.hpp +++ b/src/renderer/vt/WinTelnetEngine.hpp @@ -34,7 +34,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT ScrollFrame() noexcept override; diff --git a/src/renderer/vt/Xterm256Engine.cpp b/src/renderer/vt/Xterm256Engine.cpp index 7a2781d5f..af8738a56 100644 --- a/src/renderer/vt/Xterm256Engine.cpp +++ b/src/renderer/vt/Xterm256Engine.cpp @@ -14,7 +14,8 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe, const Viewport initialViewport, _In_reads_(cColorTable) const COLORREF* const ColorTable, const WORD cColorTable) : - XtermEngine(std::move(hPipe), shutdownEvent, colorProvider, initialViewport, ColorTable, cColorTable, false) + XtermEngine(std::move(hPipe), shutdownEvent, colorProvider, initialViewport, ColorTable, cColorTable, false), + _lastExtendedAttrsState{ ExtendedAttributes::Normal } { } @@ -26,6 +27,7 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe, // - colorBackground: The RGB Color to use to paint the background of the text. // - legacyColorAttribute: A console attributes bit field specifying the brush // colors we should use. +// - extendedAttrs - extended text attributes (italic, underline, etc.) to use. // - isSettingDefaultBrushes: indicates if we should change the background color of // the window. Unused for VT // Return Value: @@ -33,7 +35,7 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe, [[nodiscard]] HRESULT Xterm256Engine::UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool /*isSettingDefaultBrushes*/) noexcept { //When we update the brushes, check the wAttrs to see if the LVB_UNDERSCORE @@ -42,11 +44,70 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe, // We have to do this here, instead of in PaintBufferGridLines, because // we'll have already painted the text by the time PaintBufferGridLines // is called. + // TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE RETURN_IF_FAILED(_UpdateUnderline(legacyColorAttribute)); + // Only do extended attributes in xterm-256color, as to not break telnet.exe. + RETURN_IF_FAILED(_UpdateExtendedAttrs(extendedAttrs)); + return VtEngine::_RgbUpdateDrawingBrushes(colorForeground, colorBackground, - isBold, + WI_IsFlagSet(extendedAttrs, ExtendedAttributes::Bold), _ColorTable, _cColorTable); } + +// Routine Description: +// - Write a VT sequence to either start or stop underlining text. +// Arguments: +// - legacyColorAttribute: A console attributes bit field containing information +// about the underlining state of the text. +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT Xterm256Engine::_UpdateExtendedAttrs(const ExtendedAttributes extendedAttrs) noexcept +{ + // Helper lambda to check if a state (attr) has changed since it's last + // value (lastState), and appropriately start/end that state with the given + // begin/end functions. + auto updateFlagAndState = [extendedAttrs, this](const ExtendedAttributes attr, + std::function beginFn, + std::function endFn) -> HRESULT { + const bool flagSet = WI_AreAllFlagsSet(extendedAttrs, attr); + const bool lastState = WI_AreAllFlagsSet(_lastExtendedAttrsState, attr); + if (flagSet != lastState) + { + if (flagSet) + { + RETURN_IF_FAILED(beginFn(this)); + } + else + { + RETURN_IF_FAILED(endFn(this)); + } + WI_SetAllFlags(_lastExtendedAttrsState, attr); + } + return S_OK; + }; + + auto hr = updateFlagAndState(ExtendedAttributes::Italics, + &Xterm256Engine::_BeginItalics, + &Xterm256Engine::_EndItalics); + RETURN_IF_FAILED(hr); + + hr = updateFlagAndState(ExtendedAttributes::Blinking, + &Xterm256Engine::_BeginBlink, + &Xterm256Engine::_EndBlink); + RETURN_IF_FAILED(hr); + + hr = updateFlagAndState(ExtendedAttributes::Invisible, + &Xterm256Engine::_BeginInvisible, + &Xterm256Engine::_EndInvisible); + RETURN_IF_FAILED(hr); + + hr = updateFlagAndState(ExtendedAttributes::CrossedOut, + &Xterm256Engine::_BeginCrossedOut, + &Xterm256Engine::_EndCrossedOut); + RETURN_IF_FAILED(hr); + + return S_OK; +} diff --git a/src/renderer/vt/Xterm256Engine.hpp b/src/renderer/vt/Xterm256Engine.hpp index 7ad288b34..729ed1243 100644 --- a/src/renderer/vt/Xterm256Engine.hpp +++ b/src/renderer/vt/Xterm256Engine.hpp @@ -35,10 +35,16 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept override; private: + [[nodiscard]] HRESULT _UpdateExtendedAttrs(const ExtendedAttributes extendedAttrs) noexcept; + + // We're only using Italics, Blinking, Invisible and Crossed Out for now + // See GH#2916 for adding a more complete implementation. + ExtendedAttributes _lastExtendedAttrsState; + #ifdef UNIT_TESTING friend class VtRendererTest; #endif diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 2e56da386..b48dab303 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -172,6 +172,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // - colorBackground: The RGB Color to use to paint the background of the text. // - legacyColorAttribute: A console attributes bit field specifying the brush // colors we should use. +// - extendedAttrs - extended text attributes (italic, underline, etc.) to use. // - isSettingDefaultBrushes: indicates if we should change the background color of // the window. Unused for VT // Return Value: @@ -179,7 +180,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, [[nodiscard]] HRESULT XtermEngine::UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool /*isSettingDefaultBrushes*/) noexcept { //When we update the brushes, check the wAttrs to see if the LVB_UNDERSCORE @@ -188,10 +189,14 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // We have to do this here, instead of in PaintBufferGridLines, because // we'll have already painted the text by the time PaintBufferGridLines // is called. - + // TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE RETURN_IF_FAILED(_UpdateUnderline(legacyColorAttribute)); // The base xterm mode only knows about 16 colors - return VtEngine::_16ColorUpdateDrawingBrushes(colorForeground, colorBackground, isBold, _ColorTable, _cColorTable); + return VtEngine::_16ColorUpdateDrawingBrushes(colorForeground, + colorBackground, + WI_IsFlagSet(extendedAttrs, ExtendedAttributes::Bold), + _ColorTable, + _cColorTable); } // Routine Description: diff --git a/src/renderer/vt/XtermEngine.hpp b/src/renderer/vt/XtermEngine.hpp index 67992a9ab..2a482501d 100644 --- a/src/renderer/vt/XtermEngine.hpp +++ b/src/renderer/vt/XtermEngine.hpp @@ -45,7 +45,7 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT PaintBufferLine(std::basic_string_view const clusters, const COORD coord, diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 8a419a7e9..d2c7804a9 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -69,7 +69,7 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept = 0; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& pfiFontInfoDesired, _Out_ FontInfo& pfiFontInfo) noexcept override; @@ -172,9 +172,20 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT _ResizeWindow(const short sWidth, const short sHeight) noexcept; [[nodiscard]] HRESULT _BeginUnderline() noexcept; - [[nodiscard]] HRESULT _EndUnderline() noexcept; + [[nodiscard]] HRESULT _BeginItalics() noexcept; + [[nodiscard]] HRESULT _EndItalics() noexcept; + + [[nodiscard]] HRESULT _BeginBlink() noexcept; + [[nodiscard]] HRESULT _EndBlink() noexcept; + + [[nodiscard]] HRESULT _BeginInvisible() noexcept; + [[nodiscard]] HRESULT _EndInvisible() noexcept; + + [[nodiscard]] HRESULT _BeginCrossedOut() noexcept; + [[nodiscard]] HRESULT _EndCrossedOut() noexcept; + [[nodiscard]] HRESULT _RequestCursor() noexcept; [[nodiscard]] virtual HRESULT _MoveCursor(const COORD coord) noexcept = 0; diff --git a/src/renderer/wddmcon/WddmConRenderer.cpp b/src/renderer/wddmcon/WddmConRenderer.cpp index 150d4a1e6..23804ec31 100644 --- a/src/renderer/wddmcon/WddmConRenderer.cpp +++ b/src/renderer/wddmcon/WddmConRenderer.cpp @@ -307,7 +307,7 @@ bool WddmConEngine::IsInitialized() [[nodiscard]] HRESULT WddmConEngine::UpdateDrawingBrushes(COLORREF const /*colorForeground*/, COLORREF const /*colorBackground*/, const WORD legacyColorAttribute, - const bool /*isBold*/, + const ExtendedAttributes /*extendedAttrs*/, bool const /*isSettingDefaultBrushes*/) noexcept { _currentLegacyColorAttribute = legacyColorAttribute; diff --git a/src/renderer/wddmcon/WddmConRenderer.hpp b/src/renderer/wddmcon/WddmConRenderer.hpp index 714e977e3..c214d585a 100644 --- a/src/renderer/wddmcon/WddmConRenderer.hpp +++ b/src/renderer/wddmcon/WddmConRenderer.hpp @@ -52,7 +52,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(COLORREF const colorForeground, COLORREF const colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, bool const isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo) noexcept override; [[nodiscard]] HRESULT UpdateDpi(int const iDpi) noexcept override; diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp index e76349ce0..17f26c66d 100644 --- a/src/terminal/adapter/DispatchTypes.hpp +++ b/src/terminal/adapter/DispatchTypes.hpp @@ -13,21 +13,28 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes Scrollback = 3 }; + // TODO:GH#2916 add support for DoublyUnderlined, Faint(2) to the adapter as well. enum GraphicsOptions : unsigned int { Off = 0, BoldBright = 1, - RGBColor = 2, - // 2 is also Faint, decreased intensity (ISO 6429). + // The 2 and 5 entries here are for BOTH the extended graphics options, + // as well as the Faint/Blink options. + RGBColorOrFaint = 2, // 2 is also Faint, decreased intensity (ISO 6429). + Italics = 3, Underline = 4, - Xterm256Index = 5, - // 5 is also Blink (appears as Bold). - // the 2 and 5 entries here are only for the extended graphics options - // as we do not currently support those features individually + BlinkOrXterm256Index = 5, // 5 is also Blink (appears as Bold). Negative = 7, + Invisible = 8, + CrossedOut = 9, + DoublyUnderlined = 21, UnBold = 22, + NotItalics = 23, NoUnderline = 24, - Positive = 27, + Steady = 25, // _not_ blink + Positive = 27, // _not_ inverse + Visible = 28, // _not_ invisible + NotCrossedOut = 29, ForegroundBlack = 30, ForegroundRed = 31, ForegroundGreen = 32, diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 7123e6950..d539eb8c5 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -6,19 +6,7 @@ #include "adaptDispatch.hpp" #include "conGetSet.hpp" #include "../../types/inc/Viewport.hpp" - -// Inspired from RETURN_IF_WIN32_BOOL_FALSE -// WIL doesn't include a RETURN_IF_FALSE, and RETURN_IF_WIN32_BOOL_FALSE -// will actually return the value of GLE. -#define RETURN_IF_FALSE(b) \ - do \ - { \ - BOOL __boolRet = wil::verify_bool(b); \ - if (!__boolRet) \ - { \ - return b; \ - } \ - } while (0, 0) +#include "../../types/inc/utils.hpp" using namespace Microsoft::Console::Types; using namespace Microsoft::Console::VirtualTerminal; @@ -504,15 +492,15 @@ bool AdaptDispatch::_InsertDeleteHelper(_In_ unsigned int const uiCount, const b { // We'll be doing short math on the distance since all console APIs use shorts. So check that we can successfully convert the uint into a short first. SHORT sDistance; - RETURN_IF_FALSE(SUCCEEDED(UIntToShort(uiCount, &sDistance))); + RETURN_BOOL_IF_FALSE(SUCCEEDED(UIntToShort(uiCount, &sDistance))); // get current cursor, attributes CONSOLE_SCREEN_BUFFER_INFOEX csbiex = { 0 }; csbiex.cbSize = sizeof(CONSOLE_SCREEN_BUFFER_INFOEX); // Make sure to reset the viewport (with MoveToBottom )to where it was // before the user scrolled the console output - RETURN_IF_FALSE(_conApi->MoveToBottom()); - RETURN_IF_FALSE(_conApi->GetConsoleScreenBufferInfoEx(&csbiex)); + RETURN_BOOL_IF_FALSE(_conApi->MoveToBottom()); + RETURN_BOOL_IF_FALSE(_conApi->GetConsoleScreenBufferInfoEx(&csbiex)); const auto cursor = csbiex.dwCursorPosition; // Rectangle to cut out of the existing buffer. This is inclusive. diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index a4fc702c5..255dd2c2e 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -170,10 +170,12 @@ namespace Microsoft::Console::VirtualTerminal bool _SetBoldColorHelper(const DispatchTypes::GraphicsOptions option); bool _SetDefaultColorHelper(const DispatchTypes::GraphicsOptions option); + bool _SetExtendedTextAttributeHelper(const DispatchTypes::GraphicsOptions option); static bool s_IsXtermColorOption(const DispatchTypes::GraphicsOptions opt); static bool s_IsRgbColorOption(const DispatchTypes::GraphicsOptions opt); static bool s_IsBoldColorOption(const DispatchTypes::GraphicsOptions opt) noexcept; static bool s_IsDefaultColorOption(const DispatchTypes::GraphicsOptions opt) noexcept; + static bool s_IsExtendedTextAttribute(const DispatchTypes::GraphicsOptions opt) noexcept; }; } diff --git a/src/terminal/adapter/adaptDispatchGraphics.cpp b/src/terminal/adapter/adaptDispatchGraphics.cpp index f676e08ad..85076d082 100644 --- a/src/terminal/adapter/adaptDispatchGraphics.cpp +++ b/src/terminal/adapter/adaptDispatchGraphics.cpp @@ -5,6 +5,7 @@ #include "adaptDispatch.hpp" #include "conGetSet.hpp" +#include "../../types/inc/utils.hpp" #define ENABLE_INTSAFE_SIGNED_FUNCTIONS #include @@ -36,7 +37,9 @@ void AdaptDispatch::s_DisableAllColors(_Inout_ WORD* const pAttr, const bool fIs // Arguments: // - pAttr - Pointer to font attributes field to adjust // - wApplyThis - Color values to apply to the low or high word of the font attributes field. -// - fIsForeground - TRUE = foreground color. FALSE = background color. Specifies which half of the bit field to reset and then apply wApplyThis upon. +// - fIsForeground - TRUE = foreground color. FALSE = background color. +// Specifies which half of the bit field to reset and then apply wApplyThis +// upon. // Return Value: // - void AdaptDispatch::s_ApplyColors(_Inout_ WORD* const pAttr, const WORD wApplyThis, const bool fIsForeground) @@ -62,7 +65,9 @@ void AdaptDispatch::s_ApplyColors(_Inout_ WORD* const pAttr, const WORD wApplyTh // Routine Description: // - Helper to apply the actual flags to each text attributes field. -// - Placed as a helper so it can be recursive/re-entrant for some of the convenience flag methods that perform similar/multiple operations in one command. +// - Placed as a helper so it can be recursive/re-entrant for some of the +// convenience flag methods that perform similar/multiple operations in one +// command. // Arguments: // - opt - Graphics option sent to us by the parser/requestor. // - pAttr - Pointer to the font attribute field to adjust @@ -83,6 +88,7 @@ void AdaptDispatch::_SetGraphicsOptionHelper(const DispatchTypes::GraphicsOption _fChangedMetaAttrs = true; break; case DispatchTypes::GraphicsOptions::Underline: + // TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE *pAttr |= COMMON_LVB_UNDERSCORE; _fChangedMetaAttrs = true; break; @@ -261,6 +267,30 @@ void AdaptDispatch::_SetGraphicsOptionHelper(const DispatchTypes::GraphicsOption } } +// Routine Description: +// Returns true if the GraphicsOption represents an extended text attribute. +// These include things such as Underlined, Italics, Blinking, etc. +// Return Value: +// - true if the opt is the indicator for an extended text attribute, false otherwise. +bool AdaptDispatch::s_IsExtendedTextAttribute(const DispatchTypes::GraphicsOptions opt) noexcept +{ + // TODO:GH#2916 add support for DoublyUnderlined, Faint(RGBColorOrFaint). + // These two are currently partially implemented as other things: + // * Faint is approximately the opposite of bold does, though it's much + // [more complicated]( + // https://github.com/microsoft/terminal/issues/2916#issuecomment-535860910) + // and less supported/used. + // * Doubly underlined should exist in a trinary state with Underlined + return opt == DispatchTypes::GraphicsOptions::Italics || + opt == DispatchTypes::GraphicsOptions::NotItalics || + opt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index || + opt == DispatchTypes::GraphicsOptions::Steady || + opt == DispatchTypes::GraphicsOptions::Invisible || + opt == DispatchTypes::GraphicsOptions::Visible || + opt == DispatchTypes::GraphicsOptions::CrossedOut || + opt == DispatchTypes::GraphicsOptions::NotCrossedOut; +} + // Routine Description: // Returns true if the GraphicsOption represents an extended color option. // These are followed by up to 4 more values which compose the entire option. @@ -338,7 +368,7 @@ bool AdaptDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTypes *pfIsForeground = false; } - if (typeOpt == DispatchTypes::GraphicsOptions::RGBColor && cOptions >= 5) + if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint && cOptions >= 5) { *pcOptionsConsumed = 5; // ensure that each value fits in a byte @@ -350,7 +380,7 @@ bool AdaptDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTypes fSuccess = !!_conApi->SetConsoleRGBTextAttribute(*prgbColor, *pfIsForeground); } - else if (typeOpt == DispatchTypes::GraphicsOptions::Xterm256Index && cOptions >= 3) + else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index && cOptions >= 3) { *pcOptionsConsumed = 3; if (rgOptions[2] <= 255) // ensure that the provided index is on the table @@ -374,31 +404,92 @@ bool AdaptDispatch::_SetDefaultColorHelper(const DispatchTypes::GraphicsOptions { const bool fg = option == GraphicsOptions::Off || option == GraphicsOptions::ForegroundDefault; const bool bg = option == GraphicsOptions::Off || option == GraphicsOptions::BackgroundDefault; + bool success = _conApi->PrivateSetDefaultAttributes(fg, bg); + if (success && fg && bg) { // If we're resetting both the FG & BG, also reset the meta attributes (underline) // as well as the boldness success = _conApi->PrivateSetLegacyAttributes(0, false, false, true) && - _conApi->PrivateBoldText(false); + _conApi->PrivateBoldText(false) && + _conApi->PrivateSetExtendedTextAttributes(ExtendedAttributes::Normal); } return success; } -// Routine Description: -// - SGR - Modifies the graphical rendering options applied to the next characters written into the buffer. -// - Options include colors, invert, underlines, and other "font style" type options. +// Method Description: +// - Sets the attributes for extended text attributes. Retrieves the current +// extended attrs from the console, modifies them according to the new +// GraphicsOption, and the sets them again. +// - Notably does _not_ handle Bold, Faint, Underline, DoublyUnderlined, or +// NoUnderline. Those should be handled in TODO:GH#2916. // Arguments: -// - rgOptions - An array of options that will be applied from 0 to N, in order, one at a time by setting or removing flags in the font style properties. +// - opt: the graphics option to set +// Return Value: +// - True if handled successfully. False otherwise. +bool AdaptDispatch::_SetExtendedTextAttributeHelper(const DispatchTypes::GraphicsOptions opt) +{ + ExtendedAttributes attrs{ ExtendedAttributes::Normal }; + + RETURN_BOOL_IF_FALSE(_conApi->PrivateGetExtendedTextAttributes(&attrs)); + + switch (opt) + { + case DispatchTypes::GraphicsOptions::Italics: + WI_SetFlag(attrs, ExtendedAttributes::Italics); + break; + case DispatchTypes::GraphicsOptions::NotItalics: + WI_ClearFlag(attrs, ExtendedAttributes::Italics); + break; + case DispatchTypes::GraphicsOptions::BlinkOrXterm256Index: + WI_SetFlag(attrs, ExtendedAttributes::Blinking); + break; + case DispatchTypes::GraphicsOptions::Steady: + WI_ClearFlag(attrs, ExtendedAttributes::Blinking); + break; + case DispatchTypes::GraphicsOptions::Invisible: + WI_SetFlag(attrs, ExtendedAttributes::Invisible); + break; + case DispatchTypes::GraphicsOptions::Visible: + WI_ClearFlag(attrs, ExtendedAttributes::Invisible); + break; + case DispatchTypes::GraphicsOptions::CrossedOut: + WI_SetFlag(attrs, ExtendedAttributes::CrossedOut); + break; + case DispatchTypes::GraphicsOptions::NotCrossedOut: + WI_ClearFlag(attrs, ExtendedAttributes::CrossedOut); + break; + // TODO:GH#2916 add support for the following + // case DispatchTypes::GraphicsOptions::DoublyUnderlined: + // case DispatchTypes::GraphicsOptions::RGBColorOrFaint: + // case DispatchTypes::GraphicsOptions::DoublyUnderlined: + } + + return _conApi->PrivateSetExtendedTextAttributes(attrs); +} + +// Routine Description: +// - SGR - Modifies the graphical rendering options applied to the next +// characters written into the buffer. +// - Options include colors, invert, underlines, and other "font style" +// type options. + +// Arguments: +// - rgOptions - An array of options that will be applied from 0 to N, in order, +// one at a time by setting or removing flags in the font style properties. // - cOptions - The count of options (a.k.a. the N in the above line of comments) // Return Value: // - True if handled successfully. False otherwise. -bool AdaptDispatch::SetGraphicsRendition(_In_reads_(cOptions) const DispatchTypes::GraphicsOptions* const rgOptions, const size_t cOptions) +bool AdaptDispatch::SetGraphicsRendition(_In_reads_(cOptions) const DispatchTypes::GraphicsOptions* const rgOptions, + const size_t cOptions) { - // We use the private function here to get just the default color attributes as a performance optimization. - // Calling the public GetConsoleScreenBufferInfoEx costs a lot of performance time/power in a tight loop - // because it has to fill the Largest Window Size by asking the OS and wastes time memcpying colors and other data - // we do not need to resolve this Set Graphics Rendition request. + // We use the private function here to get just the default color attributes + // as a performance optimization. Calling the public + // GetConsoleScreenBufferInfoEx costs a lot of performance time/power in a + // tight loop because it has to fill the Largest Window Size by asking the + // OS and wastes time memcpying colors and other data we do not need to + // resolve this Set Graphics Rendition request. WORD attr; bool fSuccess = !!_conApi->PrivateGetConsoleScreenBufferAttributes(&attr); @@ -416,6 +507,10 @@ bool AdaptDispatch::SetGraphicsRendition(_In_reads_(cOptions) const DispatchType { fSuccess = _SetBoldColorHelper(rgOptions[i]); } + else if (s_IsExtendedTextAttribute(opt)) + { + fSuccess = _SetExtendedTextAttributeHelper(rgOptions[i]); + } else if (s_IsRgbColorOption(opt)) { COLORREF rgbColor; @@ -424,14 +519,21 @@ bool AdaptDispatch::SetGraphicsRendition(_In_reads_(cOptions) const DispatchType size_t cOptionsConsumed = 0; // _SetRgbColorsHelper will call the appropriate ConApi function - fSuccess = _SetRgbColorsHelper(&(rgOptions[i]), cOptions - i, &rgbColor, &fIsForeground, &cOptionsConsumed); + fSuccess = _SetRgbColorsHelper(&(rgOptions[i]), + cOptions - i, + &rgbColor, + &fIsForeground, + &cOptionsConsumed); i += (cOptionsConsumed - 1); // cOptionsConsumed includes the opt we're currently on. } else { _SetGraphicsOptionHelper(opt, &attr); - fSuccess = !!_conApi->PrivateSetLegacyAttributes(attr, _fChangedForeground, _fChangedBackground, _fChangedMetaAttrs); + fSuccess = !!_conApi->PrivateSetLegacyAttributes(attr, + _fChangedForeground, + _fChangedBackground, + _fChangedMetaAttrs); // Make sure we un-bold if (fSuccess && opt == DispatchTypes::GraphicsOptions::Off) diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp index 37b85596c..b337fe2c7 100644 --- a/src/terminal/adapter/conGetSet.hpp +++ b/src/terminal/adapter/conGetSet.hpp @@ -52,6 +52,8 @@ namespace Microsoft::Console::VirtualTerminal const bool fIsForeground) = 0; virtual BOOL SetConsoleRGBTextAttribute(const COLORREF rgbColor, const bool fIsForeground) = 0; virtual BOOL PrivateBoldText(const bool bolded) = 0; + virtual BOOL PrivateGetExtendedTextAttributes(ExtendedAttributes* const pAttrs) = 0; + virtual BOOL PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs) = 0; virtual BOOL PrivateWriteConsoleInputW(_Inout_ std::deque>& events, _Out_ size_t& eventsWritten) = 0; diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index b2a6f4bb8..09258aca3 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -339,6 +339,18 @@ public: return !!_fPrivateBoldTextResult; } + BOOL PrivateGetExtendedTextAttributes(ExtendedAttributes* const /*pAttrs*/) + { + Log::Comment(L"PrivateGetExtendedTextAttributes MOCK called..."); + return true; + } + + BOOL PrivateSetExtendedTextAttributes(const ExtendedAttributes /*attrs*/) + { + Log::Comment(L"PrivateSetExtendedTextAttributes MOCK called..."); + return true; + } + BOOL PrivateWriteConsoleInputW(_Inout_ std::deque>& events, _Out_ size_t& eventsWritten) override { @@ -2630,7 +2642,7 @@ public: Log::Comment(L"Test 1: Change Foreground"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; - rgOptions[1] = DispatchTypes::GraphicsOptions::Xterm256Index; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)2; // Green _testGetSet->_wExpectedAttribute = FOREGROUND_GREEN; _testGetSet->_iExpectedXtermTableEntry = 2; @@ -2640,7 +2652,7 @@ public: Log::Comment(L"Test 2: Change Background"); rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; - rgOptions[1] = DispatchTypes::GraphicsOptions::Xterm256Index; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)9; // Bright Red _testGetSet->_wExpectedAttribute = FOREGROUND_GREEN | BACKGROUND_RED | BACKGROUND_INTENSITY; _testGetSet->_iExpectedXtermTableEntry = 9; @@ -2650,7 +2662,7 @@ public: Log::Comment(L"Test 3: Change Foreground to RGB color"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; - rgOptions[1] = DispatchTypes::GraphicsOptions::Xterm256Index; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)42; // Arbitrary Color _testGetSet->_iExpectedXtermTableEntry = 42; _testGetSet->_fExpectedIsForeground = true; @@ -2659,7 +2671,7 @@ public: Log::Comment(L"Test 4: Change Background to RGB color"); rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; - rgOptions[1] = DispatchTypes::GraphicsOptions::Xterm256Index; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)142; // Arbitrary Color _testGetSet->_iExpectedXtermTableEntry = 142; _testGetSet->_fExpectedIsForeground = false; @@ -2671,7 +2683,7 @@ public: // to have its own color table and translate the pre-existing RGB BG into a legacy BG. // Fortunately, the ft_api:RgbColorTests IS smart enough to test that. rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; - rgOptions[1] = DispatchTypes::GraphicsOptions::Xterm256Index; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)9; // Bright Red _testGetSet->_wExpectedAttribute = FOREGROUND_RED | FOREGROUND_INTENSITY | BACKGROUND_RED | BACKGROUND_INTENSITY; _testGetSet->_iExpectedXtermTableEntry = 9; diff --git a/src/types/inc/utils.hpp b/src/types/inc/utils.hpp index ff1c8dd17..6dfa216ab 100644 --- a/src/types/inc/utils.hpp +++ b/src/types/inc/utils.hpp @@ -11,6 +11,19 @@ Author(s): - Mike Griese (migrie) 12-Jun-2018 --*/ +// Inspired from RETURN_IF_WIN32_BOOL_FALSE +// WIL doesn't include a RETURN_BOOL_IF_FALSE, and RETURN_IF_WIN32_BOOL_FALSE +// will actually return the value of GLE. +#define RETURN_BOOL_IF_FALSE(b) \ + do \ + { \ + BOOL __boolRet = wil::verify_bool(b); \ + if (!__boolRet) \ + { \ + return __boolRet; \ + } \ + } while (0, 0) + namespace Microsoft::Console::Utils { bool IsValidHandle(const HANDLE handle) noexcept;