From 287870ca4d137599d59f9814fd3dd3a79aee3afd Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 25 Jun 2020 18:00:22 +0100 Subject: [PATCH 1/6] Add static properties in TextAttribute to track the legacy default colors. --- src/buffer/out/TextAttribute.cpp | 23 +++++++++++++++++++---- src/buffer/out/TextAttribute.hpp | 6 +++++- src/host/settings.cpp | 6 +++++- src/interactivity/win32/menu.cpp | 3 +++ 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index 1d8692ccf19..0fc8c28136c 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -5,16 +5,31 @@ #include "TextAttribute.hpp" #include "../../inc/conattrs.hpp" +BYTE TextAttribute::s_legacyDefaultForeground = 7; +BYTE TextAttribute::s_legacyDefaultBackground = 0; + // Routine Description: -// - Returns a WORD with legacy-style attributes for this textattribute. +// - Sets the legacy attributes which map to and from the default colors. // Parameters: // - defaultAttributes: the attribute values to be used for default colors. // Return value: +// - None +void TextAttribute::SetLegacyDefaultAttributes(const WORD defaultAttributes) noexcept +{ + s_legacyDefaultForeground = defaultAttributes & FG_ATTRS; + s_legacyDefaultBackground = (defaultAttributes & BG_ATTRS) >> 4; +} + +// Routine Description: +// - Returns a WORD with legacy-style attributes for this textattribute. +// Parameters: +// - None +// Return value: // - a WORD with legacy-style attributes for this textattribute. -WORD TextAttribute::GetLegacyAttributes(const WORD defaultAttributes) const noexcept +WORD TextAttribute::GetLegacyAttributes() const noexcept { - const BYTE fgIndex = _foreground.GetLegacyIndex(defaultAttributes & FG_ATTRS); - const BYTE bgIndex = _background.GetLegacyIndex((defaultAttributes & BG_ATTRS) >> 4); + const BYTE fgIndex = _foreground.GetLegacyIndex(s_legacyDefaultForeground); + const BYTE bgIndex = _background.GetLegacyIndex(s_legacyDefaultBackground); const WORD metaAttrs = _wAttrLegacy & META_ATTRS; const bool brighten = _foreground.IsIndex16() && IsBold(); return fgIndex | (bgIndex << 4) | metaAttrs | (brighten ? FOREGROUND_INTENSITY : 0); diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index edb6e771012..41a06f5f6bd 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -59,7 +59,8 @@ class TextAttribute final { } - WORD GetLegacyAttributes(const WORD defaultAttributes = 0x07) const noexcept; + static void SetLegacyDefaultAttributes(const WORD defaultAttributes) noexcept; + WORD GetLegacyAttributes() const noexcept; COLORREF CalculateRgbForeground(std::basic_string_view colorTable, COLORREF defaultFgColor, @@ -151,6 +152,9 @@ class TextAttribute final COLORREF _GetRgbBackground(std::basic_string_view colorTable, COLORREF defaultColor) const noexcept; + static BYTE s_legacyDefaultForeground; + static BYTE s_legacyDefaultBackground; + WORD _wAttrLegacy; TextColor _foreground; TextColor _background; diff --git a/src/host/settings.cpp b/src/host/settings.cpp index a9fc90131de..36a458f1b5f 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -349,6 +349,10 @@ void Settings::Validate() } } + // At this point the default fill attributes are fully initialized + // so we can pass on the final colors to the TextAttribute class. + TextAttribute::SetLegacyDefaultAttributes(_wFillAttribute); + FAIL_FAST_IF(!(_dwWindowSize.X > 0)); FAIL_FAST_IF(!(_dwWindowSize.Y > 0)); FAIL_FAST_IF(!(_dwScreenBufferSize.X > 0)); @@ -767,7 +771,7 @@ COLORREF Settings::GetColorTableEntry(const size_t index) const // - A WORD representing the legacy attributes that most closely represent the given fullcolor attributes. WORD Settings::GenerateLegacyAttributes(const TextAttribute attributes) const { - return attributes.GetLegacyAttributes(_wFillAttribute); + return attributes.GetLegacyAttributes(); } COLORREF Settings::GetCursorColor() const noexcept diff --git a/src/interactivity/win32/menu.cpp b/src/interactivity/win32/menu.cpp index bba8402750c..ed747fcc3a6 100644 --- a/src/interactivity/win32/menu.cpp +++ b/src/interactivity/win32/menu.cpp @@ -583,6 +583,9 @@ void Menu::s_PropertiesUpdate(PCONSOLE_STATE_INFO pStateInfo) gci.SetDefaultForegroundColor(pStateInfo->DefaultForeground); gci.SetDefaultBackgroundColor(pStateInfo->DefaultBackground); + // Make sure the updated fill attributes are passed on to the TextAttribute class. + TextAttribute::SetLegacyDefaultAttributes(pStateInfo->ScreenAttributes); + // Set the screen info's default text attributes to defaults - ScreenInfo.SetDefaultAttributes(gci.GetDefaultAttributes(), TextAttribute{ gci.GetPopupFillAttribute() }); From 871777542fdf380e85406d18794f6c5f5843a105 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 25 Jun 2020 18:42:24 +0100 Subject: [PATCH 2/6] Translate default colors when constructing a TextAttribute from a legacy value. --- src/buffer/out/TextAttribute.hpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index 41a06f5f6bd..24478aad86e 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -42,8 +42,8 @@ class TextAttribute final explicit constexpr TextAttribute(const WORD wLegacyAttr) noexcept : _wAttrLegacy{ gsl::narrow_cast(wLegacyAttr & META_ATTRS) }, - _foreground{ gsl::narrow_cast(wLegacyAttr & FG_ATTRS), true }, - _background{ gsl::narrow_cast((wLegacyAttr & BG_ATTRS) >> 4), true }, + _foreground{ s_LegacyIndexOrDefault(wLegacyAttr & FG_ATTRS, s_legacyDefaultForeground) }, + _background{ s_LegacyIndexOrDefault((wLegacyAttr & BG_ATTRS) >> 4, s_legacyDefaultBackground) }, _extendedAttrs{ ExtendedAttributes::Normal } { // If we're given lead/trailing byte information with the legacy color, strip it. @@ -152,6 +152,11 @@ class TextAttribute final COLORREF _GetRgbBackground(std::basic_string_view colorTable, COLORREF defaultColor) const noexcept; + static constexpr TextColor s_LegacyIndexOrDefault(const BYTE requestedIndex, const BYTE defaultIndex) + { + return requestedIndex == defaultIndex ? TextColor{} : TextColor{ requestedIndex, true }; + } + static BYTE s_legacyDefaultForeground; static BYTE s_legacyDefaultBackground; From d0a253c9bc3d6d9554ad937f967274ff9b36e02c Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 25 Jun 2020 18:58:37 +0100 Subject: [PATCH 3/6] Get rid of the Settings::GetDefaultAttributes method that isn't needed anymore. --- src/host/output.cpp | 2 +- src/host/settings.cpp | 14 -------------- src/host/settings.hpp | 2 -- src/host/ut_host/ScreenBufferTests.cpp | 18 +++++++++--------- src/inc/test/CommonState.hpp | 4 ++-- src/interactivity/win32/menu.cpp | 2 +- 6 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/host/output.cpp b/src/host/output.cpp index ae8dcd41164..33731880ab1 100644 --- a/src/host/output.cpp +++ b/src/host/output.cpp @@ -41,7 +41,7 @@ using namespace Microsoft::Console::Interactivity; NTSTATUS Status = SCREEN_INFORMATION::CreateInstance(gci.GetWindowSize(), fiFont, gci.GetScreenBufferSize(), - gci.GetDefaultAttributes(), + TextAttribute{}, TextAttribute{ gci.GetPopupFillAttribute() }, gci.GetCursorSize(), &gci.ScreenBuffers); diff --git a/src/host/settings.cpp b/src/host/settings.cpp index 36a458f1b5f..42a5e684745 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -824,20 +824,6 @@ void Settings::SetDefaultBackgroundColor(const COLORREF defaultBackground) noexc _DefaultBackground = defaultBackground; } -TextAttribute Settings::GetDefaultAttributes() const noexcept -{ - auto attrs = TextAttribute{ _wFillAttribute }; - if (_DefaultForeground != INVALID_COLOR) - { - attrs.SetDefaultForeground(); - } - if (_DefaultBackground != INVALID_COLOR) - { - attrs.SetDefaultBackground(); - } - return attrs; -} - bool Settings::IsTerminalScrolling() const noexcept { return _TerminalScrolling; diff --git a/src/host/settings.hpp b/src/host/settings.hpp index 2163204c9a4..3e41e04a56e 100644 --- a/src/host/settings.hpp +++ b/src/host/settings.hpp @@ -179,8 +179,6 @@ class Settings COLORREF GetDefaultBackgroundColor() const noexcept; void SetDefaultBackgroundColor(const COLORREF defaultBackground) noexcept; - TextAttribute GetDefaultAttributes() const noexcept; - bool IsTerminalScrolling() const noexcept; void SetTerminalScrolling(const bool terminalScrollingEnabled) noexcept; diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 608ac078e51..e948d7d3e72 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -1317,7 +1317,7 @@ void ScreenBufferTests::VtScrollMarginsNewlineColor() const COLORREF magenta = RGB(255, 0, 255); gci.SetDefaultForegroundColor(yellow); gci.SetDefaultBackgroundColor(magenta); - const TextAttribute defaultAttrs = gci.GetDefaultAttributes(); + const TextAttribute defaultAttrs = {}; si.SetAttributes(defaultAttrs); Log::Comment(NoThrowString().Format(L"Begin by clearing the screen.")); @@ -2071,7 +2071,7 @@ void ScreenBufferTests::TestAltBufferVtDispatching() WI_SetFlag(mainBuffer.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING); // Make sure we're suing the default attributes at the start of the test, // Otherwise they could be polluted from a previous test. - mainBuffer.SetAttributes(gci.GetDefaultAttributes()); + mainBuffer.SetAttributes({}); VERIFY_IS_NULL(mainBuffer._psiAlternateBuffer); VERIFY_IS_NULL(mainBuffer._psiMainBuffer); @@ -2121,7 +2121,7 @@ void ScreenBufferTests::TestAltBufferVtDispatching() // recall: vt coordinates are (row, column), 1-indexed VERIFY_ARE_EQUAL(COORD({ 5, 4 }), altCursor.GetPosition()); - const TextAttribute expectedDefaults = gci.GetDefaultAttributes(); + const TextAttribute expectedDefaults = {}; TextAttribute expectedRgb = expectedDefaults; expectedRgb.SetBackground(RGB(255, 0, 255)); @@ -2194,7 +2194,7 @@ void ScreenBufferTests::SetDefaultsIndividuallyBothDefault() gci.SetDefaultForegroundColor(yellow); gci.SetDefaultBackgroundColor(magenta); - si.SetDefaultAttributes(gci.GetDefaultAttributes(), TextAttribute{ gci.GetPopupFillAttribute() }); + si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 6 X's:")); Log::Comment(NoThrowString().Format(L" The first in default-fg on default-bg (yellow on magenta)")); @@ -2303,7 +2303,7 @@ void ScreenBufferTests::SetDefaultsTogether() gci.SetDefaultForegroundColor(yellow); gci.SetDefaultBackgroundColor(magenta); - si.SetDefaultAttributes(gci.GetDefaultAttributes(), TextAttribute{ gci.GetPopupFillAttribute() }); + si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 6 X's:")); Log::Comment(NoThrowString().Format(L" The first in default-fg on default-bg (yellow on magenta)")); @@ -2378,7 +2378,7 @@ void ScreenBufferTests::ReverseResetWithDefaultBackground() gci.SetDefaultForegroundColor(INVALID_COLOR); gci.SetDefaultBackgroundColor(magenta); - si.SetDefaultAttributes(gci.GetDefaultAttributes(), TextAttribute{ gci.GetPopupFillAttribute() }); + si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 3 X's:")); Log::Comment(NoThrowString().Format(L" The first in default-attr on default color (magenta)")); @@ -2446,7 +2446,7 @@ void ScreenBufferTests::BackspaceDefaultAttrs() COLORREF magenta = RGB(255, 0, 255); gci.SetDefaultBackgroundColor(magenta); - si.SetDefaultAttributes(gci.GetDefaultAttributes(), TextAttribute{ gci.GetPopupFillAttribute() }); + si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 2 X's, then backspace one.")); @@ -2509,7 +2509,7 @@ void ScreenBufferTests::BackspaceDefaultAttrsWriteCharsLegacy() COLORREF magenta = RGB(255, 0, 255); gci.SetDefaultBackgroundColor(magenta); - si.SetDefaultAttributes(gci.GetDefaultAttributes(), TextAttribute{ gci.GetPopupFillAttribute() }); + si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 2 X's, then backspace one.")); @@ -2577,7 +2577,7 @@ void ScreenBufferTests::BackspaceDefaultAttrsInPrompt() COLORREF magenta = RGB(255, 0, 255); gci.SetDefaultBackgroundColor(magenta); - si.SetDefaultAttributes(gci.GetDefaultAttributes(), TextAttribute{ gci.GetPopupFillAttribute() }); + si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); TextAttribute expectedDefaults{}; Log::Comment(NoThrowString().Format(L"Write 3 X's, move to the left, then delete-char the second.")); diff --git a/src/inc/test/CommonState.hpp b/src/inc/test/CommonState.hpp index 4c5fde73628..b493babe4e4 100644 --- a/src/inc/test/CommonState.hpp +++ b/src/inc/test/CommonState.hpp @@ -100,7 +100,7 @@ class CommonState THROW_IF_FAILED(SCREEN_INFORMATION::CreateInstance(coordWindowSize, *m_pFontInfo, coordScreenBufferSize, - gci.GetDefaultAttributes(), + TextAttribute{}, TextAttribute{ FOREGROUND_BLUE | FOREGROUND_INTENSITY | BACKGROUND_RED }, uiCursorSize, &gci.pCurrentScreenBuffer)); @@ -157,7 +157,7 @@ class CommonState UINT uiCursorSize = 12; - auto initialAttributes = useDefaultAttributes ? gci.GetDefaultAttributes() : + auto initialAttributes = useDefaultAttributes ? TextAttribute{} : TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | BACKGROUND_RED | BACKGROUND_INTENSITY }; m_backupTextBufferInfo.swap(gci.pCurrentScreenBuffer->_textBuffer); diff --git a/src/interactivity/win32/menu.cpp b/src/interactivity/win32/menu.cpp index ed747fcc3a6..76b3252e003 100644 --- a/src/interactivity/win32/menu.cpp +++ b/src/interactivity/win32/menu.cpp @@ -587,7 +587,7 @@ void Menu::s_PropertiesUpdate(PCONSOLE_STATE_INFO pStateInfo) TextAttribute::SetLegacyDefaultAttributes(pStateInfo->ScreenAttributes); // Set the screen info's default text attributes to defaults - - ScreenInfo.SetDefaultAttributes(gci.GetDefaultAttributes(), TextAttribute{ gci.GetPopupFillAttribute() }); + ScreenInfo.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); CommandHistory::s_ResizeAll(pStateInfo->HistoryBufferSize); gci.SetNumberOfHistoryBuffers(pStateInfo->NumberOfHistoryBuffers); From 9d6363f3661aaf69689b80ea58f0532f3a333383 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 25 Jun 2020 19:05:23 +0100 Subject: [PATCH 4/6] Get rid of the Settings::GenerateLegacyAttributes method that isn't needed anymore. --- src/host/consoleInformation.cpp | 7 +++---- src/host/getset.cpp | 2 +- src/host/output.cpp | 3 +-- src/host/screenInfo.cpp | 8 +++----- src/host/settings.cpp | 13 ------------- src/host/settings.hpp | 3 --- 6 files changed, 8 insertions(+), 28 deletions(-) diff --git a/src/host/consoleInformation.cpp b/src/host/consoleInformation.cpp index 32ec1959ee4..4812ec6d544 100644 --- a/src/host/consoleInformation.cpp +++ b/src/host/consoleInformation.cpp @@ -356,8 +356,8 @@ Microsoft::Console::CursorBlinker& CONSOLE_INFORMATION::GetCursorBlinker() noexc } // Method Description: -// - Generates a CHAR_INFO for this output cell, using our -// GenerateLegacyAttributes method to generate the legacy style attributes. +// - Generates a CHAR_INFO for this output cell, using the TextAttribute +// GetLegacyAttributes method to generate the legacy style attributes. // Arguments: // - cell: The cell to get the CHAR_INFO from // Return Value: @@ -371,8 +371,7 @@ CHAR_INFO CONSOLE_INFORMATION::AsCharInfo(const OutputCellView& cell) const noex // use gci to look up the correct legacy attributes to use // (for mapping RGB values to the nearest table value) const auto& attr = cell.TextAttr(); - ci.Attributes = GenerateLegacyAttributes(attr); - ; + ci.Attributes = attr.GetLegacyAttributes(); ci.Attributes |= cell.DbcsAttr().GeneratePublicApiAttributeFormat(); return ci; } diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 8ee25582560..f6ab8cb6062 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -881,7 +881,7 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont (target.Y == -currentBufferDimensions.Y); const bool noClipProvided = clip == std::nullopt; const bool fillIsBlank = (fillCharacter == UNICODE_SPACE) && - (fillAttribute == gci.GenerateLegacyAttributes(buffer.GetAttributes())); + (fillAttribute == buffer.GetAttributes().GetLegacyAttributes()); if (sourceIsWholeBuffer && targetIsNegativeBufferHeight && noClipProvided && fillIsBlank) { diff --git a/src/host/output.cpp b/src/host/output.cpp index 33731880ab1..29be702d314 100644 --- a/src/host/output.cpp +++ b/src/host/output.cpp @@ -148,8 +148,7 @@ std::vector ReadOutputAttributes(const SCREEN_INFORMATION& screenInfo, // While we haven't read enough cells yet and the iterator is still valid (hasn't reached end of buffer) while (amountRead < amountToRead && it) { - const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - const auto legacyAttributes = gci.GenerateLegacyAttributes(it->TextAttr()); + const auto legacyAttributes = it->TextAttr().GetLegacyAttributes(); // If the first thing we read is trailing, pad with a space. // OR If the last thing we read is leading, pad with a space. diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 96dff5edcbd..30f0712ad2f 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -356,8 +356,8 @@ void SCREEN_INFORMATION::GetScreenBufferInformation(_Out_ PCOORD pcoordSize, *psrWindow = _viewport.ToInclusive(); - *pwAttributes = gci.GenerateLegacyAttributes(GetAttributes()); - *pwPopupAttributes = gci.GenerateLegacyAttributes(_PopupAttributes); + *pwAttributes = GetAttributes().GetLegacyAttributes(); + *pwPopupAttributes = _PopupAttributes.GetLegacyAttributes(); // the copy length must be constant for now to keep OACR happy with buffer overruns. for (size_t i = 0; i < COLOR_TABLE_SIZE; i++) @@ -572,8 +572,6 @@ void SCREEN_INFORMATION::NotifyAccessibilityEventing(const short sStartX, const short sEndX, const short sEndY) { - const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - // Fire off a winevent to let accessibility apps know what changed. if (IsActiveScreenBuffer()) { @@ -586,7 +584,7 @@ void SCREEN_INFORMATION::NotifyAccessibilityEventing(const short sStartX, { const auto cellData = GetCellDataAt({ sStartX, sStartY }); const LONG charAndAttr = MAKELONG(Utf16ToUcs2(cellData->Chars()), - gci.GenerateLegacyAttributes(cellData->TextAttr())); + cellData->TextAttr().GetLegacyAttributes()); _pAccessibilityNotifier->NotifyConsoleUpdateSimpleEvent(MAKELONG(sStartX, sStartY), charAndAttr); } diff --git a/src/host/settings.cpp b/src/host/settings.cpp index 42a5e684745..1eb534aa0b0 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -761,19 +761,6 @@ COLORREF Settings::GetColorTableEntry(const size_t index) const return _colorTable.at(index); } -// Routine Description: -// - Generates a legacy attribute from the given TextAttributes. -// This needs to be a method on the Settings because the generated index -// is dependent upon the default fill attributes. -// Parameters: -// - attributes - The TextAttributes to generate a legacy attribute for. -// Return value: -// - A WORD representing the legacy attributes that most closely represent the given fullcolor attributes. -WORD Settings::GenerateLegacyAttributes(const TextAttribute attributes) const -{ - return attributes.GetLegacyAttributes(); -} - COLORREF Settings::GetCursorColor() const noexcept { return _CursorColor; diff --git a/src/host/settings.hpp b/src/host/settings.hpp index 3e41e04a56e..a7d719ff205 100644 --- a/src/host/settings.hpp +++ b/src/host/settings.hpp @@ -250,7 +250,4 @@ class Settings COLORREF _DefaultBackground; bool _TerminalScrolling; friend class RegistrySerialization; - -public: - WORD GenerateLegacyAttributes(const TextAttribute attributes) const; }; From 1afa9a4c09f16db330fd112b1fe052b645385665 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 25 Jun 2020 19:51:18 +0100 Subject: [PATCH 5/6] Remove legacy check which is no longer applicable for all legacy attribute values. --- src/buffer/out/ut_textbuffer/TextAttributeTests.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp index b4cc587dd80..4cf7b02dd33 100644 --- a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp +++ b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp @@ -117,13 +117,10 @@ void TextAttributeTests::TestRoundtripExhaustive() auto attr = TextAttribute(wLegacy); - bool isLegacy = attr.IsLegacy(); - bool areEqual = (wLegacy == attr.GetLegacyAttributes()); - if (!(isLegacy && areEqual)) + if (wLegacy != attr.GetLegacyAttributes()) { Log::Comment(NoThrowString().Format( L"Failed on wLegacy=0x%x", wLegacy)); - VERIFY_IS_TRUE(attr.IsLegacy()); VERIFY_ARE_EQUAL(wLegacy, attr.GetLegacyAttributes()); } } From 15c369e14d7785a02f515f5c529d7a851a13087d Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 25 Jun 2020 20:41:35 +0100 Subject: [PATCH 6/6] Add test to confirm legacy default index values are correctly mapped to default colors. --- .../out/ut_textbuffer/TextAttributeTests.cpp | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp index 4cf7b02dd33..f9dfcc76f57 100644 --- a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp +++ b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp @@ -21,6 +21,7 @@ class TextAttributeTests TEST_METHOD(TestRoundtripExhaustive); TEST_METHOD(TestTextAttributeColorGetters); TEST_METHOD(TestReverseDefaultColors); + TEST_METHOD(TestRoundtripDefaultColors); static const int COLOR_TABLE_SIZE = 16; COLORREF _colorTable[COLOR_TABLE_SIZE]; @@ -202,3 +203,44 @@ void TextAttributeTests::TestReverseDefaultColors() VERIFY_ARE_EQUAL(green, attr._GetRgbBackground(view, _defaultBg)); VERIFY_ARE_EQUAL(green, attr.CalculateRgbBackground(view, _defaultFg, _defaultBg)); } + +void TextAttributeTests::TestRoundtripDefaultColors() +{ + // Set the legacy default colors to yellow on blue. + const BYTE fgLegacyDefault = FOREGROUND_RED; + const BYTE bgLegacyDefault = BACKGROUND_BLUE; + TextAttribute::SetLegacyDefaultAttributes(fgLegacyDefault | bgLegacyDefault); + + WORD legacyAttribute; + TextAttribute textAttribute; + + Log::Comment(L"Foreground legacy default index should map to default text color."); + legacyAttribute = fgLegacyDefault | BACKGROUND_GREEN; + textAttribute.SetDefaultForeground(); + textAttribute.SetIndexedBackground256(BACKGROUND_GREEN >> 4); + VERIFY_ARE_EQUAL(textAttribute, TextAttribute{ legacyAttribute }); + + Log::Comment(L"Default foreground text color should map back to legacy default index."); + VERIFY_ARE_EQUAL(legacyAttribute, textAttribute.GetLegacyAttributes()); + + Log::Comment(L"Background legacy default index should map to default text color."); + legacyAttribute = FOREGROUND_GREEN | bgLegacyDefault; + textAttribute.SetIndexedForeground256(FOREGROUND_GREEN); + textAttribute.SetDefaultBackground(); + VERIFY_ARE_EQUAL(textAttribute, TextAttribute{ legacyAttribute }); + + Log::Comment(L"Default background text color should map back to legacy default index."); + VERIFY_ARE_EQUAL(legacyAttribute, textAttribute.GetLegacyAttributes()); + + Log::Comment(L"Foreground and background legacy defaults should map to default text colors."); + legacyAttribute = fgLegacyDefault | bgLegacyDefault; + textAttribute.SetDefaultForeground(); + textAttribute.SetDefaultBackground(); + VERIFY_ARE_EQUAL(textAttribute, TextAttribute{ legacyAttribute }); + + Log::Comment(L"Default foreground and background text colors should map back to legacy defaults."); + VERIFY_ARE_EQUAL(legacyAttribute, textAttribute.GetLegacyAttributes()); + + // Reset the legacy default colors to white on black. + TextAttribute::SetLegacyDefaultAttributes(FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE); +}