From 769600bc5943a1a300b09fee753f464a6bea73e7 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 8 Jul 2021 23:19:28 +0200 Subject: [PATCH] wip --- src/buffer/out/TextAttribute.cpp | 4 +- src/buffer/out/TextAttribute.hpp | 31 ++-- src/buffer/out/TextColor.cpp | 134 ++++-------------- src/buffer/out/TextColor.h | 118 ++++++++++----- src/buffer/out/lib/bufferout.vcxproj | 2 +- .../TerminalCore/terminalrenderdata.cpp | 2 +- src/host/consoleInformation.cpp | 2 +- src/host/settings.cpp | 10 -- src/host/settings.hpp | 8 +- src/host/telemetry.cpp | 4 +- 10 files changed, 139 insertions(+), 176 deletions(-) diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index 2791bc352..f3756251b 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -11,6 +11,8 @@ static_assert(sizeof(TextAttribute) == 14); static_assert(alignof(TextAttribute) == 2); // Ensure that we can memcpy() and memmove() the struct for performance. static_assert(std::is_trivially_copyable_v); +// Assert that the use of memcmp() for comparisons is safe. +static_assert(std::has_unique_object_representations_v); BYTE TextAttribute::s_legacyDefaultForeground = 7; BYTE TextAttribute::s_legacyDefaultBackground = 0; @@ -98,7 +100,7 @@ bool TextAttribute::IsLegacy() const noexcept // - blinkingIsFaint: true if blinking should be interpreted as faint. // Return Value: // - the foreground and background colors that should be displayed. -std::pair TextAttribute::CalculateRgbColors(const gsl::span colorTable, +std::pair TextAttribute::CalculateRgbColors(const std::array& colorTable, const COLORREF defaultFgColor, const COLORREF defaultBgColor, const bool reverseScreenMode, diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index e99360da2..f9a68ac33 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -64,7 +64,7 @@ public: static TextAttribute StripErroneousVT16VersionsOfLegacyDefaults(const TextAttribute& attribute) noexcept; WORD GetLegacyAttributes() const noexcept; - std::pair CalculateRgbColors(const gsl::span colorTable, + std::pair CalculateRgbColors(const std::array& colorTable, const COLORREF defaultFgColor, const COLORREF defaultBgColor, const bool reverseScreenMode = false, @@ -82,12 +82,15 @@ public: void Invert() noexcept; - friend constexpr bool operator==(const TextAttribute& a, const TextAttribute& b) noexcept; - friend constexpr bool operator!=(const TextAttribute& a, const TextAttribute& b) noexcept; - friend constexpr bool operator==(const TextAttribute& attr, const WORD& legacyAttr) noexcept; - friend constexpr bool operator!=(const TextAttribute& attr, const WORD& legacyAttr) noexcept; - friend constexpr bool operator==(const WORD& legacyAttr, const TextAttribute& attr) noexcept; - friend constexpr bool operator!=(const WORD& legacyAttr, const TextAttribute& attr) noexcept; + inline bool operator==(const TextAttribute& other) const noexcept + { + return memcmp(this, &other, sizeof(TextAttribute)) == 0; + } + + inline bool operator!=(const TextAttribute& other) const noexcept + { + return memcmp(this, &other, sizeof(TextAttribute)) != 0; + } bool IsLegacy() const noexcept; bool IsBold() const noexcept; @@ -195,20 +198,6 @@ enum class TextAttributeBehavior StoredOnly, // only use the contained text attribute and skip the insertion of anything else }; -constexpr bool operator==(const TextAttribute& a, const TextAttribute& b) noexcept -{ - return a._wAttrLegacy == b._wAttrLegacy && - a._foreground == b._foreground && - a._background == b._background && - a._extendedAttrs == b._extendedAttrs && - a._hyperlinkId == b._hyperlinkId; -} - -constexpr bool operator!=(const TextAttribute& a, const TextAttribute& b) noexcept -{ - return !(a == b); -} - #ifdef UNIT_TESTING #define LOG_ATTR(attr) (Log::Comment(NoThrowString().Format( \ diff --git a/src/buffer/out/TextColor.cpp b/src/buffer/out/TextColor.cpp index 204050229..905210778 100644 --- a/src/buffer/out/TextColor.cpp +++ b/src/buffer/out/TextColor.cpp @@ -50,75 +50,10 @@ constexpr std::array Index256ToIndex16 = { // clang-format on -bool TextColor::CanBeBrightened() const noexcept -{ - return IsIndex16() || IsDefault(); -} - -bool TextColor::IsLegacy() const noexcept -{ - return IsIndex16() || (IsIndex256() && _index < 16); -} - -bool TextColor::IsIndex16() const noexcept -{ - return _meta == ColorType::IsIndex16; -} - -bool TextColor::IsIndex256() const noexcept -{ - return _meta == ColorType::IsIndex256; -} - -bool TextColor::IsDefault() const noexcept -{ - return _meta == ColorType::IsDefault; -} - -bool TextColor::IsRgb() const noexcept -{ - return _meta == ColorType::IsRgb; -} - -// Method Description: -// - Sets the color value of this attribute, and sets this color to be an RGB -// attribute. -// Arguments: -// - rgbColor: the COLORREF containing the color information for this TextColor -// Return Value: -// - -void TextColor::SetColor(const COLORREF rgbColor) noexcept -{ - _meta = ColorType::IsRgb; - _red = GetRValue(rgbColor); - _green = GetGValue(rgbColor); - _blue = GetBValue(rgbColor); -} - -// Method Description: -// - Sets this TextColor to be a legacy-style index into the color table. -// Arguments: -// - index: the index of the colortable we should use for this TextColor. -// - isIndex256: is this a 256 color index (true) or a 16 color index (false). -// Return Value: -// - -void TextColor::SetIndex(const BYTE index, const bool isIndex256) noexcept -{ - _meta = isIndex256 ? ColorType::IsIndex256 : ColorType::IsIndex16; - _index = index; -} - -// Method Description: -// - Sets this TextColor to be a default text color, who's appearance is -// controlled by the terminal's implementation of what a default color is. -// Arguments: -// - -// Return Value: -// - -void TextColor::SetDefault() noexcept -{ - _meta = ColorType::IsDefault; -} +// We should only need 4B for TextColor. Any more than that is just waste. +static_assert(sizeof(TextColor) == 4); +// Assert that the use of memcmp() for comparisons is safe. +static_assert(std::has_unique_object_representations_v); // Method Description: // - Retrieve the real color value for this TextColor. @@ -138,7 +73,7 @@ void TextColor::SetDefault() noexcept // - brighten: if true, we'll brighten a dark color table index. // Return Value: // - a COLORREF containing the real value of this TextColor. -COLORREF TextColor::GetColor(gsl::span colorTable, +COLORREF TextColor::GetColor(const std::array& colorTable, const COLORREF defaultColor, bool brighten) const noexcept { @@ -146,7 +81,6 @@ COLORREF TextColor::GetColor(gsl::span colorTable, { if (brighten) { - FAIL_FAST_IF(colorTable.size() < 16); // See MSFT:20266024 for context on this fix. // Additionally todo MSFT:20271956 to fix this better for 19H2+ // If we're a default color, check to see if the defaultColor exists @@ -156,6 +90,18 @@ COLORREF TextColor::GetColor(gsl::span colorTable, // (Settings::_DefaultForeground==INVALID_COLOR, and the index // from _wFillAttribute is being used instead.) // If we find a match, return instead the bright version of this color +#ifdef _M_AMD64 + const auto needle = _mm_set1_epi32(defaultColor); + const auto haystack1 = _mm_loadu_si128(reinterpret_cast(colorTable.data() + 0)); + const auto haystack2 = _mm_loadu_si128(reinterpret_cast(colorTable.data() + 4)); + const auto result1 = _mm_cmpeq_epi32(haystack1, needle); + const auto result2 = _mm_cmpeq_epi32(haystack2, needle); + const auto shuffle16to8 = _mm_setr_epi8(0, 2, 4, 6, 8, 10, 12, 14, -1, -1, -1, -1, -1, -1, -1, -1); + const auto result = _mm_shuffle_epi8(_mm_packs_epi16(result1, result2), shuffle16to8); + const auto combined = _mm_movemask_epi8(result); + unsigned long index; + return _BitScanForward(&index, combined) ? til::at(colorTable, index + 8) : defaultColor; +#else for (size_t i = 0; i < 8; i++) { if (til::at(colorTable, i) == defaultColor) @@ -163,6 +109,7 @@ COLORREF TextColor::GetColor(gsl::span colorTable, return til::at(colorTable, i + 8); } } +#endif } return defaultColor; @@ -171,13 +118,9 @@ COLORREF TextColor::GetColor(gsl::span colorTable, { return GetRGB(); } - else if (IsIndex16() && brighten) - { - return til::at(colorTable, _index | 8); - } else { - return til::at(colorTable, _index); + return til::at(colorTable, IsIndex16() & brighten ? _index : _index | 8); } } @@ -189,37 +132,20 @@ COLORREF TextColor::GetColor(gsl::span colorTable, // - an index into the 16-color table BYTE TextColor::GetLegacyIndex(const BYTE defaultIndex) const noexcept { - if (IsDefault()) + switch (_meta) { + case ColorType::IsDefault: return defaultIndex; - } - else if (IsIndex16()) - { - return GetIndex(); - } - else if (IsIndex256()) - { - return Index256ToIndex16.at(GetIndex()); - } - else - { + case ColorType::IsIndex16: + return _index; + case ColorType::IsIndex256: + return Index256ToIndex16[_index]; + default: // We compress the RGB down to an 8-bit value and use that to // lookup a representative 16-color index from a hard-coded table. - const BYTE compressedRgb = (_red & 0b11100000) + - ((_green >> 3) & 0b00011100) + - ((_blue >> 6) & 0b00000011); - return CompressedRgbToIndex16.at(compressedRgb); + const BYTE compressedRgb = (_red & 0b11100000) | + ((_green >> 3) & 0b00011100) | + (_blue >> 6); + return CompressedRgbToIndex16[compressedRgb]; } } - -// Method Description: -// - Return a COLORREF containing our stored value. Will return garbage if this -//attribute is not a RGB attribute. -// Arguments: -// - -// Return Value: -// - a COLORREF containing our stored value -COLORREF TextColor::GetRGB() const noexcept -{ - return RGB(_red, _green, _blue); -} diff --git a/src/buffer/out/TextColor.h b/src/buffer/out/TextColor.h index e8dcfcd82..9a259cd05 100644 --- a/src/buffer/out/TextColor.h +++ b/src/buffer/out/TextColor.h @@ -39,12 +39,23 @@ Revision History: enum class ColorType : BYTE { - IsIndex256 = 0x0, - IsIndex16 = 0x1, - IsDefault = 0x2, - IsRgb = 0x3 + None = 0b0000, + IsDefault = 0b0001, + IsIndex16 = 0b0010, + IsIndex256 = 0b0100, + IsRgb = 0b1000, }; +constexpr ColorType operator|(ColorType lhs, ColorType rhs) noexcept +{ + return static_cast(static_cast(lhs) | static_cast(rhs)); +} + +constexpr ColorType operator&(ColorType lhs, ColorType rhs) noexcept +{ + return static_cast(static_cast(lhs) & static_cast(rhs)); +} + struct TextColor { public: @@ -72,21 +83,74 @@ public: { } - friend constexpr bool operator==(const TextColor& a, const TextColor& b) noexcept; - friend constexpr bool operator!=(const TextColor& a, const TextColor& b) noexcept; + bool operator==(const TextColor& other) const noexcept + { + return memcmp(this, &other, sizeof(TextColor)) == 0; + } - bool CanBeBrightened() const noexcept; - bool IsLegacy() const noexcept; - bool IsIndex16() const noexcept; - bool IsIndex256() const noexcept; - bool IsDefault() const noexcept; - bool IsRgb() const noexcept; + bool operator!=(const TextColor& other) const noexcept + { + return memcmp(this, &other, sizeof(TextColor)) != 0; + } - void SetColor(const COLORREF rgbColor) noexcept; - void SetIndex(const BYTE index, const bool isIndex256) noexcept; - void SetDefault() noexcept; + constexpr bool CanBeBrightened() const noexcept + { + return WI_IsAnyFlagSet(_meta, ColorType::IsDefault | ColorType::IsIndex16); + } - COLORREF GetColor(gsl::span colorTable, + constexpr bool IsLegacy() const noexcept + { + // This is basically: + // return IsIndex16() || (IsIndex256() && _index < 16); + // but without any branches. + return (_index < 16) & WI_IsAnyFlagSet(_meta, ColorType::IsIndex16 | ColorType::IsIndex256); + } + + constexpr bool IsIndex16() const noexcept + { + return _meta == ColorType::IsIndex16; + } + + constexpr bool IsIndex256() const noexcept + { + return _meta == ColorType::IsIndex256; + } + + constexpr bool IsDefault() const noexcept + { + return _meta == ColorType::IsDefault; + } + + constexpr bool IsRgb() const noexcept + { + return _meta == ColorType::IsRgb; + } + + inline void SetColor(const COLORREF rgbColor) noexcept + { + _meta = ColorType::IsRgb; + _red = GetRValue(rgbColor); + _green = GetGValue(rgbColor); + _blue = GetBValue(rgbColor); + } + + inline void SetIndex(const BYTE index, const bool isIndex256) noexcept + { + _meta = isIndex256 ? ColorType::IsIndex256 : ColorType::IsIndex16; + _index = index; + _green = 0; + _blue = 0; + } + + inline void SetDefault() noexcept + { + _meta = ColorType::IsDefault; + _index = 0; + _green = 0; + _blue = 0; + } + + COLORREF GetColor(const std::array& colorTable, const COLORREF defaultColor, const bool brighten = false) const noexcept; @@ -97,16 +161,19 @@ public: return _index; } - COLORREF GetRGB() const noexcept; + constexpr COLORREF GetRGB() const noexcept + { + return RGB(_red, _green, _blue); + } private: + ColorType _meta; union { BYTE _red, _index; }; BYTE _green; BYTE _blue; - ColorType _meta; #ifdef UNIT_TESTING friend class TextBufferTests; @@ -115,19 +182,6 @@ private: #endif }; -bool constexpr operator==(const TextColor& a, const TextColor& b) noexcept -{ - return a._meta == b._meta && - a._red == b._red && - a._green == b._green && - a._blue == b._blue; -} - -bool constexpr operator!=(const TextColor& a, const TextColor& b) noexcept -{ - return !(a == b); -} - #ifdef UNIT_TESTING namespace WEX @@ -157,5 +211,3 @@ namespace WEX } } #endif - -static_assert(sizeof(TextColor) <= 4 * sizeof(BYTE), "We should only need 4B for an entire TextColor. Any more than that is just waste"); diff --git a/src/buffer/out/lib/bufferout.vcxproj b/src/buffer/out/lib/bufferout.vcxproj index 2a5ea58de..cfe899c73 100644 --- a/src/buffer/out/lib/bufferout.vcxproj +++ b/src/buffer/out/lib/bufferout.vcxproj @@ -44,7 +44,7 @@ - + diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp index 5566e7c8d..b24104ca6 100644 --- a/src/cascadia/TerminalCore/terminalrenderdata.cpp +++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp @@ -45,7 +45,7 @@ const TextAttribute Terminal::GetDefaultBrushColors() noexcept std::pair Terminal::GetAttributeColors(const TextAttribute& attr) const noexcept { _blinkingState.RecordBlinkingUsage(attr); - auto colors = attr.CalculateRgbColors({ _colorTable.data(), _colorTable.size() }, + auto colors = attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, _screenReversed, diff --git a/src/host/consoleInformation.cpp b/src/host/consoleInformation.cpp index 81ef9faa7..ba45a69cb 100644 --- a/src/host/consoleInformation.cpp +++ b/src/host/consoleInformation.cpp @@ -259,7 +259,7 @@ COLORREF CONSOLE_INFORMATION::GetDefaultBackground() const noexcept std::pair CONSOLE_INFORMATION::LookupAttributeColors(const TextAttribute& attr) const noexcept { _blinkingState.RecordBlinkingUsage(attr); - return attr.CalculateRgbColors(Get256ColorTable(), + return attr.CalculateRgbColors(GetColorTable(), GetDefaultForeground(), GetDefaultBackground(), IsScreenReversed(), diff --git a/src/host/settings.cpp b/src/host/settings.cpp index af1de09e3..a9fb5d093 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -726,16 +726,6 @@ void Settings::SetHistoryNoDup(const bool bHistoryNoDup) _bHistoryNoDup = bHistoryNoDup; } -gsl::span Settings::Get16ColorTable() const -{ - return Get256ColorTable().subspan(0, 16); -} - -gsl::span Settings::Get256ColorTable() const -{ - return { _colorTable.data(), _colorTable.size() }; -} - void Settings::SetColorTableEntry(const size_t index, const COLORREF ColorValue) { _colorTable.at(index) = ColorValue; diff --git a/src/host/settings.hpp b/src/host/settings.hpp index 25eb06dac..e8468da6b 100644 --- a/src/host/settings.hpp +++ b/src/host/settings.hpp @@ -159,8 +159,12 @@ public: bool GetHistoryNoDup() const; void SetHistoryNoDup(const bool fHistoryNoDup); - gsl::span Get16ColorTable() const; - gsl::span Get256ColorTable() const; + // The first 16 items of the color table are the same as the 16-color palette. + inline const std::array& GetColorTable() const noexcept + { + return _colorTable; + } + void SetColorTableEntry(const size_t index, const COLORREF ColorValue); COLORREF GetColorTableEntry(const size_t index) const; diff --git a/src/host/telemetry.cpp b/src/host/telemetry.cpp index 7b4ce210b..7924f2f69 100644 --- a/src/host/telemetry.cpp +++ b/src/host/telemetry.cpp @@ -419,7 +419,7 @@ void Telemetry::WriteFinalTraceLog() TraceLoggingBool(gci.GetQuickEdit(), "QuickEdit"), TraceLoggingValue(gci.GetWindowAlpha(), "WindowAlpha"), TraceLoggingBool(gci.GetWrapText(), "WrapText"), - TraceLoggingUInt32Array((UINT32 const*)gci.Get16ColorTable().data(), (UINT16)gci.Get16ColorTable().size(), "ColorTable"), + TraceLoggingUInt32Array((UINT32 const*)gci.GetColorTable().data(), 16, "ColorTable"), TraceLoggingValue(gci.CP, "CodePageInput"), TraceLoggingValue(gci.OutputCP, "CodePageOutput"), TraceLoggingValue(gci.GetFontSize().X, "FontSizeX"), @@ -453,7 +453,7 @@ void Telemetry::WriteFinalTraceLog() TraceLoggingValue(gci.GetShowWindow(), "ShowWindow"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); - static_assert(sizeof(UINT32) == sizeof(gci.Get16ColorTable()[0]), "gci.Get16ColorTable()"); + static_assert(sizeof(UINT32) == sizeof(gci.GetColorTable()[0]), "gci.Get16ColorTable()"); // I could use the TraceLoggingUIntArray, but then we would have to know the order of the enums on the backend. // So just log each enum count separately with its string representation which makes it more human readable.