From 79859cff84b84565ff3f3f6f7679eae70951f0da Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 7 Nov 2021 00:00:31 +0000 Subject: [PATCH 1/8] Merge default colors into color table. --- src/buffer/out/TextAttribute.cpp | 2 +- src/buffer/out/TextAttribute.hpp | 2 +- src/buffer/out/TextColor.cpp | 2 +- src/buffer/out/TextColor.h | 6 ++- .../out/ut_textbuffer/TextAttributeTests.cpp | 2 +- .../out/ut_textbuffer/TextColorTests.cpp | 2 +- src/cascadia/TerminalCore/Terminal.cpp | 13 ++++-- src/cascadia/TerminalCore/Terminal.hpp | 4 +- src/cascadia/TerminalCore/TerminalApi.cpp | 6 +-- .../TerminalCore/terminalrenderdata.cpp | 12 ++--- .../ConptyRoundtripTests.cpp | 4 +- .../TerminalApiTest.cpp | 1 - src/host/consoleInformation.cpp | 4 +- src/host/getset.cpp | 4 +- src/host/registry.cpp | 24 ++++++++++ src/host/settings.cpp | 43 ++++++----------- src/host/settings.hpp | 12 +---- src/host/ut_host/ConptyOutputTests.cpp | 4 +- src/host/ut_host/ScreenBufferTests.cpp | 46 +++++++++---------- src/inc/conattrs.hpp | 1 - src/interactivity/win32/menu.cpp | 8 ++-- src/propslib/RegistrySerialization.cpp | 2 - 22 files changed, 103 insertions(+), 101 deletions(-) diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index b99c3121d30..d8c86325e2f 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -135,7 +135,7 @@ bool TextAttribute::IsLegacy() const noexcept // - boldIsBright: true if "bold" should be interpreted as bright. (defaults to true) // Return Value: // - the foreground and background colors that should be displayed. -std::pair TextAttribute::CalculateRgbColors(const std::array& 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 e973a9cd3cd..3be8d8c5a64 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -64,7 +64,7 @@ class TextAttribute final static TextAttribute StripErroneousVT16VersionsOfLegacyDefaults(const TextAttribute& attribute) noexcept; WORD GetLegacyAttributes() const noexcept; - std::pair CalculateRgbColors(const std::array& colorTable, + std::pair CalculateRgbColors(const std::array& colorTable, const COLORREF defaultFgColor, const COLORREF defaultBgColor, const bool reverseScreenMode = false, diff --git a/src/buffer/out/TextColor.cpp b/src/buffer/out/TextColor.cpp index 88bc8657f35..a416e761d89 100644 --- a/src/buffer/out/TextColor.cpp +++ b/src/buffer/out/TextColor.cpp @@ -143,7 +143,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(const std::array& colorTable, const COLORREF defaultColor, bool brighten) const noexcept +COLORREF TextColor::GetColor(const std::array& colorTable, const COLORREF defaultColor, bool brighten) const noexcept { if (IsDefault()) { diff --git a/src/buffer/out/TextColor.h b/src/buffer/out/TextColor.h index 35b62ff6557..8b782c59833 100644 --- a/src/buffer/out/TextColor.h +++ b/src/buffer/out/TextColor.h @@ -65,6 +65,10 @@ struct TextColor static constexpr BYTE BRIGHT_CYAN = 14; static constexpr BYTE BRIGHT_WHITE = 15; + static constexpr size_t DEFAULT_FOREGROUND = 256; + static constexpr size_t DEFAULT_BACKGROUND = 257; + static constexpr size_t TABLE_SIZE = 258; + constexpr TextColor() noexcept : _meta{ ColorType::IsDefault }, _red{ 0 }, @@ -103,7 +107,7 @@ struct TextColor void SetIndex(const BYTE index, const bool isIndex256) noexcept; void SetDefault() noexcept; - COLORREF GetColor(const std::array& colorTable, const COLORREF defaultColor, bool brighten = false) const noexcept; + COLORREF GetColor(const std::array& colorTable, const COLORREF defaultColor, bool brighten = false) const noexcept; BYTE GetLegacyIndex(const BYTE defaultIndex) const noexcept; constexpr BYTE GetIndex() const noexcept diff --git a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp index 1b61955bba4..952b94056a4 100644 --- a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp +++ b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp @@ -24,7 +24,7 @@ class TextAttributeTests TEST_METHOD(TestRoundtripDefaultColors); TEST_METHOD(TestBoldAsBright); - std::array _colorTable; + std::array _colorTable; COLORREF _defaultFg = RGB(1, 2, 3); COLORREF _defaultBg = RGB(4, 5, 6); }; diff --git a/src/buffer/out/ut_textbuffer/TextColorTests.cpp b/src/buffer/out/ut_textbuffer/TextColorTests.cpp index d419bfc33c5..78f42341a64 100644 --- a/src/buffer/out/ut_textbuffer/TextColorTests.cpp +++ b/src/buffer/out/ut_textbuffer/TextColorTests.cpp @@ -23,7 +23,7 @@ class TextColorTests TEST_METHOD(TestRgbColor); TEST_METHOD(TestChangeColor); - std::array _colorTable; + std::array _colorTable; COLORREF _defaultFg = RGB(1, 2, 3); COLORREF _defaultBg = RGB(4, 5, 6); }; diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 6f8d06453d1..1621716c968 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -41,8 +41,6 @@ Terminal::Terminal() : _mutableViewport{ Viewport::Empty() }, _title{}, _colorTable{}, - _defaultFg{ RGB(255, 255, 255) }, - _defaultBg{ ARGB(0, 0, 0, 0) }, _screenReversed{ false }, _pfnWriteInput{ nullptr }, _scrollOffset{ 0 }, @@ -81,6 +79,9 @@ Terminal::Terminal() : _terminalInput = std::make_unique(passAlongInput); _InitializeColorTable(); + + _colorTable.at(TextColor::DEFAULT_FOREGROUND) = RGB(255, 255, 255); + _colorTable.at(TextColor::DEFAULT_BACKGROUND) = ARGB(0, 0, 0, 0); } void Terminal::Create(COORD viewportSize, SHORT scrollbackLines, IRenderTarget& renderTarget) @@ -180,9 +181,11 @@ void Terminal::UpdateAppearance(const ICoreAppearance& appearance) { // Set the default background as transparent to prevent the // DX layer from overwriting the background image or acrylic effect - til::color newBackgroundColor{ appearance.DefaultBackground() }; - _defaultBg = newBackgroundColor.with_alpha(0); - _defaultFg = appearance.DefaultForeground(); + const til::color newBackgroundColor{ appearance.DefaultBackground() }; + _colorTable.at(TextColor::DEFAULT_BACKGROUND) = newBackgroundColor.with_alpha(0); + const til::color newForegroundColor{ appearance.DefaultForeground() }; + _colorTable.at(TextColor::DEFAULT_FOREGROUND) = newForegroundColor; + _intenseIsBright = appearance.IntenseIsBright(); _adjustIndistinguishableColors = appearance.AdjustIndistinguishableColors(); diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 5a7b0875165..48ba235b7e5 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -281,9 +281,7 @@ class Microsoft::Terminal::Core::Terminal final : std::optional _startingTabColor; // This is still stored as a COLORREF because it interacts with some code in ConTypes - std::array _colorTable; - til::color _defaultFg; - til::color _defaultBg; + std::array _colorTable; CursorType _defaultCursorShape; bool _screenReversed; mutable Microsoft::Console::Render::BlinkingState _blinkingState; diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 455851ac8de..9579d0dd906 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -451,7 +451,7 @@ bool Terminal::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle) noex bool Terminal::SetDefaultForeground(const COLORREF color) noexcept try { - _defaultFg = color; + _colorTable.at(TextColor::DEFAULT_FOREGROUND) = color; // Repaint everything - the colors might have changed _buffer->GetRenderTarget().TriggerRedrawAll(); @@ -468,7 +468,7 @@ CATCH_RETURN_FALSE() bool Terminal::SetDefaultBackground(const COLORREF color) noexcept try { - _defaultBg = color; + _colorTable.at(TextColor::DEFAULT_BACKGROUND) = color; _pfnBackgroundColorChanged(color); // Repaint everything - the colors might have changed @@ -479,7 +479,7 @@ CATCH_RETURN_FALSE() til::color Terminal::GetDefaultBackground() const noexcept { - return _defaultBg; + return _colorTable.at(TextColor::DEFAULT_BACKGROUND); } bool Terminal::SetInputMode(const TerminalInput::Mode mode, const bool enabled) noexcept diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp index df3483ab0f3..53f5b89eba0 100644 --- a/src/cascadia/TerminalCore/terminalrenderdata.cpp +++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp @@ -73,19 +73,19 @@ std::pair Terminal::GetAttributeColors(const TextAttribute& if (attr.IsReverseVideo() ^ _screenReversed) { colors.first = _adjustedForegroundColors[fgIndex][bgIndex]; - colors.second = fgTextColor.GetColor(_colorTable, _defaultFg); + colors.second = fgTextColor.GetColor(_colorTable, _colorTable.at(TextColor::DEFAULT_FOREGROUND)); } else { colors.first = _adjustedForegroundColors[bgIndex][fgIndex]; - colors.second = bgTextColor.GetColor(_colorTable, _defaultBg); + colors.second = bgTextColor.GetColor(_colorTable, _colorTable.at(TextColor::DEFAULT_BACKGROUND)); } } else { colors = attr.CalculateRgbColors(_colorTable, - _defaultFg, - _defaultBg, + _colorTable.at(TextColor::DEFAULT_FOREGROUND), + _colorTable.at(TextColor::DEFAULT_BACKGROUND), _screenReversed, _blinkingState.IsBlinkingFaint(), _intenseIsBright); @@ -312,8 +312,8 @@ void Terminal::_MakeAdjustedColorArray() // to include the default background and default foreground colors std::array colorTableWithDefaults; std::copy_n(std::begin(_colorTable), 16, std::begin(colorTableWithDefaults)); - colorTableWithDefaults[DefaultBgIndex] = _defaultBg; - colorTableWithDefaults[DefaultFgIndex] = _defaultFg; + colorTableWithDefaults[DefaultBgIndex] = _colorTable.at(TextColor::DEFAULT_BACKGROUND); + colorTableWithDefaults[DefaultFgIndex] = _colorTable.at(TextColor::DEFAULT_FOREGROUND); for (auto fgIndex = 0; fgIndex < 18; ++fgIndex) { const auto fg = til::at(colorTableWithDefaults, fgIndex); diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 2145032fd1f..dca50a5121c 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -96,8 +96,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final auto& g = ServiceLocator::LocateGlobals(); auto& gci = g.getConsoleInformation(); - gci.SetDefaultForegroundColor(INVALID_COLOR); - gci.SetDefaultBackgroundColor(INVALID_COLOR); + gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, INVALID_COLOR); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, INVALID_COLOR); gci.SetFillAttribute(0x07); // DARK_WHITE on DARK_BLACK m_state->PrepareNewTextBufferInfo(true, TerminalViewWidth, TerminalViewHeight); diff --git a/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp b/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp index acb85c49237..8606be708f8 100644 --- a/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp @@ -59,7 +59,6 @@ void TerminalApiTest::SetColorTableEntry() VERIFY_IS_TRUE(term.SetColorTableEntry(128, 100)); VERIFY_IS_TRUE(term.SetColorTableEntry(255, 100)); - VERIFY_IS_FALSE(term.SetColorTableEntry(256, 100)); VERIFY_IS_FALSE(term.SetColorTableEntry(512, 100)); } diff --git a/src/host/consoleInformation.cpp b/src/host/consoleInformation.cpp index 51ed7de9b57..6ba8ac719dc 100644 --- a/src/host/consoleInformation.cpp +++ b/src/host/consoleInformation.cpp @@ -230,7 +230,7 @@ InputBuffer* const CONSOLE_INFORMATION::GetActiveInputBuffer() const // - the default foreground color of the console. COLORREF CONSOLE_INFORMATION::GetDefaultForeground() const noexcept { - const auto fg = GetDefaultForegroundColor(); + const auto fg = GetColorTableEntry(TextColor::DEFAULT_FOREGROUND); return fg != INVALID_COLOR ? fg : GetLegacyColorTableEntry(LOBYTE(GetFillAttribute()) & FG_ATTRS); } @@ -245,7 +245,7 @@ COLORREF CONSOLE_INFORMATION::GetDefaultForeground() const noexcept // - the default background color of the console. COLORREF CONSOLE_INFORMATION::GetDefaultBackground() const noexcept { - const auto bg = GetDefaultBackgroundColor(); + const auto bg = GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); return bg != INVALID_COLOR ? bg : GetLegacyColorTableEntry((LOBYTE(GetFillAttribute()) & BG_ATTRS) >> 4); } diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 4f85d4a5b20..ab70e570a0b 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -2031,7 +2031,7 @@ void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo) Globals& g = ServiceLocator::LocateGlobals(); CONSOLE_INFORMATION& gci = g.getConsoleInformation(); - gci.SetDefaultForegroundColor(value); + gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, value); // Update the screen colors if we're not a pty // No need to force a redraw in pty mode. @@ -2058,7 +2058,7 @@ void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo) Globals& g = ServiceLocator::LocateGlobals(); CONSOLE_INFORMATION& gci = g.getConsoleInformation(); - gci.SetDefaultBackgroundColor(value); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, value); // Update the screen colors if we're not a pty // No need to force a redraw in pty mode. diff --git a/src/host/registry.cpp b/src/host/registry.cpp index 49bd0642354..f7f3b7bcb64 100644 --- a/src/host/registry.cpp +++ b/src/host/registry.cpp @@ -314,6 +314,30 @@ void Registry::LoadFromRegistry(_In_ PCWSTR const pwszConsoleTitle) } } + // Default foreground color + Status = RegistrySerialization::s_QueryValue(hTitleKey, + CONSOLE_REGISTRY_DEFAULTFOREGROUND, + sizeof(dwValue), + REG_DWORD, + (PBYTE)&dwValue, + nullptr); + if (NT_SUCCESS(Status)) + { + _pSettings->SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, dwValue); + } + + // Default background color + Status = RegistrySerialization::s_QueryValue(hTitleKey, + CONSOLE_REGISTRY_DEFAULTBACKGROUND, + sizeof(dwValue), + REG_DWORD, + (PBYTE)&dwValue, + nullptr); + if (NT_SUCCESS(Status)) + { + _pSettings->SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, dwValue); + } + GetEditKeys(hConsoleKey); // Close the registry keys diff --git a/src/host/settings.cpp b/src/host/settings.cpp index d5918166870..7af88f0bfd6 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -56,8 +56,6 @@ Settings::Settings() : _fScreenReversed(false), // window size pixels initialized below _fInterceptCopyPaste(0), - _DefaultForeground(INVALID_COLOR), - _DefaultBackground(INVALID_COLOR), _fUseDx(UseDx::Disabled), _fCopyColor(false) { @@ -83,6 +81,9 @@ Settings::Settings() : gsl::span tableView = { _colorTable.data(), _colorTable.size() }; ::Microsoft::Console::Utils::InitializeColorTable(tableView); + + _colorTable.at(TextColor::DEFAULT_FOREGROUND) = INVALID_COLOR; + _colorTable.at(TextColor::DEFAULT_BACKGROUND) = INVALID_COLOR; } // Routine Description: @@ -232,8 +233,8 @@ void Settings::InitFromStateInfo(_In_ PCONSOLE_STATE_INFO pStateInfo) _CursorColor = pStateInfo->CursorColor; _CursorType = static_cast(pStateInfo->CursorType); _fInterceptCopyPaste = pStateInfo->InterceptCopyPaste; - _DefaultForeground = pStateInfo->DefaultForeground; - _DefaultBackground = pStateInfo->DefaultBackground; + _colorTable.at(TextColor::DEFAULT_FOREGROUND) = pStateInfo->DefaultForeground; + _colorTable.at(TextColor::DEFAULT_BACKGROUND) = pStateInfo->DefaultBackground; _TerminalScrolling = pStateInfo->TerminalScrolling; } @@ -277,8 +278,8 @@ CONSOLE_STATE_INFO Settings::CreateConsoleStateInfo() const csi.CursorColor = _CursorColor; csi.CursorType = static_cast(_CursorType); csi.InterceptCopyPaste = _fInterceptCopyPaste; - csi.DefaultForeground = _DefaultForeground; - csi.DefaultBackground = _DefaultBackground; + csi.DefaultForeground = _colorTable.at(TextColor::DEFAULT_FOREGROUND); + csi.DefaultBackground = _colorTable.at(TextColor::DEFAULT_BACKGROUND); csi.TerminalScrolling = _TerminalScrolling; return csi; } @@ -330,16 +331,20 @@ void Settings::Validate() WI_ClearAllFlags(_wFillAttribute, ~(FG_ATTRS | BG_ATTRS)); WI_ClearAllFlags(_wPopupFillAttribute, ~(FG_ATTRS | BG_ATTRS)); + const auto defaultForeground = _colorTable.at(TextColor::DEFAULT_FOREGROUND); + const auto defaultBackground = _colorTable.at(TextColor::DEFAULT_BACKGROUND); + // If the extended color options are set to invalid values (all the same color), reset them. - if (_CursorColor != Cursor::s_InvertCursorColor && _CursorColor == _DefaultBackground) + if (_CursorColor != Cursor::s_InvertCursorColor && _CursorColor == defaultBackground) { _CursorColor = Cursor::s_InvertCursorColor; } - if (_DefaultForeground != INVALID_COLOR && _DefaultForeground == _DefaultBackground) + if (defaultForeground != INVALID_COLOR && defaultForeground == defaultBackground) { // INVALID_COLOR is used as an "unset" sentinel in future attribute functions. - _DefaultForeground = _DefaultBackground = INVALID_COLOR; + _colorTable.at(TextColor::DEFAULT_FOREGROUND) = INVALID_COLOR; + _colorTable.at(TextColor::DEFAULT_BACKGROUND) = INVALID_COLOR; // If the damaged settings _further_ propagated to the default fill attribute, fix it. if (_wFillAttribute == 0) { @@ -790,26 +795,6 @@ void Settings::SetInterceptCopyPaste(const bool interceptCopyPaste) noexcept _fInterceptCopyPaste = interceptCopyPaste; } -COLORREF Settings::GetDefaultForegroundColor() const noexcept -{ - return _DefaultForeground; -} - -void Settings::SetDefaultForegroundColor(const COLORREF defaultForeground) noexcept -{ - _DefaultForeground = defaultForeground; -} - -COLORREF Settings::GetDefaultBackgroundColor() const noexcept -{ - return _DefaultBackground; -} - -void Settings::SetDefaultBackgroundColor(const COLORREF defaultBackground) noexcept -{ - _DefaultBackground = defaultBackground; -} - bool Settings::IsTerminalScrolling() const noexcept { return _TerminalScrolling; diff --git a/src/host/settings.hpp b/src/host/settings.hpp index 3963be4bc39..65015cc7fb1 100644 --- a/src/host/settings.hpp +++ b/src/host/settings.hpp @@ -167,7 +167,7 @@ class Settings void SetHistoryNoDup(const bool fHistoryNoDup); // The first 16 items of the color table are the same as the 16-color palette. - inline const std::array& GetColorTable() const noexcept + inline const std::array& GetColorTable() const noexcept { return _colorTable; } @@ -186,12 +186,6 @@ class Settings bool GetInterceptCopyPaste() const noexcept; void SetInterceptCopyPaste(const bool interceptCopyPaste) noexcept; - COLORREF GetDefaultForegroundColor() const noexcept; - void SetDefaultForegroundColor(const COLORREF defaultForeground) noexcept; - - COLORREF GetDefaultBackgroundColor() const noexcept; - void SetDefaultBackgroundColor(const COLORREF defaultBackground) noexcept; - bool IsTerminalScrolling() const noexcept; void SetTerminalScrolling(const bool terminalScrollingEnabled) noexcept; @@ -242,7 +236,7 @@ class Settings UseDx _fUseDx; bool _fCopyColor; - std::array _colorTable; + std::array _colorTable; // this is used for the special STARTF_USESIZE mode. bool _fUseWindowSizePixels; @@ -254,8 +248,6 @@ class Settings bool _fInterceptCopyPaste; - COLORREF _DefaultForeground; - COLORREF _DefaultBackground; bool _TerminalScrolling; friend class RegistrySerialization; }; diff --git a/src/host/ut_host/ConptyOutputTests.cpp b/src/host/ut_host/ConptyOutputTests.cpp index d412fb5625f..a8865c78377 100644 --- a/src/host/ut_host/ConptyOutputTests.cpp +++ b/src/host/ut_host/ConptyOutputTests.cpp @@ -67,8 +67,8 @@ class ConptyOutputTests // Set up some sane defaults auto& g = ServiceLocator::LocateGlobals(); auto& gci = g.getConsoleInformation(); - gci.SetDefaultForegroundColor(INVALID_COLOR); - gci.SetDefaultBackgroundColor(INVALID_COLOR); + gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, INVALID_COLOR); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, INVALID_COLOR); gci.SetFillAttribute(0x07); // DARK_WHITE on DARK_BLACK m_state->PrepareNewTextBufferInfo(true, TerminalViewWidth, TerminalViewHeight); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 1b65bfc2a09..91fdf00378e 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -59,8 +59,8 @@ class ScreenBufferTests { // Set up some sane defaults CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - gci.SetDefaultForegroundColor(INVALID_COLOR); - gci.SetDefaultBackgroundColor(INVALID_COLOR); + gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, INVALID_COLOR); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, INVALID_COLOR); gci.SetFillAttribute(0x07); // DARK_WHITE on DARK_BLACK m_state->PrepareNewTextBufferInfo(); @@ -1388,8 +1388,8 @@ void ScreenBufferTests::VtScrollMarginsNewlineColor() const COLORREF yellow = RGB(255, 255, 0); const COLORREF magenta = RGB(255, 0, 255); - gci.SetDefaultForegroundColor(yellow); - gci.SetDefaultBackgroundColor(magenta); + gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, yellow); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); const TextAttribute defaultAttrs = {}; si.SetAttributes(defaultAttrs); @@ -2259,8 +2259,8 @@ void ScreenBufferTests::SetDefaultsIndividuallyBothDefault() COLORREF brightGreen = gci.GetColorTableEntry(TextColor::BRIGHT_GREEN); COLORREF darkBlue = gci.GetColorTableEntry(TextColor::DARK_BLUE); - gci.SetDefaultForegroundColor(yellow); - gci.SetDefaultBackgroundColor(magenta); + gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, yellow); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 6 X's:")); @@ -2361,8 +2361,8 @@ void ScreenBufferTests::SetDefaultsTogether() COLORREF yellow = RGB(255, 255, 0); COLORREF color250 = gci.GetColorTableEntry(250); - gci.SetDefaultForegroundColor(yellow); - gci.SetDefaultBackgroundColor(magenta); + gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, yellow); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 6 X's:")); @@ -2432,8 +2432,8 @@ void ScreenBufferTests::ReverseResetWithDefaultBackground() COLORREF magenta = RGB(255, 0, 255); - gci.SetDefaultForegroundColor(INVALID_COLOR); - gci.SetDefaultBackgroundColor(magenta); + gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, INVALID_COLOR); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 3 X's:")); @@ -2501,7 +2501,7 @@ void ScreenBufferTests::BackspaceDefaultAttrs() COLORREF magenta = RGB(255, 0, 255); - gci.SetDefaultBackgroundColor(magenta); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 2 X's, then backspace one.")); @@ -2564,7 +2564,7 @@ void ScreenBufferTests::BackspaceDefaultAttrsWriteCharsLegacy() COLORREF magenta = RGB(255, 0, 255); - gci.SetDefaultBackgroundColor(magenta); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 2 X's, then backspace one.")); @@ -2632,7 +2632,7 @@ void ScreenBufferTests::BackspaceDefaultAttrsInPrompt() COLORREF magenta = RGB(255, 0, 255); - gci.SetDefaultBackgroundColor(magenta); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); TextAttribute expectedDefaults{}; @@ -2889,15 +2889,15 @@ void ScreenBufferTests::SetDefaultForegroundColor() StateMachine& stateMachine = mainBuffer.GetStateMachine(); - COLORREF originalColor = gci.GetDefaultForegroundColor(); - COLORREF newColor = gci.GetDefaultForegroundColor(); + COLORREF originalColor = gci.GetColorTableEntry(TextColor::DEFAULT_FOREGROUND); + COLORREF newColor = gci.GetColorTableEntry(TextColor::DEFAULT_FOREGROUND); COLORREF testColor = RGB(0x33, 0x66, 0x99); VERIFY_ARE_NOT_EQUAL(originalColor, testColor); Log::Comment(L"Valid Hexadecimal Notation"); stateMachine.ProcessString(L"\x1b]10;rgb:33/66/99\x1b\\"); - newColor = gci.GetDefaultForegroundColor(); + newColor = gci.GetColorTableEntry(TextColor::DEFAULT_FOREGROUND); VERIFY_ARE_EQUAL(testColor, newColor); Log::Comment(L"Valid Hexadecimal Notation"); @@ -2905,7 +2905,7 @@ void ScreenBufferTests::SetDefaultForegroundColor() testColor = RGB(0xff, 0xff, 0xff); stateMachine.ProcessString(L"\x1b]10;rgb:ff/ff/ff\x1b\\"); - newColor = gci.GetDefaultForegroundColor(); + newColor = gci.GetColorTableEntry(TextColor::DEFAULT_FOREGROUND); VERIFY_ARE_EQUAL(testColor, newColor); Log::Comment(L"Invalid syntax"); @@ -2913,7 +2913,7 @@ void ScreenBufferTests::SetDefaultForegroundColor() testColor = RGB(153, 102, 51); stateMachine.ProcessString(L"\x1b]10;99/66/33\x1b\\"); - newColor = gci.GetDefaultForegroundColor(); + newColor = gci.GetColorTableEntry(TextColor::DEFAULT_FOREGROUND); VERIFY_ARE_NOT_EQUAL(testColor, newColor); // it will, in fact leave the color the way it was VERIFY_ARE_EQUAL(originalColor, newColor); @@ -2934,15 +2934,15 @@ void ScreenBufferTests::SetDefaultBackgroundColor() StateMachine& stateMachine = mainBuffer.GetStateMachine(); - COLORREF originalColor = gci.GetDefaultBackgroundColor(); - COLORREF newColor = gci.GetDefaultBackgroundColor(); + COLORREF originalColor = gci.GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); + COLORREF newColor = gci.GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); COLORREF testColor = RGB(0x33, 0x66, 0x99); VERIFY_ARE_NOT_EQUAL(originalColor, testColor); Log::Comment(L"Valid Hexadecimal Notation"); stateMachine.ProcessString(L"\x1b]11;rgb:33/66/99\x1b\\"); - newColor = gci.GetDefaultBackgroundColor(); + newColor = gci.GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); VERIFY_ARE_EQUAL(testColor, newColor); Log::Comment(L"Valid Hexadecimal Notation"); @@ -2950,7 +2950,7 @@ void ScreenBufferTests::SetDefaultBackgroundColor() testColor = RGB(0xff, 0xff, 0xff); stateMachine.ProcessString(L"\x1b]11;rgb:ff/ff/ff\x1b\\"); - newColor = gci.GetDefaultBackgroundColor(); + newColor = gci.GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); VERIFY_ARE_EQUAL(testColor, newColor); Log::Comment(L"Invalid Syntax"); @@ -2958,7 +2958,7 @@ void ScreenBufferTests::SetDefaultBackgroundColor() testColor = RGB(153, 102, 51); stateMachine.ProcessString(L"\x1b]11;99/66/33\x1b\\"); - newColor = gci.GetDefaultBackgroundColor(); + newColor = gci.GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); VERIFY_ARE_NOT_EQUAL(testColor, newColor); // it will, in fact leave the color the way it was VERIFY_ARE_EQUAL(originalColor, newColor); diff --git a/src/inc/conattrs.hpp b/src/inc/conattrs.hpp index 8589590d402..0f542e436c8 100644 --- a/src/inc/conattrs.hpp +++ b/src/inc/conattrs.hpp @@ -37,4 +37,3 @@ enum class CursorType : unsigned int constexpr COLORREF INVALID_COLOR = 0xffffffff; constexpr WORD COLOR_TABLE_SIZE = 16; -constexpr WORD XTERM_COLOR_TABLE_SIZE = 256; diff --git a/src/interactivity/win32/menu.cpp b/src/interactivity/win32/menu.cpp index 9364e636429..401c39abda9 100644 --- a/src/interactivity/win32/menu.cpp +++ b/src/interactivity/win32/menu.cpp @@ -376,8 +376,8 @@ void Menu::s_ShowPropertiesDialog(HWND const hwnd, BOOL const Defaults) pStateInfo->InterceptCopyPaste = gci.GetInterceptCopyPaste(); // Get the properties from the settings - pStateInfo->DefaultForeground = gci.GetDefaultForegroundColor(); - pStateInfo->DefaultBackground = gci.GetDefaultBackgroundColor(); + pStateInfo->DefaultForeground = gci.GetColorTableEntry(TextColor::DEFAULT_FOREGROUND); + pStateInfo->DefaultBackground = gci.GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); pStateInfo->TerminalScrolling = gci.IsTerminalScrolling(); // end console v2 properties @@ -579,8 +579,8 @@ void Menu::s_PropertiesUpdate(PCONSOLE_STATE_INFO pStateInfo) gci.SetFillAttribute(pStateInfo->ScreenAttributes); gci.SetPopupFillAttribute(pStateInfo->PopupAttributes); // Store our updated Default Color values - gci.SetDefaultForegroundColor(pStateInfo->DefaultForeground); - gci.SetDefaultBackgroundColor(pStateInfo->DefaultBackground); + gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, pStateInfo->DefaultForeground); + gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, pStateInfo->DefaultBackground); // Make sure the updated fill attributes are passed on to the TextAttribute class. TextAttribute::SetLegacyDefaultAttributes(pStateInfo->ScreenAttributes); diff --git a/src/propslib/RegistrySerialization.cpp b/src/propslib/RegistrySerialization.cpp index d9e9bcfbeb5..e5a4610f91d 100644 --- a/src/propslib/RegistrySerialization.cpp +++ b/src/propslib/RegistrySerialization.cpp @@ -60,8 +60,6 @@ const RegistrySerialization::_RegPropertyMap RegistrySerialization::s_PropertyMa { _RegPropertyType::Dword, CONSOLE_REGISTRY_CURSORCOLOR, SET_FIELD_AND_SIZE(_CursorColor) }, { _RegPropertyType::Dword, CONSOLE_REGISTRY_CURSORTYPE, SET_FIELD_AND_SIZE(_CursorType) }, { _RegPropertyType::Boolean, CONSOLE_REGISTRY_INTERCEPTCOPYPASTE, SET_FIELD_AND_SIZE(_fInterceptCopyPaste) }, - { _RegPropertyType::Dword, CONSOLE_REGISTRY_DEFAULTFOREGROUND, SET_FIELD_AND_SIZE(_DefaultForeground) }, - { _RegPropertyType::Dword, CONSOLE_REGISTRY_DEFAULTBACKGROUND, SET_FIELD_AND_SIZE(_DefaultBackground) }, { _RegPropertyType::Boolean, CONSOLE_REGISTRY_TERMINALSCROLLING, SET_FIELD_AND_SIZE(_TerminalScrolling) }, { _RegPropertyType::Dword, CONSOLE_REGISTRY_USEDX, SET_FIELD_AND_SIZE(_fUseDx) }, { _RegPropertyType::Boolean, CONSOLE_REGISTRY_COPYCOLOR, SET_FIELD_AND_SIZE(_fCopyColor) } From 05e20f6ebc9f9a81812d5fb2fdaa7810d6794371 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 7 Nov 2021 00:39:53 +0000 Subject: [PATCH 2/8] Eliminate default color APIs in ConGetSet and ITerminalApi. --- .../PublicTerminalCore/HwndTerminal.cpp | 8 +-- src/cascadia/TerminalControl/ControlCore.cpp | 2 +- src/cascadia/TerminalCore/ITerminalApi.hpp | 3 - src/cascadia/TerminalCore/Terminal.hpp | 3 - src/cascadia/TerminalCore/TerminalApi.cpp | 45 ++------------- .../TerminalCore/TerminalDispatch.cpp | 4 +- src/host/getset.cpp | 56 +------------------ src/host/getset.h | 4 -- src/host/outputStream.cpp | 24 -------- src/host/outputStream.hpp | 4 -- src/terminal/adapter/adaptDispatch.cpp | 8 +-- src/terminal/adapter/conGetSet.hpp | 2 - .../adapter/ut_adapter/adapterTest.cpp | 28 ---------- 13 files changed, 17 insertions(+), 174 deletions(-) diff --git a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp index 6a1a8282cb5..389d2ac762d 100644 --- a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp +++ b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp @@ -239,8 +239,8 @@ HRESULT HwndTerminal::Initialize() _terminal->SetBackgroundCallback([](auto) {}); _terminal->Create(COORD{ 80, 25 }, 1000, *_renderer); - _terminal->SetDefaultBackground(RGB(12, 12, 12)); - _terminal->SetDefaultForeground(RGB(204, 204, 204)); + _terminal->SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, RGB(12, 12, 12)); + _terminal->SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, RGB(204, 204, 204)); _terminal->SetWriteInputCallback([=](std::wstring& input) noexcept { _WriteTextToConnection(input); }); localPointerToThread->EnablePainting(); @@ -781,8 +781,8 @@ void _stdcall TerminalSetTheme(void* terminal, TerminalTheme theme, LPCWSTR font { auto lock = publicTerminal->_terminal->LockForWriting(); - publicTerminal->_terminal->SetDefaultForeground(theme.DefaultForeground); - publicTerminal->_terminal->SetDefaultBackground(theme.DefaultBackground); + publicTerminal->_terminal->SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, theme.DefaultForeground); + publicTerminal->_terminal->SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, theme.DefaultBackground); publicTerminal->_renderEngine->SetSelectionBackground(theme.DefaultSelectionBackground, theme.SelectionBackgroundAlpha); // Set the font colors diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index b08e4c4741c..5f4dcb91594 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1089,7 +1089,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation til::color ControlCore::BackgroundColor() const { - return _terminal->GetDefaultBackground(); + return _terminal->GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); } // Method Description: diff --git a/src/cascadia/TerminalCore/ITerminalApi.hpp b/src/cascadia/TerminalCore/ITerminalApi.hpp index 3f09cea8f02..40e7c879b2b 100644 --- a/src/cascadia/TerminalCore/ITerminalApi.hpp +++ b/src/cascadia/TerminalCore/ITerminalApi.hpp @@ -45,9 +45,6 @@ namespace Microsoft::Terminal::Core virtual bool SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle) noexcept = 0; virtual bool SetCursorColor(const DWORD color) noexcept = 0; - virtual bool SetDefaultForeground(const DWORD color) noexcept = 0; - virtual bool SetDefaultBackground(const DWORD color) noexcept = 0; - virtual bool SetInputMode(const ::Microsoft::Console::VirtualTerminal::TerminalInput::Mode mode, const bool enabled) noexcept = 0; virtual bool SetScreenMode(const bool reverseMode) noexcept = 0; diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 48ba235b7e5..15568951033 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -109,8 +109,6 @@ class Microsoft::Terminal::Core::Terminal final : bool SetColorTableEntry(const size_t tableIndex, const COLORREF color) noexcept override; bool SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle) noexcept override; bool SetCursorColor(const COLORREF color) noexcept override; - bool SetDefaultForeground(const COLORREF color) noexcept override; - bool SetDefaultBackground(const COLORREF color) noexcept override; bool SetInputMode(const ::Microsoft::Console::VirtualTerminal::TerminalInput::Mode mode, const bool enabled) noexcept override; @@ -212,7 +210,6 @@ class Microsoft::Terminal::Core::Terminal final : void ClearPatternTree() noexcept; const std::optional GetTabColor() const noexcept; - til::color GetDefaultBackground() const noexcept; Microsoft::Console::Render::BlinkingState& GetBlinkingState() const noexcept; diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 9579d0dd906..7373ebbdf89 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -383,6 +383,11 @@ try { _colorTable.at(tableIndex) = color; + if (tableIndex == TextColor::DEFAULT_BACKGROUND) + { + _pfnBackgroundColorChanged(color); + } + // Repaint everything - the colors might have changed _buffer->GetRenderTarget().TriggerRedrawAll(); return true; @@ -442,46 +447,6 @@ bool Terminal::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle) noex return true; } -// Method Description: -// - Updates the default foreground color from a COLORREF, format 0x00BBGGRR. -// Arguments: -// - color: the new COLORREF to use as the default foreground color -// Return Value: -// - true -bool Terminal::SetDefaultForeground(const COLORREF color) noexcept -try -{ - _colorTable.at(TextColor::DEFAULT_FOREGROUND) = color; - - // Repaint everything - the colors might have changed - _buffer->GetRenderTarget().TriggerRedrawAll(); - return true; -} -CATCH_RETURN_FALSE() - -// Method Description: -// - Updates the default background color from a COLORREF, format 0x00BBGGRR. -// Arguments: -// - color: the new COLORREF to use as the default background color -// Return Value: -// - true -bool Terminal::SetDefaultBackground(const COLORREF color) noexcept -try -{ - _colorTable.at(TextColor::DEFAULT_BACKGROUND) = color; - _pfnBackgroundColorChanged(color); - - // Repaint everything - the colors might have changed - _buffer->GetRenderTarget().TriggerRedrawAll(); - return true; -} -CATCH_RETURN_FALSE() - -til::color Terminal::GetDefaultBackground() const noexcept -{ - return _colorTable.at(TextColor::DEFAULT_BACKGROUND); -} - bool Terminal::SetInputMode(const TerminalInput::Mode mode, const bool enabled) noexcept try { diff --git a/src/cascadia/TerminalCore/TerminalDispatch.cpp b/src/cascadia/TerminalCore/TerminalDispatch.cpp index 414291cb0a1..02f27e3d6e1 100644 --- a/src/cascadia/TerminalCore/TerminalDispatch.cpp +++ b/src/cascadia/TerminalCore/TerminalDispatch.cpp @@ -247,7 +247,7 @@ CATCH_LOG_RETURN_FALSE() bool TerminalDispatch::SetDefaultForeground(const DWORD color) noexcept try { - return _terminalApi.SetDefaultForeground(color); + return _terminalApi.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, color); } CATCH_LOG_RETURN_FALSE() @@ -260,7 +260,7 @@ CATCH_LOG_RETURN_FALSE() bool TerminalDispatch::SetDefaultBackground(const DWORD color) noexcept try { - return _terminalApi.SetDefaultBackground(color); + return _terminalApi.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, color); } CATCH_LOG_RETURN_FALSE() diff --git a/src/host/getset.cpp b/src/host/getset.cpp index ab70e570a0b..cb58a034bbf 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -1998,7 +1998,7 @@ void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo) // terminals as well is global, not per-screen-buffer. [[nodiscard]] HRESULT DoSrvPrivateSetColorTableEntry(const size_t index, const COLORREF value) noexcept { - RETURN_HR_IF(E_INVALIDARG, index >= 256); + RETURN_HR_IF(E_INVALIDARG, index >= TextColor::TABLE_SIZE); try { Globals& g = ServiceLocator::LocateGlobals(); @@ -2018,60 +2018,6 @@ void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo) CATCH_RETURN(); } -// Method Description: -// - Sets the default foreground color to the color specified in value. -// Arguments: -// - value: the new RGB value to use, as a COLORREF, format 0x00BBGGRR. -// Return Value: -// - S_OK -[[nodiscard]] HRESULT DoSrvPrivateSetDefaultForegroundColor(const COLORREF value) noexcept -{ - try - { - Globals& g = ServiceLocator::LocateGlobals(); - CONSOLE_INFORMATION& gci = g.getConsoleInformation(); - - gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, value); - - // Update the screen colors if we're not a pty - // No need to force a redraw in pty mode. - if (g.pRender && !gci.IsInVtIoMode()) - { - g.pRender->TriggerRedrawAll(); - } - - return S_OK; - } - CATCH_RETURN(); -} - -// Method Description: -// - Sets the default background color to the color specified in value. -// Arguments: -// - value: the new RGB value to use, as a COLORREF, format 0x00BBGGRR. -// Return Value: -// - S_OK -[[nodiscard]] HRESULT DoSrvPrivateSetDefaultBackgroundColor(const COLORREF value) noexcept -{ - try - { - Globals& g = ServiceLocator::LocateGlobals(); - CONSOLE_INFORMATION& gci = g.getConsoleInformation(); - - gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, value); - - // Update the screen colors if we're not a pty - // No need to force a redraw in pty mode. - if (g.pRender && !gci.IsInVtIoMode()) - { - g.pRender->TriggerRedrawAll(); - } - - return S_OK; - } - CATCH_RETURN(); -} - // Routine Description: // - A private API call for filling a region of the screen buffer. // Arguments: diff --git a/src/host/getset.h b/src/host/getset.h index 5a084d206ab..ac59eea0b2c 100644 --- a/src/host/getset.h +++ b/src/host/getset.h @@ -66,10 +66,6 @@ void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo); [[nodiscard]] HRESULT DoSrvPrivateGetColorTableEntry(const size_t index, COLORREF& value) noexcept; [[nodiscard]] HRESULT DoSrvPrivateSetColorTableEntry(const size_t index, const COLORREF value) noexcept; -[[nodiscard]] HRESULT DoSrvPrivateSetDefaultForegroundColor(const COLORREF value) noexcept; - -[[nodiscard]] HRESULT DoSrvPrivateSetDefaultBackgroundColor(const COLORREF value) noexcept; - [[nodiscard]] HRESULT DoSrvPrivateFillRegion(SCREEN_INFORMATION& screenInfo, const COORD startPosition, const size_t fillLength, diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index 8ef3f44b530..ec3f6d5a012 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -633,30 +633,6 @@ bool ConhostInternalGetSet::PrivateSetColorTableEntry(const size_t index, const return SUCCEEDED(DoSrvPrivateSetColorTableEntry(index, value)); } -// Method Description: -// - Connects the PrivateSetDefaultForeground call directly into our Driver Message servicing -// call inside Conhost.exe -// Arguments: -// - value: the new RGB value to use, as a COLORREF, format 0x00BBGGRR. -// Return Value: -// - true if successful (see DoSrvPrivateSetDefaultForegroundColor). false otherwise. -bool ConhostInternalGetSet::PrivateSetDefaultForeground(const COLORREF value) const noexcept -{ - return SUCCEEDED(DoSrvPrivateSetDefaultForegroundColor(value)); -} - -// Method Description: -// - Connects the PrivateSetDefaultBackground call directly into our Driver Message servicing -// call inside Conhost.exe -// Arguments: -// - value: the new RGB value to use, as a COLORREF, format 0x00BBGGRR. -// Return Value: -// - true if successful (see DoSrvPrivateSetDefaultBackgroundColor). false otherwise. -bool ConhostInternalGetSet::PrivateSetDefaultBackground(const COLORREF value) const noexcept -{ - return SUCCEEDED(DoSrvPrivateSetDefaultBackgroundColor(value)); -} - // Routine Description: // - Connects the PrivateFillRegion call directly into our Driver Message servicing // call inside Conhost.exe diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index d019e3efa3c..6d60c8df8e7 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -122,10 +122,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const noexcept override; bool PrivateSetColorTableEntry(const size_t index, const COLORREF value) const noexcept override; - bool PrivateSetDefaultForeground(const COLORREF value) const noexcept override; - - bool PrivateSetDefaultBackground(const COLORREF value) const noexcept override; - bool PrivateFillRegion(const COORD startPosition, const size_t fillLength, const wchar_t fillChar, diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index eef56cb8318..1eeb6c1a2b9 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -2293,10 +2293,10 @@ bool AdaptDispatch::SetColorTableEntry(const size_t tableIndex, const DWORD dwCo // - dwColor: The new RGB color value to use, as a COLORREF, format 0x00BBGGRR. // Return Value: // True if handled successfully. False otherwise. -bool Microsoft::Console::VirtualTerminal::AdaptDispatch::SetDefaultForeground(const DWORD dwColor) +bool AdaptDispatch::SetDefaultForeground(const DWORD dwColor) { bool success = true; - success = _pConApi->PrivateSetDefaultForeground(dwColor); + success = _pConApi->PrivateSetColorTableEntry(TextColor::DEFAULT_FOREGROUND, dwColor); // If we're a conpty, always return false, so that we send the updated color // value to the terminal. Still handle the sequence so apps that use @@ -2316,10 +2316,10 @@ bool Microsoft::Console::VirtualTerminal::AdaptDispatch::SetDefaultForeground(co // - dwColor: The new RGB color value to use, as a COLORREF, format 0x00BBGGRR. // Return Value: // True if handled successfully. False otherwise. -bool Microsoft::Console::VirtualTerminal::AdaptDispatch::SetDefaultBackground(const DWORD dwColor) +bool AdaptDispatch::SetDefaultBackground(const DWORD dwColor) { bool success = true; - success = _pConApi->PrivateSetDefaultBackground(dwColor); + success = _pConApi->PrivateSetColorTableEntry(TextColor::DEFAULT_BACKGROUND, dwColor); // If we're a conpty, always return false, so that we send the updated color // value to the terminal. Still handle the sequence so apps that use diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp index 833e84b17bb..ba1e9f6dc76 100644 --- a/src/terminal/adapter/conGetSet.hpp +++ b/src/terminal/adapter/conGetSet.hpp @@ -89,8 +89,6 @@ namespace Microsoft::Console::VirtualTerminal virtual bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const = 0; virtual bool PrivateSetColorTableEntry(const size_t index, const COLORREF value) const = 0; - virtual bool PrivateSetDefaultForeground(const COLORREF value) const = 0; - virtual bool PrivateSetDefaultBackground(const COLORREF value) const = 0; virtual bool PrivateFillRegion(const COORD startPosition, const size_t fillLength, diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index 98a1ab2c033..d0dfde215db 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -465,28 +465,6 @@ class TestGetSet final : public ConGetSet return _privateSetColorTableEntryResult; } - bool PrivateSetDefaultForeground(const COLORREF value) const noexcept override - { - Log::Comment(L"PrivateSetDefaultForeground MOCK called..."); - if (_privateSetDefaultForegroundResult) - { - VERIFY_ARE_EQUAL(_expectedDefaultForegroundColorValue, value); - } - - return _privateSetDefaultForegroundResult; - } - - bool PrivateSetDefaultBackground(const COLORREF value) const noexcept override - { - Log::Comment(L"PrivateSetDefaultForeground MOCK called..."); - if (_privateSetDefaultBackgroundResult) - { - VERIFY_ARE_EQUAL(_expectedDefaultBackgroundColorValue, value); - } - - return _privateSetDefaultBackgroundResult; - } - bool PrivateFillRegion(const COORD /*startPosition*/, const size_t /*fillLength*/, const wchar_t /*fillChar*/, @@ -750,12 +728,6 @@ class TestGetSet final : public ConGetSet size_t _expectedColorTableIndex = SIZE_MAX; COLORREF _expectedColorValue = INVALID_COLOR; - bool _privateSetDefaultForegroundResult = false; - COLORREF _expectedDefaultForegroundColorValue = INVALID_COLOR; - - bool _privateSetDefaultBackgroundResult = false; - COLORREF _expectedDefaultBackgroundColorValue = INVALID_COLOR; - SIZE _expectedCellSize = {}; private: From a8bad338a157ecf6a2f5e39d5cc4ec84d4ae42de Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 7 Nov 2021 10:58:11 +0000 Subject: [PATCH 3/8] Standardize color table APIs in ConGetSet and ITerminalApi. --- src/cascadia/TerminalCore/ITerminalApi.hpp | 3 +- src/cascadia/TerminalCore/Terminal.hpp | 1 + src/cascadia/TerminalCore/TerminalApi.cpp | 16 ++++++ src/host/getset.cpp | 55 ------------------- src/host/getset.h | 3 - src/host/outputStream.cpp | 48 +++++++++++----- src/host/outputStream.hpp | 4 +- src/terminal/adapter/adaptDispatch.cpp | 6 +- src/terminal/adapter/conGetSet.hpp | 4 +- .../adapter/ut_adapter/adapterTest.cpp | 36 ++++++------ 10 files changed, 78 insertions(+), 98 deletions(-) diff --git a/src/cascadia/TerminalCore/ITerminalApi.hpp b/src/cascadia/TerminalCore/ITerminalApi.hpp index 40e7c879b2b..5ac77984de3 100644 --- a/src/cascadia/TerminalCore/ITerminalApi.hpp +++ b/src/cascadia/TerminalCore/ITerminalApi.hpp @@ -40,7 +40,8 @@ namespace Microsoft::Terminal::Core virtual bool WarningBell() noexcept = 0; virtual bool SetWindowTitle(std::wstring_view title) noexcept = 0; - virtual bool SetColorTableEntry(const size_t tableIndex, const DWORD color) noexcept = 0; + virtual COLORREF GetColorTableEntry(const size_t tableIndex) const noexcept = 0; + virtual bool SetColorTableEntry(const size_t tableIndex, const COLORREF color) noexcept = 0; virtual bool SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle) noexcept = 0; virtual bool SetCursorColor(const DWORD color) noexcept = 0; diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 15568951033..b214f288fdc 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -106,6 +106,7 @@ class Microsoft::Terminal::Core::Terminal final : bool EraseInDisplay(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) noexcept override; bool WarningBell() noexcept override; bool SetWindowTitle(std::wstring_view title) noexcept override; + COLORREF GetColorTableEntry(const size_t tableIndex) const noexcept override; bool SetColorTableEntry(const size_t tableIndex, const COLORREF color) noexcept override; bool SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle) noexcept override; bool SetCursorColor(const COLORREF color) noexcept override; diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 7373ebbdf89..d6ed7fde91f 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -370,6 +370,22 @@ try } CATCH_RETURN_FALSE() +// Method Description: +// - Retrieves the value in the colortable at the specified index. +// Arguments: +// - tableIndex: the index of the color table to retrieve. +// Return Value: +// - the COLORREF value for the color at that index in the table. +COLORREF Terminal::GetColorTableEntry(const size_t tableIndex) const noexcept +try +{ + return _colorTable.at(tableIndex); +} +catch (...) +{ + return INVALID_COLOR; +} + // Method Description: // - Updates the value in the colortable at index tableIndex to the new color // color. color is a COLORREF, format 0x00BBGGRR. diff --git a/src/host/getset.cpp b/src/host/getset.cpp index cb58a034bbf..5b32c90fb47 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -1963,61 +1963,6 @@ void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo) screenInfo.GetActiveBuffer().MoveToBottom(); } -// Method Description: -// - Retrieve the color table value at the specified index. -// Arguments: -// - index: the index in the table to retrieve. -// - value: receives the RGB value for the color at that index in the table. -// Return Value: -// - E_INVALIDARG if index is >= 256, else S_OK -[[nodiscard]] HRESULT DoSrvPrivateGetColorTableEntry(const size_t index, COLORREF& value) noexcept -{ - RETURN_HR_IF(E_INVALIDARG, index >= 256); - try - { - Globals& g = ServiceLocator::LocateGlobals(); - CONSOLE_INFORMATION& gci = g.getConsoleInformation(); - - value = gci.GetColorTableEntry(index); - - return S_OK; - } - CATCH_RETURN(); -} - -// Method Description: -// - Sets the color table value in index to the color specified in value. -// Can be used to set the 256-color table as well as the 16-color table. -// Arguments: -// - index: the index in the table to change. -// - value: the new RGB value to use for that index in the color table. -// Return Value: -// - E_INVALIDARG if index is >= 256, else S_OK -// Notes: -// Does not take a buffer parameter. The color table for a console and for -// terminals as well is global, not per-screen-buffer. -[[nodiscard]] HRESULT DoSrvPrivateSetColorTableEntry(const size_t index, const COLORREF value) noexcept -{ - RETURN_HR_IF(E_INVALIDARG, index >= TextColor::TABLE_SIZE); - try - { - Globals& g = ServiceLocator::LocateGlobals(); - CONSOLE_INFORMATION& gci = g.getConsoleInformation(); - - gci.SetColorTableEntry(index, value); - - // Update the screen colors if we're not a pty - // No need to force a redraw in pty mode. - if (g.pRender && !gci.IsInVtIoMode()) - { - g.pRender->TriggerRedrawAll(); - } - - return S_OK; - } - CATCH_RETURN(); -} - // Routine Description: // - A private API call for filling a region of the screen buffer. // Arguments: diff --git a/src/host/getset.h b/src/host/getset.h index ac59eea0b2c..cff83e9223c 100644 --- a/src/host/getset.h +++ b/src/host/getset.h @@ -63,9 +63,6 @@ void DoSrvPrivateInsertLines(const size_t count); void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo); -[[nodiscard]] HRESULT DoSrvPrivateGetColorTableEntry(const size_t index, COLORREF& value) noexcept; -[[nodiscard]] HRESULT DoSrvPrivateSetColorTableEntry(const size_t index, const COLORREF value) noexcept; - [[nodiscard]] HRESULT DoSrvPrivateFillRegion(SCREEN_INFORMATION& screenInfo, const COORD startPosition, const size_t fillLength, diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index ec3f6d5a012..ce1df8386cb 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -608,30 +608,50 @@ bool ConhostInternalGetSet::MoveToBottom() const } // Method Description: -// - Connects the PrivateGetColorTableEntry call directly into our Driver Message servicing -// call inside Conhost.exe +// - Retrieves the value in the colortable at the specified index. // Arguments: -// - index: the index in the table to retrieve. -// - value: receives the RGB value for the color at that index in the table. +// - tableIndex: the index of the color table to retrieve. // Return Value: -// - true if successful (see DoSrvPrivateGetColorTableEntry). false otherwise. -bool ConhostInternalGetSet::PrivateGetColorTableEntry(const size_t index, COLORREF& value) const noexcept +// - the COLORREF value for the color at that index in the table. +COLORREF ConhostInternalGetSet::GetColorTableEntry(const size_t tableIndex) const noexcept +try +{ + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + + return gci.GetColorTableEntry(tableIndex); +} +catch (...) { - return SUCCEEDED(DoSrvPrivateGetColorTableEntry(index, value)); + return INVALID_COLOR; } // Method Description: -// - Connects the PrivateSetColorTableEntry call directly into our Driver Message servicing -// call inside Conhost.exe +// - Updates the value in the colortable at index tableIndex to the new color +// color. color is a COLORREF, format 0x00BBGGRR. // Arguments: -// - index: the index in the table to change. -// - value: the new RGB value to use for that index in the color table. +// - tableIndex: the index of the color table to update. +// - color: the new COLORREF to use as that color table value. // Return Value: -// - true if successful (see DoSrvPrivateSetColorTableEntry). false otherwise. -bool ConhostInternalGetSet::PrivateSetColorTableEntry(const size_t index, const COLORREF value) const noexcept +// - true if successful. false otherwise. +bool ConhostInternalGetSet::SetColorTableEntry(const size_t tableIndex, const COLORREF color) noexcept +try { - return SUCCEEDED(DoSrvPrivateSetColorTableEntry(index, value)); + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + + gci.SetColorTableEntry(tableIndex, color); + + // Update the screen colors if we're not a pty + // No need to force a redraw in pty mode. + if (g.pRender && !gci.IsInVtIoMode()) + { + g.pRender->TriggerRedrawAll(); + } + + return true; } +CATCH_RETURN_FALSE() // Routine Description: // - Connects the PrivateFillRegion call directly into our Driver Message servicing diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index 6d60c8df8e7..7c7f0a6c7b3 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -119,8 +119,8 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: bool MoveToBottom() const override; - bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const noexcept override; - bool PrivateSetColorTableEntry(const size_t index, const COLORREF value) const noexcept override; + COLORREF GetColorTableEntry(const size_t tableIndex) const noexcept override; + bool SetColorTableEntry(const size_t tableIndex, const COLORREF color) noexcept override; bool PrivateFillRegion(const COORD startPosition, const size_t fillLength, diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 1eeb6c1a2b9..dc7b161f990 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -2273,7 +2273,7 @@ bool AdaptDispatch::SetClipboard(const std::wstring_view /*content*/) noexcept // True if handled successfully. False otherwise. bool AdaptDispatch::SetColorTableEntry(const size_t tableIndex, const DWORD dwColor) { - const bool success = _pConApi->PrivateSetColorTableEntry(tableIndex, dwColor); + const bool success = _pConApi->SetColorTableEntry(tableIndex, dwColor); // If we're a conpty, always return false, so that we send the updated color // value to the terminal. Still handle the sequence so apps that use @@ -2296,7 +2296,7 @@ bool AdaptDispatch::SetColorTableEntry(const size_t tableIndex, const DWORD dwCo bool AdaptDispatch::SetDefaultForeground(const DWORD dwColor) { bool success = true; - success = _pConApi->PrivateSetColorTableEntry(TextColor::DEFAULT_FOREGROUND, dwColor); + success = _pConApi->SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, dwColor); // If we're a conpty, always return false, so that we send the updated color // value to the terminal. Still handle the sequence so apps that use @@ -2319,7 +2319,7 @@ bool AdaptDispatch::SetDefaultForeground(const DWORD dwColor) bool AdaptDispatch::SetDefaultBackground(const DWORD dwColor) { bool success = true; - success = _pConApi->PrivateSetColorTableEntry(TextColor::DEFAULT_BACKGROUND, dwColor); + success = _pConApi->SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, dwColor); // If we're a conpty, always return false, so that we send the updated color // value to the terminal. Still handle the sequence so apps that use diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp index ba1e9f6dc76..59dd4c11aa1 100644 --- a/src/terminal/adapter/conGetSet.hpp +++ b/src/terminal/adapter/conGetSet.hpp @@ -87,8 +87,8 @@ namespace Microsoft::Console::VirtualTerminal virtual bool MoveToBottom() const = 0; - virtual bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const = 0; - virtual bool PrivateSetColorTableEntry(const size_t index, const COLORREF value) const = 0; + virtual COLORREF GetColorTableEntry(const size_t tableIndex) const = 0; + virtual bool SetColorTableEntry(const size_t tableIndex, const COLORREF color) = 0; virtual bool PrivateFillRegion(const COORD startPosition, const size_t fillLength, diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index d0dfde215db..1bf7a31c25d 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -438,31 +438,31 @@ class TestGetSet final : public ConGetSet return _moveToBottomResult; } - bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const noexcept override + COLORREF GetColorTableEntry(const size_t tableIndex) const noexcept override { - Log::Comment(L"PrivateGetColorTableEntry MOCK called..."); + Log::Comment(L"GetColorTableEntry MOCK called..."); - if (_privateGetColorTableEntryResult) + if (_getColorTableEntryResult) { - VERIFY_ARE_EQUAL(_expectedColorTableIndex, index); + VERIFY_ARE_EQUAL(_expectedColorTableIndex, tableIndex); // Simply returning the index as the color value makes it easy for // tests to confirm that they've received the color they expected. - value = gsl::narrow_cast(index); + return gsl::narrow_cast(tableIndex); } - return _privateGetColorTableEntryResult; + return INVALID_COLOR; } - bool PrivateSetColorTableEntry(const size_t index, const COLORREF value) const noexcept override + bool SetColorTableEntry(const size_t tableIndex, const COLORREF color) noexcept override { - Log::Comment(L"PrivateSetColorTableEntry MOCK called..."); - if (_privateSetColorTableEntryResult) + Log::Comment(L"SetColorTableEntry MOCK called..."); + if (_setColorTableEntryResult) { - VERIFY_ARE_EQUAL(_expectedColorTableIndex, index); - VERIFY_ARE_EQUAL(_expectedColorValue, value); + VERIFY_ARE_EQUAL(_expectedColorTableIndex, tableIndex); + VERIFY_ARE_EQUAL(_expectedColorValue, color); } - return _privateSetColorTableEntryResult; + return _setColorTableEntryResult; } bool PrivateFillRegion(const COORD /*startPosition*/, @@ -723,8 +723,8 @@ class TestGetSet final : public ConGetSet bool _getConsoleOutputCPResult = false; bool _moveToBottomResult = false; - bool _privateGetColorTableEntryResult = false; - bool _privateSetColorTableEntryResult = false; + bool _getColorTableEntryResult = false; + bool _setColorTableEntryResult = false; size_t _expectedColorTableIndex = SIZE_MAX; COLORREF _expectedColorValue = INVALID_COLOR; @@ -2278,7 +2278,7 @@ class AdapterTest VTParameter rgOptions[16]; size_t cOptions = 3; - _testGetSet->_privateGetColorTableEntryResult = true; + _testGetSet->_getColorTableEntryResult = true; _testGetSet->_expectedAttribute = _testGetSet->_attribute; Log::Comment(L"Test 1: Change Foreground"); @@ -2328,7 +2328,7 @@ class AdapterTest VTParameter rgOptions[16]; - _testGetSet->_privateGetColorTableEntryResult = true; + _testGetSet->_getColorTableEntryResult = true; _testGetSet->_expectedAttribute = _testGetSet->_attribute; Log::Comment(L"Test 1: Change Indexed Foreground with missing index parameter"); @@ -2371,7 +2371,7 @@ class AdapterTest { _testGetSet->PrepData(); - _testGetSet->_privateSetColorTableEntryResult = true; + _testGetSet->_setColorTableEntryResult = true; const auto testColor = RGB(1, 2, 3); _testGetSet->_expectedColorValue = testColor; @@ -2381,7 +2381,7 @@ class AdapterTest VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(i, testColor)); } - // Test in pty mode - we should fail, but PrivateSetColorTableEntry should still be called + // Test in pty mode - we should fail, but SetColorTableEntry should still be called _testGetSet->_isPty = true; _testGetSet->_expectedColorTableIndex = 15; // Windows BRIGHT_WHITE From 96dd85fd7274eaf53dc5c9011d28039881fd05e5 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 7 Nov 2021 15:19:43 +0000 Subject: [PATCH 4/8] Use index values when referring to default colors. --- src/buffer/out/TextAttribute.cpp | 12 +-- src/buffer/out/TextAttribute.hpp | 4 +- src/buffer/out/TextColor.cpp | 6 +- src/buffer/out/TextColor.h | 2 +- .../out/ut_textbuffer/TextAttributeTests.cpp | 96 ++++++++++--------- .../out/ut_textbuffer/TextColorTests.cpp | 72 +++++++------- .../TerminalCore/terminalrenderdata.cpp | 8 +- src/host/consoleInformation.cpp | 40 ++++---- src/host/renderData.cpp | 6 +- src/host/renderData.hpp | 4 +- src/host/server.h | 6 +- src/interactivity/win32/Clipboard.cpp | 11 ++- 12 files changed, 139 insertions(+), 128 deletions(-) diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index d8c86325e2f..cefc7b6bc0a 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -128,22 +128,22 @@ bool TextAttribute::IsLegacy() const noexcept // - Calculates rgb colors based off of current color table and active modification attributes. // Arguments: // - colorTable: the current color table rgb values. -// - defaultFgColor: the default foreground color rgb value. -// - defaultBgColor: the default background color rgb value. +// - defaultFgIndex: the color table index of the default foreground color. +// - defaultBgIndex: the color table index of the default background color. // - reverseScreenMode: true if the screen mode is reversed. // - blinkingIsFaint: true if blinking should be interpreted as faint. (defaults to false) // - boldIsBright: true if "bold" should be interpreted as bright. (defaults to true) // Return Value: // - the foreground and background colors that should be displayed. std::pair TextAttribute::CalculateRgbColors(const std::array& colorTable, - const COLORREF defaultFgColor, - const COLORREF defaultBgColor, + const size_t defaultFgIndex, + const size_t defaultBgIndex, const bool reverseScreenMode, const bool blinkingIsFaint, const bool boldIsBright) const noexcept { - auto fg = _foreground.GetColor(colorTable, defaultFgColor, boldIsBright && IsBold()); - auto bg = _background.GetColor(colorTable, defaultBgColor); + auto fg = _foreground.GetColor(colorTable, defaultFgIndex, boldIsBright && IsBold()); + auto bg = _background.GetColor(colorTable, defaultBgIndex); if (IsFaint() || (IsBlinking() && blinkingIsFaint)) { fg = (fg >> 1) & 0x7F7F7F; // Divide foreground color components by two. diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index 3be8d8c5a64..54e143d1461 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -65,8 +65,8 @@ class TextAttribute final WORD GetLegacyAttributes() const noexcept; std::pair CalculateRgbColors(const std::array& colorTable, - const COLORREF defaultFgColor, - const COLORREF defaultBgColor, + const size_t defaultFgIndex, + const size_t defaultBgIndex, const bool reverseScreenMode = false, const bool blinkingIsFaint = false, const bool boldIsBright = true) const noexcept; diff --git a/src/buffer/out/TextColor.cpp b/src/buffer/out/TextColor.cpp index a416e761d89..e20195ceaac 100644 --- a/src/buffer/out/TextColor.cpp +++ b/src/buffer/out/TextColor.cpp @@ -139,14 +139,16 @@ void TextColor::SetDefault() noexcept // Arguments: // - colorTable: The table of colors we should use to look up the value of // an indexed attribute from. -// - defaultColor: The color value to use if we're a default attribute. +// - defaultIndex: The color table index to use if we're a default attribute. // - 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(const std::array& colorTable, const COLORREF defaultColor, bool brighten) const noexcept +COLORREF TextColor::GetColor(const std::array& colorTable, const size_t defaultIndex, bool brighten) const noexcept { if (IsDefault()) { + const auto defaultColor = til::at(colorTable, defaultIndex); + if (brighten) { // See MSFT:20266024 for context on this fix. diff --git a/src/buffer/out/TextColor.h b/src/buffer/out/TextColor.h index 8b782c59833..fa8ef5da7f9 100644 --- a/src/buffer/out/TextColor.h +++ b/src/buffer/out/TextColor.h @@ -107,7 +107,7 @@ struct TextColor void SetIndex(const BYTE index, const bool isIndex256) noexcept; void SetDefault() noexcept; - COLORREF GetColor(const std::array& colorTable, const COLORREF defaultColor, bool brighten = false) const noexcept; + COLORREF GetColor(const std::array& colorTable, const size_t defaultIndex, bool brighten = false) const noexcept; BYTE GetLegacyIndex(const BYTE defaultIndex) const noexcept; constexpr BYTE GetIndex() const noexcept diff --git a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp index 952b94056a4..e5153618e64 100644 --- a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp +++ b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp @@ -25,8 +25,10 @@ class TextAttributeTests TEST_METHOD(TestBoldAsBright); std::array _colorTable; - COLORREF _defaultFg = RGB(1, 2, 3); - COLORREF _defaultBg = RGB(4, 5, 6); + const COLORREF _defaultFg = RGB(1, 2, 3); + const COLORREF _defaultBg = RGB(4, 5, 6); + const size_t _defaultFgIndex = TextColor::DEFAULT_FOREGROUND; + const size_t _defaultBgIndex = TextColor::DEFAULT_BACKGROUND; }; bool TextAttributeTests::ClassSetup() @@ -47,6 +49,8 @@ bool TextAttributeTests::ClassSetup() _colorTable[13] = RGB(180, 0, 158); // Bright Magenta _colorTable[14] = RGB(249, 241, 165); // Bright Yellow _colorTable[15] = RGB(242, 242, 242); // White + _colorTable[_defaultFgIndex] = _defaultFg; + _colorTable[_defaultBgIndex] = _defaultBg; return true; } @@ -132,17 +136,17 @@ void TextAttributeTests::TestTextAttributeColorGetters() // values when reverse video is not set VERIFY_IS_FALSE(attr.IsReverseVideo()); - VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(red, green), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(red, green), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex)); // with reverse video set, calculated foreground/background values should be // switched while getters stay the same attr.SetReverseVideo(true); - VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(green, red), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(green, red), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex)); // reset the reverse video attr.SetReverseVideo(false); @@ -151,17 +155,17 @@ void TextAttributeTests::TestTextAttributeColorGetters() // while the background and getters stay the same attr.SetFaint(true); - VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(faintRed, green), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(faintRed, green), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex)); // with reverse video set, calculated foreground/background values should be // switched, and the background fainter, while getters stay the same attr.SetReverseVideo(true); - VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(green, faintRed), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(green, faintRed), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex)); // reset the reverse video and faint attributes attr.SetReverseVideo(false); @@ -171,17 +175,17 @@ void TextAttributeTests::TestTextAttributeColorGetters() // background, while getters stay the same attr.SetInvisible(true); - VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(green, green), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(green, green), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex)); // with reverse video set, the calculated background value should match // the foreground, while getters stay the same attr.SetReverseVideo(true); - VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(red, red), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(red, red), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex)); } void TextAttributeTests::TestReverseDefaultColors() @@ -194,34 +198,34 @@ void TextAttributeTests::TestReverseDefaultColors() // values when reverse video is not set VERIFY_IS_FALSE(attr.IsReverseVideo()); - VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex)); // with reverse video set, calculated foreground/background values should be // switched while getters stay the same attr.SetReverseVideo(true); VERIFY_IS_TRUE(attr.IsReverseVideo()); - VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(_defaultBg, _defaultFg), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultBg, _defaultFg), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex)); attr.SetForeground(red); VERIFY_IS_TRUE(attr.IsReverseVideo()); - VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(_defaultBg, red), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultBg, red), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex)); attr.Invert(); VERIFY_IS_FALSE(attr.IsReverseVideo()); attr.SetDefaultForeground(); attr.SetBackground(green); - VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, green), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, green), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex)); } void TextAttributeTests::TestRoundtripDefaultColors() @@ -277,43 +281,43 @@ void TextAttributeTests::TestBoldAsBright() // values when not bold VERIFY_IS_FALSE(attr.IsBold()); - VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(_colorTable, _defaultFg)); - VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(_colorTable, _defaultBg)); - VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, true)); - VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, false)); + VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(_colorTable, _defaultFgIndex)); + VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(_colorTable, _defaultBgIndex)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, true)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, false)); // with bold set, calculated foreground/background values shouldn't change for the default colors. attr.SetBold(true); VERIFY_IS_TRUE(attr.IsBold()); - VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, true)); - VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, false)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, true)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, false)); attr.SetIndexedForeground(TextColor::DARK_BLACK); VERIFY_IS_TRUE(attr.IsBold()); Log::Comment(L"Foreground should be bright black when bold is bright is enabled"); - VERIFY_ARE_EQUAL(std::make_pair(brightBlack, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, true)); + VERIFY_ARE_EQUAL(std::make_pair(brightBlack, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, true)); Log::Comment(L"Foreground should be dark black when bold is bright is disabled"); - VERIFY_ARE_EQUAL(std::make_pair(darkBlack, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, false)); + VERIFY_ARE_EQUAL(std::make_pair(darkBlack, _defaultBg), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, false)); attr.SetIndexedBackground(TextColor::DARK_GREEN); VERIFY_IS_TRUE(attr.IsBold()); Log::Comment(L"background should be unaffected by 'bold is bright'"); - VERIFY_ARE_EQUAL(std::make_pair(brightBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, true)); - VERIFY_ARE_EQUAL(std::make_pair(darkBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, false)); + VERIFY_ARE_EQUAL(std::make_pair(brightBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, true)); + VERIFY_ARE_EQUAL(std::make_pair(darkBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, false)); attr.SetBold(false); VERIFY_IS_FALSE(attr.IsBold()); Log::Comment(L"when not bold, 'bold is bright' changes nothing"); - VERIFY_ARE_EQUAL(std::make_pair(darkBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, true)); - VERIFY_ARE_EQUAL(std::make_pair(darkBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, false)); + VERIFY_ARE_EQUAL(std::make_pair(darkBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, true)); + VERIFY_ARE_EQUAL(std::make_pair(darkBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, false)); Log::Comment(L"When set to a bright color, and bold, 'bold is bright' changes nothing"); attr.SetBold(true); attr.SetIndexedForeground(TextColor::BRIGHT_BLACK); VERIFY_IS_TRUE(attr.IsBold()); - VERIFY_ARE_EQUAL(std::make_pair(brightBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, true)); - VERIFY_ARE_EQUAL(std::make_pair(brightBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFg, _defaultBg, false, false, false)); + VERIFY_ARE_EQUAL(std::make_pair(brightBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, true)); + VERIFY_ARE_EQUAL(std::make_pair(brightBlack, darkGreen), attr.CalculateRgbColors(_colorTable, _defaultFgIndex, _defaultBgIndex, false, false, false)); } diff --git a/src/buffer/out/ut_textbuffer/TextColorTests.cpp b/src/buffer/out/ut_textbuffer/TextColorTests.cpp index 78f42341a64..829daf9d2f2 100644 --- a/src/buffer/out/ut_textbuffer/TextColorTests.cpp +++ b/src/buffer/out/ut_textbuffer/TextColorTests.cpp @@ -24,8 +24,10 @@ class TextColorTests TEST_METHOD(TestChangeColor); std::array _colorTable; - COLORREF _defaultFg = RGB(1, 2, 3); - COLORREF _defaultBg = RGB(4, 5, 6); + const COLORREF _defaultFg = RGB(1, 2, 3); + const COLORREF _defaultBg = RGB(4, 5, 6); + const size_t _defaultFgIndex = TextColor::DEFAULT_FOREGROUND; + const size_t _defaultBgIndex = TextColor::DEFAULT_BACKGROUND; }; bool TextColorTests::ClassSetup() @@ -46,6 +48,8 @@ bool TextColorTests::ClassSetup() _colorTable[13] = RGB(180, 0, 158); // Bright Magenta _colorTable[14] = RGB(249, 241, 165); // Bright Yellow _colorTable[15] = RGB(242, 242, 242); // White + _colorTable[_defaultFgIndex] = _defaultFg; + _colorTable[_defaultBgIndex] = _defaultBg; return true; } @@ -57,16 +61,16 @@ void TextColorTests::TestDefaultColor() VERIFY_IS_FALSE(defaultColor.IsLegacy()); VERIFY_IS_FALSE(defaultColor.IsRgb()); - auto color = defaultColor.GetColor(_colorTable, _defaultFg, false); + auto color = defaultColor.GetColor(_colorTable, _defaultFgIndex, false); VERIFY_ARE_EQUAL(_defaultFg, color); - color = defaultColor.GetColor(_colorTable, _defaultFg, true); + color = defaultColor.GetColor(_colorTable, _defaultFgIndex, true); VERIFY_ARE_EQUAL(_defaultFg, color); - color = defaultColor.GetColor(_colorTable, _defaultBg, false); + color = defaultColor.GetColor(_colorTable, _defaultBgIndex, false); VERIFY_ARE_EQUAL(_defaultBg, color); - color = defaultColor.GetColor(_colorTable, _defaultBg, true); + color = defaultColor.GetColor(_colorTable, _defaultBgIndex, true); VERIFY_ARE_EQUAL(_defaultBg, color); } @@ -78,16 +82,16 @@ void TextColorTests::TestDarkIndexColor() VERIFY_IS_TRUE(indexColor.IsLegacy()); VERIFY_IS_FALSE(indexColor.IsRgb()); - auto color = indexColor.GetColor(_colorTable, _defaultFg, false); + auto color = indexColor.GetColor(_colorTable, _defaultFgIndex, false); VERIFY_ARE_EQUAL(_colorTable[7], color); - color = indexColor.GetColor(_colorTable, _defaultFg, true); + color = indexColor.GetColor(_colorTable, _defaultFgIndex, true); VERIFY_ARE_EQUAL(_colorTable[15], color); - color = indexColor.GetColor(_colorTable, _defaultBg, false); + color = indexColor.GetColor(_colorTable, _defaultBgIndex, false); VERIFY_ARE_EQUAL(_colorTable[7], color); - color = indexColor.GetColor(_colorTable, _defaultBg, true); + color = indexColor.GetColor(_colorTable, _defaultBgIndex, true); VERIFY_ARE_EQUAL(_colorTable[15], color); } @@ -99,16 +103,16 @@ void TextColorTests::TestBrightIndexColor() VERIFY_IS_TRUE(indexColor.IsLegacy()); VERIFY_IS_FALSE(indexColor.IsRgb()); - auto color = indexColor.GetColor(_colorTable, _defaultFg, false); + auto color = indexColor.GetColor(_colorTable, _defaultFgIndex, false); VERIFY_ARE_EQUAL(_colorTable[15], color); - color = indexColor.GetColor(_colorTable, _defaultFg, true); + color = indexColor.GetColor(_colorTable, _defaultFgIndex, true); VERIFY_ARE_EQUAL(_colorTable[15], color); - color = indexColor.GetColor(_colorTable, _defaultBg, false); + color = indexColor.GetColor(_colorTable, _defaultBgIndex, false); VERIFY_ARE_EQUAL(_colorTable[15], color); - color = indexColor.GetColor(_colorTable, _defaultBg, true); + color = indexColor.GetColor(_colorTable, _defaultBgIndex, true); VERIFY_ARE_EQUAL(_colorTable[15], color); } @@ -121,16 +125,16 @@ void TextColorTests::TestRgbColor() VERIFY_IS_FALSE(rgbColor.IsLegacy()); VERIFY_IS_TRUE(rgbColor.IsRgb()); - auto color = rgbColor.GetColor(_colorTable, _defaultFg, false); + auto color = rgbColor.GetColor(_colorTable, _defaultFgIndex, false); VERIFY_ARE_EQUAL(myColor, color); - color = rgbColor.GetColor(_colorTable, _defaultFg, true); + color = rgbColor.GetColor(_colorTable, _defaultFgIndex, true); VERIFY_ARE_EQUAL(myColor, color); - color = rgbColor.GetColor(_colorTable, _defaultBg, false); + color = rgbColor.GetColor(_colorTable, _defaultBgIndex, false); VERIFY_ARE_EQUAL(myColor, color); - color = rgbColor.GetColor(_colorTable, _defaultBg, true); + color = rgbColor.GetColor(_colorTable, _defaultBgIndex, true); VERIFY_ARE_EQUAL(myColor, color); } @@ -143,55 +147,55 @@ void TextColorTests::TestChangeColor() VERIFY_IS_FALSE(rgbColor.IsLegacy()); VERIFY_IS_TRUE(rgbColor.IsRgb()); - auto color = rgbColor.GetColor(_colorTable, _defaultFg, false); + auto color = rgbColor.GetColor(_colorTable, _defaultFgIndex, false); VERIFY_ARE_EQUAL(myColor, color); - color = rgbColor.GetColor(_colorTable, _defaultFg, true); + color = rgbColor.GetColor(_colorTable, _defaultFgIndex, true); VERIFY_ARE_EQUAL(myColor, color); - color = rgbColor.GetColor(_colorTable, _defaultBg, false); + color = rgbColor.GetColor(_colorTable, _defaultBgIndex, false); VERIFY_ARE_EQUAL(myColor, color); - color = rgbColor.GetColor(_colorTable, _defaultBg, true); + color = rgbColor.GetColor(_colorTable, _defaultBgIndex, true); VERIFY_ARE_EQUAL(myColor, color); rgbColor.SetDefault(); - color = rgbColor.GetColor(_colorTable, _defaultFg, false); + color = rgbColor.GetColor(_colorTable, _defaultFgIndex, false); VERIFY_ARE_EQUAL(_defaultFg, color); - color = rgbColor.GetColor(_colorTable, _defaultFg, true); + color = rgbColor.GetColor(_colorTable, _defaultFgIndex, true); VERIFY_ARE_EQUAL(_defaultFg, color); - color = rgbColor.GetColor(_colorTable, _defaultBg, false); + color = rgbColor.GetColor(_colorTable, _defaultBgIndex, false); VERIFY_ARE_EQUAL(_defaultBg, color); - color = rgbColor.GetColor(_colorTable, _defaultBg, true); + color = rgbColor.GetColor(_colorTable, _defaultBgIndex, true); VERIFY_ARE_EQUAL(_defaultBg, color); rgbColor.SetIndex(7, false); - color = rgbColor.GetColor(_colorTable, _defaultFg, false); + color = rgbColor.GetColor(_colorTable, _defaultFgIndex, false); VERIFY_ARE_EQUAL(_colorTable[7], color); - color = rgbColor.GetColor(_colorTable, _defaultFg, true); + color = rgbColor.GetColor(_colorTable, _defaultFgIndex, true); VERIFY_ARE_EQUAL(_colorTable[15], color); - color = rgbColor.GetColor(_colorTable, _defaultBg, false); + color = rgbColor.GetColor(_colorTable, _defaultBgIndex, false); VERIFY_ARE_EQUAL(_colorTable[7], color); - color = rgbColor.GetColor(_colorTable, _defaultBg, true); + color = rgbColor.GetColor(_colorTable, _defaultBgIndex, true); VERIFY_ARE_EQUAL(_colorTable[15], color); rgbColor.SetIndex(15, false); - color = rgbColor.GetColor(_colorTable, _defaultFg, false); + color = rgbColor.GetColor(_colorTable, _defaultFgIndex, false); VERIFY_ARE_EQUAL(_colorTable[15], color); - color = rgbColor.GetColor(_colorTable, _defaultFg, true); + color = rgbColor.GetColor(_colorTable, _defaultFgIndex, true); VERIFY_ARE_EQUAL(_colorTable[15], color); - color = rgbColor.GetColor(_colorTable, _defaultBg, false); + color = rgbColor.GetColor(_colorTable, _defaultBgIndex, false); VERIFY_ARE_EQUAL(_colorTable[15], color); - color = rgbColor.GetColor(_colorTable, _defaultBg, true); + color = rgbColor.GetColor(_colorTable, _defaultBgIndex, true); VERIFY_ARE_EQUAL(_colorTable[15], color); } diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp index 53f5b89eba0..a5a5d40e493 100644 --- a/src/cascadia/TerminalCore/terminalrenderdata.cpp +++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp @@ -73,19 +73,19 @@ std::pair Terminal::GetAttributeColors(const TextAttribute& if (attr.IsReverseVideo() ^ _screenReversed) { colors.first = _adjustedForegroundColors[fgIndex][bgIndex]; - colors.second = fgTextColor.GetColor(_colorTable, _colorTable.at(TextColor::DEFAULT_FOREGROUND)); + colors.second = fgTextColor.GetColor(_colorTable, TextColor::DEFAULT_FOREGROUND); } else { colors.first = _adjustedForegroundColors[bgIndex][fgIndex]; - colors.second = bgTextColor.GetColor(_colorTable, _colorTable.at(TextColor::DEFAULT_BACKGROUND)); + colors.second = bgTextColor.GetColor(_colorTable, TextColor::DEFAULT_BACKGROUND); } } else { colors = attr.CalculateRgbColors(_colorTable, - _colorTable.at(TextColor::DEFAULT_FOREGROUND), - _colorTable.at(TextColor::DEFAULT_BACKGROUND), + TextColor::DEFAULT_FOREGROUND, + TextColor::DEFAULT_BACKGROUND, _screenReversed, _blinkingState.IsBlinkingFaint(), _intenseIsBright); diff --git a/src/host/consoleInformation.cpp b/src/host/consoleInformation.cpp index 6ba8ac719dc..f8f851e2cec 100644 --- a/src/host/consoleInformation.cpp +++ b/src/host/consoleInformation.cpp @@ -220,33 +220,33 @@ InputBuffer* const CONSOLE_INFORMATION::GetActiveInputBuffer() const } // Method Description: -// - Return the default foreground color of the console. If the settings are -// configured to have a default foreground color (separate from the color -// table), this will return that value. Otherwise it will return the value -// from the colortable corresponding to our default attributes. +// - Return the color table index of the default foreground color. If the settings +// are configured to have a default foreground color (separate from the color +// table), this will return the extended index TextColor::DEFAULT_FOREGROUND. +// Otherwise it will return an index in the range 0 to 15. // Arguments: // - // Return Value: -// - the default foreground color of the console. -COLORREF CONSOLE_INFORMATION::GetDefaultForeground() const noexcept +// - the color table index of the default foreground color. +size_t CONSOLE_INFORMATION::GetDefaultForegroundIndex() const noexcept { const auto fg = GetColorTableEntry(TextColor::DEFAULT_FOREGROUND); - return fg != INVALID_COLOR ? fg : GetLegacyColorTableEntry(LOBYTE(GetFillAttribute()) & FG_ATTRS); + return fg != INVALID_COLOR ? TextColor::DEFAULT_FOREGROUND : TextColor::TransposeLegacyIndex(LOBYTE(GetFillAttribute()) & FG_ATTRS); } // Method Description: -// - Return the default background color of the console. If the settings are -// configured to have a default background color (separate from the color -// table), this will return that value. Otherwise it will return the value -// from the colortable corresponding to our default attributes. +// - Return the color table index of the default background color. If the settings +// are configured to have a default background color (separate from the color +// table), this will return the extended index TextColor::DEFAULT_BACKGROUND. +// Otherwise it will return an index in the range 0 to 15. // Arguments: // - // Return Value: -// - the default background color of the console. -COLORREF CONSOLE_INFORMATION::GetDefaultBackground() const noexcept +// - the color table index of the default background color. +size_t CONSOLE_INFORMATION::GetDefaultBackgroundIndex() const noexcept { const auto bg = GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); - return bg != INVALID_COLOR ? bg : GetLegacyColorTableEntry((LOBYTE(GetFillAttribute()) & BG_ATTRS) >> 4); + return bg != INVALID_COLOR ? TextColor::DEFAULT_BACKGROUND : TextColor::TransposeLegacyIndex((LOBYTE(GetFillAttribute()) & BG_ATTRS) >> 4); } // Method Description: @@ -258,7 +258,7 @@ COLORREF CONSOLE_INFORMATION::GetDefaultBackground() const noexcept // - The color values of the attribute's foreground and background. std::pair CONSOLE_INFORMATION::LookupAttributeColors(const TextAttribute& attr) const noexcept { - return LookupAttributeColors(attr, GetDefaultForeground(), GetDefaultBackground()); + return LookupAttributeColors(attr, GetDefaultForegroundIndex(), GetDefaultBackgroundIndex()); } // Method Description: @@ -266,17 +266,17 @@ std::pair CONSOLE_INFORMATION::LookupAttributeColors(const T // and the given default color values. // Arguments: // - attr: the TextAttribute to retrieve the foreground and background color of. -// - defaultFg: the COLORREF to use for a default foreground color. -// - defaultBg: the COLORREF to use for a default background color. +// - defaultFgIndex: the color table index to use for a default foreground color. +// - defaultBgIndex: the color table index to use for a default background color. // Return Value: // - The color values of the attribute's foreground and background. -std::pair CONSOLE_INFORMATION::LookupAttributeColors(const TextAttribute& attr, const COLORREF defaultFg, const COLORREF defaultBg) const noexcept +std::pair CONSOLE_INFORMATION::LookupAttributeColors(const TextAttribute& attr, const size_t defaultFgIndex, const size_t defaultBgIndex) const noexcept { _blinkingState.RecordBlinkingUsage(attr); return attr.CalculateRgbColors( GetColorTable(), - defaultFg, - defaultBg, + defaultFgIndex, + defaultBgIndex, IsScreenReversed(), _blinkingState.IsBlinkingFaint()); } diff --git a/src/host/renderData.cpp b/src/host/renderData.cpp index 1eb6090d78c..e1228a4ccc1 100644 --- a/src/host/renderData.cpp +++ b/src/host/renderData.cpp @@ -110,8 +110,8 @@ void RenderData::UnlockConsole() noexcept const TextAttribute RenderData::GetDefaultBrushColors() noexcept { const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - _defaultForeground = gci.GetDefaultForeground(); - _defaultBackground = gci.GetDefaultBackground(); + _defaultForegroundIndex = gci.GetDefaultForegroundIndex(); + _defaultBackgroundIndex = gci.GetDefaultBackgroundIndex(); return gci.GetActiveOutputBuffer().GetAttributes(); } @@ -364,7 +364,7 @@ const std::vector RenderData::GetPatternId(const COORD /*location*/) con std::pair RenderData::GetAttributeColors(const TextAttribute& attr) const noexcept { const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - return gci.LookupAttributeColors(attr, _defaultForeground, _defaultBackground); + return gci.LookupAttributeColors(attr, _defaultForegroundIndex, _defaultBackgroundIndex); } #pragma endregion diff --git a/src/host/renderData.hpp b/src/host/renderData.hpp index 9b5ab7f6d13..06f73774223 100644 --- a/src/host/renderData.hpp +++ b/src/host/renderData.hpp @@ -74,6 +74,6 @@ class RenderData final : #pragma endregion private: - COLORREF _defaultForeground = gsl::at(Microsoft::Console::Utils::CampbellColorTable(), 7); - COLORREF _defaultBackground = gsl::at(Microsoft::Console::Utils::CampbellColorTable(), 0); + size_t _defaultForegroundIndex = 7; + size_t _defaultBackgroundIndex = 0; }; diff --git a/src/host/server.h b/src/host/server.h index 2d8fa0d1580..d62930f3ac0 100644 --- a/src/host/server.h +++ b/src/host/server.h @@ -124,10 +124,10 @@ class CONSOLE_INFORMATION : COOKED_READ_DATA& CookedReadData() noexcept; void SetCookedReadData(COOKED_READ_DATA* readData) noexcept; - COLORREF GetDefaultForeground() const noexcept; - COLORREF GetDefaultBackground() const noexcept; + size_t GetDefaultForegroundIndex() const noexcept; + size_t GetDefaultBackgroundIndex() const noexcept; std::pair LookupAttributeColors(const TextAttribute& attr) const noexcept; - std::pair LookupAttributeColors(const TextAttribute& attr, const COLORREF defaultFg, const COLORREF defaultBg) const noexcept; + std::pair LookupAttributeColors(const TextAttribute& attr, const size_t defaultFgIndex, const size_t defaultBgIndex) const noexcept; void SetTitle(const std::wstring_view newTitle); void SetTitlePrefix(const std::wstring_view newTitlePrefix); diff --git a/src/interactivity/win32/Clipboard.cpp b/src/interactivity/win32/Clipboard.cpp index 8e05e78272b..9c743c3042a 100644 --- a/src/interactivity/win32/Clipboard.cpp +++ b/src/interactivity/win32/Clipboard.cpp @@ -208,10 +208,10 @@ void Clipboard::StoreSelectionToClipboard(bool const copyFormatting) const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); const auto& buffer = gci.GetActiveOutputBuffer().GetTextBuffer(); - const auto defaultForeground = gci.GetDefaultForeground(); - const auto defaultBackground = gci.GetDefaultBackground(); + const auto defaultForegroundIndex = gci.GetDefaultForegroundIndex(); + const auto defaultBackgroundIndex = gci.GetDefaultBackgroundIndex(); const auto GetAttributeColors = [=, &gci](const auto& attr) { - return gci.LookupAttributeColors(attr, defaultForeground, defaultBackground); + return gci.LookupAttributeColors(attr, defaultForegroundIndex, defaultBackgroundIndex); }; bool includeCRLF, trimTrailingWhitespace; @@ -276,9 +276,10 @@ void Clipboard::CopyTextToSystemClipboard(const TextBuffer::TextAndColor& rows, if (fAlsoCopyFormatting) { - const auto& fontData = ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer().GetCurrentFont(); + const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + const auto& fontData = gci.GetActiveOutputBuffer().GetCurrentFont(); int const iFontHeightPoints = fontData.GetUnscaledSize().Y * 72 / ServiceLocator::LocateGlobals().dpi; - const COLORREF bgColor = ServiceLocator::LocateGlobals().getConsoleInformation().GetDefaultBackground(); + const COLORREF bgColor = gci.GetColorTableEntry(gci.GetDefaultBackgroundIndex()); std::string HTMLToPlaceOnClip = TextBuffer::GenHTML(rows, iFontHeightPoints, fontData.GetFaceName(), bgColor); CopyToSystemClipboard(HTMLToPlaceOnClip, L"HTML Format"); From 6ebcb5eaedb1bc86c51be74f67f32014f1998f57 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sat, 13 Nov 2021 21:47:57 +0000 Subject: [PATCH 5/8] Cache the calculation of default color indices. --- .../ConptyRoundtripTests.cpp | 1 + src/host/consoleInformation.cpp | 48 +------------------ src/host/outputStream.cpp | 11 +++++ src/host/renderData.cpp | 4 +- src/host/renderData.hpp | 4 -- src/host/server.h | 3 -- src/host/settings.cpp | 35 ++++++++++++++ src/host/settings.hpp | 9 ++++ src/host/ut_host/ConptyOutputTests.cpp | 1 + src/host/ut_host/ScreenBufferTests.cpp | 8 ++++ src/interactivity/win32/Clipboard.cpp | 4 +- src/interactivity/win32/menu.cpp | 2 + 12 files changed, 71 insertions(+), 59 deletions(-) diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index dca50a5121c..ae063f1939b 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -99,6 +99,7 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, INVALID_COLOR); gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, INVALID_COLOR); gci.SetFillAttribute(0x07); // DARK_WHITE on DARK_BLACK + gci.CalculateDefaultColorIndices(); m_state->PrepareNewTextBufferInfo(true, TerminalViewWidth, TerminalViewHeight); auto& currentBuffer = gci.GetActiveOutputBuffer(); diff --git a/src/host/consoleInformation.cpp b/src/host/consoleInformation.cpp index f8f851e2cec..8ace241782b 100644 --- a/src/host/consoleInformation.cpp +++ b/src/host/consoleInformation.cpp @@ -219,36 +219,6 @@ InputBuffer* const CONSOLE_INFORMATION::GetActiveInputBuffer() const return pInputBuffer; } -// Method Description: -// - Return the color table index of the default foreground color. If the settings -// are configured to have a default foreground color (separate from the color -// table), this will return the extended index TextColor::DEFAULT_FOREGROUND. -// Otherwise it will return an index in the range 0 to 15. -// Arguments: -// - -// Return Value: -// - the color table index of the default foreground color. -size_t CONSOLE_INFORMATION::GetDefaultForegroundIndex() const noexcept -{ - const auto fg = GetColorTableEntry(TextColor::DEFAULT_FOREGROUND); - return fg != INVALID_COLOR ? TextColor::DEFAULT_FOREGROUND : TextColor::TransposeLegacyIndex(LOBYTE(GetFillAttribute()) & FG_ATTRS); -} - -// Method Description: -// - Return the color table index of the default background color. If the settings -// are configured to have a default background color (separate from the color -// table), this will return the extended index TextColor::DEFAULT_BACKGROUND. -// Otherwise it will return an index in the range 0 to 15. -// Arguments: -// - -// Return Value: -// - the color table index of the default background color. -size_t CONSOLE_INFORMATION::GetDefaultBackgroundIndex() const noexcept -{ - const auto bg = GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); - return bg != INVALID_COLOR ? TextColor::DEFAULT_BACKGROUND : TextColor::TransposeLegacyIndex((LOBYTE(GetFillAttribute()) & BG_ATTRS) >> 4); -} - // Method Description: // - Get the colors of a particular text attribute, using our color table, // and our configured default attributes. @@ -257,26 +227,12 @@ size_t CONSOLE_INFORMATION::GetDefaultBackgroundIndex() const noexcept // Return Value: // - The color values of the attribute's foreground and background. std::pair CONSOLE_INFORMATION::LookupAttributeColors(const TextAttribute& attr) const noexcept -{ - return LookupAttributeColors(attr, GetDefaultForegroundIndex(), GetDefaultBackgroundIndex()); -} - -// Method Description: -// - Get the colors of a particular text attribute, using our color table, -// and the given default color values. -// Arguments: -// - attr: the TextAttribute to retrieve the foreground and background color of. -// - defaultFgIndex: the color table index to use for a default foreground color. -// - defaultBgIndex: the color table index to use for a default background color. -// Return Value: -// - The color values of the attribute's foreground and background. -std::pair CONSOLE_INFORMATION::LookupAttributeColors(const TextAttribute& attr, const size_t defaultFgIndex, const size_t defaultBgIndex) const noexcept { _blinkingState.RecordBlinkingUsage(attr); return attr.CalculateRgbColors( GetColorTable(), - defaultFgIndex, - defaultBgIndex, + GetDefaultForegroundIndex(), + GetDefaultBackgroundIndex(), IsScreenReversed(), _blinkingState.IsBlinkingFaint()); } diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index ce1df8386cb..e049238771a 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -642,6 +642,17 @@ try gci.SetColorTableEntry(tableIndex, color); + // If we're setting the default foreground or background colors + // we need to make sure the index is correctly set as well. + if (tableIndex == TextColor::DEFAULT_FOREGROUND) + { + gci.SetDefaultForegroundIndex(TextColor::DEFAULT_FOREGROUND); + } + if (tableIndex == TextColor::DEFAULT_BACKGROUND) + { + gci.SetDefaultBackgroundIndex(TextColor::DEFAULT_BACKGROUND); + } + // Update the screen colors if we're not a pty // No need to force a redraw in pty mode. if (g.pRender && !gci.IsInVtIoMode()) diff --git a/src/host/renderData.cpp b/src/host/renderData.cpp index e1228a4ccc1..c860b3da3c8 100644 --- a/src/host/renderData.cpp +++ b/src/host/renderData.cpp @@ -110,8 +110,6 @@ void RenderData::UnlockConsole() noexcept const TextAttribute RenderData::GetDefaultBrushColors() noexcept { const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - _defaultForegroundIndex = gci.GetDefaultForegroundIndex(); - _defaultBackgroundIndex = gci.GetDefaultBackgroundIndex(); return gci.GetActiveOutputBuffer().GetAttributes(); } @@ -364,7 +362,7 @@ const std::vector RenderData::GetPatternId(const COORD /*location*/) con std::pair RenderData::GetAttributeColors(const TextAttribute& attr) const noexcept { const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - return gci.LookupAttributeColors(attr, _defaultForegroundIndex, _defaultBackgroundIndex); + return gci.LookupAttributeColors(attr); } #pragma endregion diff --git a/src/host/renderData.hpp b/src/host/renderData.hpp index 06f73774223..f75cbb5b623 100644 --- a/src/host/renderData.hpp +++ b/src/host/renderData.hpp @@ -72,8 +72,4 @@ class RenderData final : void ColorSelection(const COORD coordSelectionStart, const COORD coordSelectionEnd, const TextAttribute attr); const bool IsUiaDataInitialized() const noexcept override { return true; } #pragma endregion - -private: - size_t _defaultForegroundIndex = 7; - size_t _defaultBackgroundIndex = 0; }; diff --git a/src/host/server.h b/src/host/server.h index d62930f3ac0..cd6baf49392 100644 --- a/src/host/server.h +++ b/src/host/server.h @@ -124,10 +124,7 @@ class CONSOLE_INFORMATION : COOKED_READ_DATA& CookedReadData() noexcept; void SetCookedReadData(COOKED_READ_DATA* readData) noexcept; - size_t GetDefaultForegroundIndex() const noexcept; - size_t GetDefaultBackgroundIndex() const noexcept; std::pair LookupAttributeColors(const TextAttribute& attr) const noexcept; - std::pair LookupAttributeColors(const TextAttribute& attr, const size_t defaultFgIndex, const size_t defaultBgIndex) const noexcept; void SetTitle(const std::wstring_view newTitle); void SetTitlePrefix(const std::wstring_view newTitlePrefix); diff --git a/src/host/settings.cpp b/src/host/settings.cpp index 7af88f0bfd6..35836cb1e0b 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -56,6 +56,8 @@ Settings::Settings() : _fScreenReversed(false), // window size pixels initialized below _fInterceptCopyPaste(0), + _defaultForegroundIndex(TextColor::DARK_WHITE), + _defaultBackgroundIndex(TextColor::DARK_BLACK), _fUseDx(UseDx::Disabled), _fCopyColor(false) { @@ -356,6 +358,8 @@ 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); + // And calculate the position of the default colors in the color table. + CalculateDefaultColorIndices(); FAIL_FAST_IF(!(_dwWindowSize.X > 0)); FAIL_FAST_IF(!(_dwWindowSize.Y > 0)); @@ -795,6 +799,37 @@ void Settings::SetInterceptCopyPaste(const bool interceptCopyPaste) noexcept _fInterceptCopyPaste = interceptCopyPaste; } +void Settings::CalculateDefaultColorIndices() noexcept +{ + const auto foregroundColor = _colorTable.at(TextColor::DEFAULT_FOREGROUND); + const auto foregroundIndex = TextColor::TransposeLegacyIndex(_wFillAttribute & FG_ATTRS); + _defaultForegroundIndex = foregroundColor != INVALID_COLOR ? TextColor::DEFAULT_FOREGROUND : foregroundIndex; + + const auto backgroundColor = _colorTable.at(TextColor::DEFAULT_BACKGROUND); + const auto backgroundIndex = TextColor::TransposeLegacyIndex((_wFillAttribute & BG_ATTRS) >> 4); + _defaultBackgroundIndex = backgroundColor != INVALID_COLOR ? TextColor::DEFAULT_BACKGROUND : backgroundIndex; +} + +size_t Settings::GetDefaultForegroundIndex() const noexcept +{ + return _defaultForegroundIndex; +} + +void Settings::SetDefaultForegroundIndex(const size_t index) noexcept +{ + _defaultForegroundIndex = index; +} + +size_t Settings::GetDefaultBackgroundIndex() const noexcept +{ + return _defaultBackgroundIndex; +} + +void Settings::SetDefaultBackgroundIndex(const size_t index) noexcept +{ + _defaultBackgroundIndex = index; +} + bool Settings::IsTerminalScrolling() const noexcept { return _TerminalScrolling; diff --git a/src/host/settings.hpp b/src/host/settings.hpp index 65015cc7fb1..5ba1efebd4b 100644 --- a/src/host/settings.hpp +++ b/src/host/settings.hpp @@ -186,6 +186,12 @@ class Settings bool GetInterceptCopyPaste() const noexcept; void SetInterceptCopyPaste(const bool interceptCopyPaste) noexcept; + void CalculateDefaultColorIndices() noexcept; + size_t GetDefaultForegroundIndex() const noexcept; + void SetDefaultForegroundIndex(const size_t index) noexcept; + size_t GetDefaultBackgroundIndex() const noexcept; + void SetDefaultBackgroundIndex(const size_t index) noexcept; + bool IsTerminalScrolling() const noexcept; void SetTerminalScrolling(const bool terminalScrollingEnabled) noexcept; @@ -248,6 +254,9 @@ class Settings bool _fInterceptCopyPaste; + size_t _defaultForegroundIndex; + size_t _defaultBackgroundIndex; + bool _TerminalScrolling; friend class RegistrySerialization; }; diff --git a/src/host/ut_host/ConptyOutputTests.cpp b/src/host/ut_host/ConptyOutputTests.cpp index a8865c78377..80ea088ec21 100644 --- a/src/host/ut_host/ConptyOutputTests.cpp +++ b/src/host/ut_host/ConptyOutputTests.cpp @@ -70,6 +70,7 @@ class ConptyOutputTests gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, INVALID_COLOR); gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, INVALID_COLOR); gci.SetFillAttribute(0x07); // DARK_WHITE on DARK_BLACK + gci.CalculateDefaultColorIndices(); m_state->PrepareNewTextBufferInfo(true, TerminalViewWidth, TerminalViewHeight); auto& currentBuffer = gci.GetActiveOutputBuffer(); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 91fdf00378e..71c1cf1b2ab 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -62,6 +62,7 @@ class ScreenBufferTests gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, INVALID_COLOR); gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, INVALID_COLOR); gci.SetFillAttribute(0x07); // DARK_WHITE on DARK_BLACK + gci.CalculateDefaultColorIndices(); m_state->PrepareNewTextBufferInfo(); auto& currentBuffer = gci.GetActiveOutputBuffer(); @@ -1390,6 +1391,7 @@ void ScreenBufferTests::VtScrollMarginsNewlineColor() const COLORREF magenta = RGB(255, 0, 255); gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, yellow); gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); + gci.CalculateDefaultColorIndices(); const TextAttribute defaultAttrs = {}; si.SetAttributes(defaultAttrs); @@ -2261,6 +2263,7 @@ void ScreenBufferTests::SetDefaultsIndividuallyBothDefault() gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, yellow); gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); + gci.CalculateDefaultColorIndices(); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 6 X's:")); @@ -2363,6 +2366,7 @@ void ScreenBufferTests::SetDefaultsTogether() gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, yellow); gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); + gci.CalculateDefaultColorIndices(); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 6 X's:")); @@ -2434,6 +2438,7 @@ void ScreenBufferTests::ReverseResetWithDefaultBackground() gci.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, INVALID_COLOR); gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); + gci.CalculateDefaultColorIndices(); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 3 X's:")); @@ -2502,6 +2507,7 @@ void ScreenBufferTests::BackspaceDefaultAttrs() COLORREF magenta = RGB(255, 0, 255); gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); + gci.CalculateDefaultColorIndices(); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 2 X's, then backspace one.")); @@ -2565,6 +2571,7 @@ void ScreenBufferTests::BackspaceDefaultAttrsWriteCharsLegacy() COLORREF magenta = RGB(255, 0, 255); gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); + gci.CalculateDefaultColorIndices(); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); Log::Comment(NoThrowString().Format(L"Write 2 X's, then backspace one.")); @@ -2633,6 +2640,7 @@ void ScreenBufferTests::BackspaceDefaultAttrsInPrompt() COLORREF magenta = RGB(255, 0, 255); gci.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, magenta); + gci.CalculateDefaultColorIndices(); si.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); TextAttribute expectedDefaults{}; diff --git a/src/interactivity/win32/Clipboard.cpp b/src/interactivity/win32/Clipboard.cpp index 9c743c3042a..a1ca3bcf35c 100644 --- a/src/interactivity/win32/Clipboard.cpp +++ b/src/interactivity/win32/Clipboard.cpp @@ -208,10 +208,8 @@ void Clipboard::StoreSelectionToClipboard(bool const copyFormatting) const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); const auto& buffer = gci.GetActiveOutputBuffer().GetTextBuffer(); - const auto defaultForegroundIndex = gci.GetDefaultForegroundIndex(); - const auto defaultBackgroundIndex = gci.GetDefaultBackgroundIndex(); const auto GetAttributeColors = [=, &gci](const auto& attr) { - return gci.LookupAttributeColors(attr, defaultForegroundIndex, defaultBackgroundIndex); + return gci.LookupAttributeColors(attr); }; bool includeCRLF, trimTrailingWhitespace; diff --git a/src/interactivity/win32/menu.cpp b/src/interactivity/win32/menu.cpp index 401c39abda9..4c56ce8678d 100644 --- a/src/interactivity/win32/menu.cpp +++ b/src/interactivity/win32/menu.cpp @@ -584,6 +584,8 @@ void Menu::s_PropertiesUpdate(PCONSOLE_STATE_INFO pStateInfo) // Make sure the updated fill attributes are passed on to the TextAttribute class. TextAttribute::SetLegacyDefaultAttributes(pStateInfo->ScreenAttributes); + // And recalculate the position of the default colors in the color table. + gci.CalculateDefaultColorIndices(); // Set the screen info's default text attributes to defaults - ScreenInfo.SetDefaultAttributes({}, TextAttribute{ gci.GetPopupFillAttribute() }); From 7004ba73db8360c213645a00aad5e5723c77a15f Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 14 Nov 2021 20:44:40 +0000 Subject: [PATCH 6/8] Merge cursor color into color table. --- src/buffer/out/TextColor.h | 3 +- src/buffer/out/cursor.cpp | 23 ++------------- src/buffer/out/cursor.h | 8 +----- src/cascadia/TerminalCore/ITerminalApi.hpp | 1 - src/cascadia/TerminalCore/Terminal.cpp | 7 +++-- src/cascadia/TerminalCore/Terminal.hpp | 1 - src/cascadia/TerminalCore/TerminalApi.cpp | 8 ------ .../TerminalCore/TerminalDispatch.cpp | 2 +- .../TerminalCore/terminalrenderdata.cpp | 2 +- src/host/getset.cpp | 6 ---- src/host/getset.h | 2 -- src/host/outputStream.cpp | 15 ---------- src/host/outputStream.hpp | 1 - src/host/registry.cpp | 12 ++++++++ src/host/renderData.cpp | 3 +- src/host/screenInfo.cpp | 28 ++----------------- src/host/screenInfo.hpp | 2 -- src/host/selection.cpp | 1 - src/host/selection.hpp | 1 - src/host/selectionState.cpp | 2 -- src/host/settings.cpp | 22 +++++---------- src/host/settings.hpp | 5 ---- src/host/ut_host/ScreenBufferTests.cpp | 12 ++------ src/interactivity/win32/menu.cpp | 5 ++-- src/propslib/RegistrySerialization.cpp | 1 - src/terminal/adapter/adaptDispatch.cpp | 2 +- src/terminal/adapter/conGetSet.hpp | 1 - .../adapter/ut_adapter/adapterTest.cpp | 12 -------- 28 files changed, 38 insertions(+), 150 deletions(-) diff --git a/src/buffer/out/TextColor.h b/src/buffer/out/TextColor.h index fa8ef5da7f9..75a8e7ae26c 100644 --- a/src/buffer/out/TextColor.h +++ b/src/buffer/out/TextColor.h @@ -67,7 +67,8 @@ struct TextColor static constexpr size_t DEFAULT_FOREGROUND = 256; static constexpr size_t DEFAULT_BACKGROUND = 257; - static constexpr size_t TABLE_SIZE = 258; + static constexpr size_t CURSOR_COLOR = 258; + static constexpr size_t TABLE_SIZE = 259; constexpr TextColor() noexcept : _meta{ ColorType::IsDefault }, diff --git a/src/buffer/out/cursor.cpp b/src/buffer/out/cursor.cpp index 1d050a21503..4a3aba783ce 100644 --- a/src/buffer/out/cursor.cpp +++ b/src/buffer/out/cursor.cpp @@ -27,9 +27,7 @@ Cursor::Cursor(const ULONG ulSize, TextBuffer& parentBuffer) noexcept : _fDeferCursorRedraw(false), _fHaveDeferredCursorRedraw(false), _ulSize(ulSize), - _cursorType(CursorType::Legacy), - _fUseColor(false), - _color(s_InvertCursorColor) + _cursorType(CursorType::Legacy) { } @@ -143,10 +141,9 @@ void Cursor::SetSize(const ULONG ulSize) noexcept _RedrawCursor(); } -void Cursor::SetStyle(const ULONG ulSize, const COLORREF color, const CursorType type) noexcept +void Cursor::SetStyle(const ULONG ulSize, const CursorType type) noexcept { _ulSize = ulSize; - _color = color; _cursorType = type; _RedrawCursor(); @@ -285,7 +282,6 @@ void Cursor::CopyProperties(const Cursor& OtherCursor) noexcept // Size will be handled separately in the resize operation. //_ulSize = OtherCursor._ulSize; _cursorType = OtherCursor._cursorType; - _color = OtherCursor._color; } void Cursor::DelayEOLWrap(const COORD coordDelayedAt) noexcept @@ -335,21 +331,6 @@ const CursorType Cursor::GetType() const noexcept return _cursorType; } -const bool Cursor::IsUsingColor() const noexcept -{ - return GetColor() != INVALID_COLOR; -} - -const COLORREF Cursor::GetColor() const noexcept -{ - return _color; -} - -void Cursor::SetColor(const unsigned int color) noexcept -{ - _color = gsl::narrow_cast(color); -} - void Cursor::SetType(const CursorType type) noexcept { _cursorType = type; diff --git a/src/buffer/out/cursor.h b/src/buffer/out/cursor.h index dbd05f53528..ec0caa01821 100644 --- a/src/buffer/out/cursor.h +++ b/src/buffer/out/cursor.h @@ -24,7 +24,6 @@ class TextBuffer; class Cursor final { public: - static const unsigned int s_InvertCursorColor = INVALID_COLOR; // the following values are used to create the textmode cursor. static constexpr unsigned int CURSOR_SMALL_SIZE = 25; // large enough to be one pixel on a six pixel font @@ -51,8 +50,6 @@ class Cursor final COORD GetPosition() const noexcept; const CursorType GetType() const noexcept; - const bool IsUsingColor() const noexcept; - const COLORREF GetColor() const noexcept; void StartDeferDrawing() noexcept; bool IsDeferDrawing() noexcept; @@ -67,7 +64,7 @@ class Cursor final void SetIsPopupShown(const bool fIsPopupShown) noexcept; void SetDelay(const bool fDelay) noexcept; void SetSize(const ULONG ulSize) noexcept; - void SetStyle(const ULONG ulSize, const COLORREF color, const CursorType type) noexcept; + void SetStyle(const ULONG ulSize, const CursorType type) noexcept; void SetPosition(const COORD cPosition) noexcept; void SetXPosition(const int NewX) noexcept; @@ -84,7 +81,6 @@ class Cursor final COORD GetDelayedAtPosition() const noexcept; bool IsDelayedEOLWrap() const noexcept; - void SetColor(const unsigned int color) noexcept; void SetType(const CursorType type) noexcept; private: @@ -117,6 +113,4 @@ class Cursor final void _RedrawCursorAlways() noexcept; CursorType _cursorType; - bool _fUseColor; - COLORREF _color; }; diff --git a/src/cascadia/TerminalCore/ITerminalApi.hpp b/src/cascadia/TerminalCore/ITerminalApi.hpp index 5ac77984de3..f8943c5cb5a 100644 --- a/src/cascadia/TerminalCore/ITerminalApi.hpp +++ b/src/cascadia/TerminalCore/ITerminalApi.hpp @@ -44,7 +44,6 @@ namespace Microsoft::Terminal::Core virtual bool SetColorTableEntry(const size_t tableIndex, const COLORREF color) noexcept = 0; virtual bool SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle) noexcept = 0; - virtual bool SetCursorColor(const DWORD color) noexcept = 0; virtual bool SetInputMode(const ::Microsoft::Console::VirtualTerminal::TerminalInput::Mode mode, const bool enabled) noexcept = 0; diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 1621716c968..b622e40e90b 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -82,6 +82,7 @@ Terminal::Terminal() : _colorTable.at(TextColor::DEFAULT_FOREGROUND) = RGB(255, 255, 255); _colorTable.at(TextColor::DEFAULT_BACKGROUND) = ARGB(0, 0, 0, 0); + _colorTable.at(TextColor::CURSOR_COLOR) = INVALID_COLOR; } void Terminal::Create(COORD viewportSize, SHORT scrollbackLines, IRenderTarget& renderTarget) @@ -185,6 +186,8 @@ void Terminal::UpdateAppearance(const ICoreAppearance& appearance) _colorTable.at(TextColor::DEFAULT_BACKGROUND) = newBackgroundColor.with_alpha(0); const til::color newForegroundColor{ appearance.DefaultForeground() }; _colorTable.at(TextColor::DEFAULT_FOREGROUND) = newForegroundColor; + const til::color newCursorColor{ appearance.CursorColor() }; + _colorTable.at(TextColor::CURSOR_COLOR) = newCursorColor; _intenseIsBright = appearance.IntenseIsBright(); _adjustIndistinguishableColors = appearance.AdjustIndistinguishableColors(); @@ -224,9 +227,7 @@ void Terminal::UpdateAppearance(const ICoreAppearance& appearance) if (_buffer) { - _buffer->GetCursor().SetStyle(appearance.CursorHeight(), - til::color{ appearance.CursorColor() }, - cursorShape); + _buffer->GetCursor().SetStyle(appearance.CursorHeight(), cursorShape); } _defaultCursorShape = cursorShape; diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index b214f288fdc..1f8a6744767 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -109,7 +109,6 @@ class Microsoft::Terminal::Core::Terminal final : COLORREF GetColorTableEntry(const size_t tableIndex) const noexcept override; bool SetColorTableEntry(const size_t tableIndex, const COLORREF color) noexcept override; bool SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle) noexcept override; - bool SetCursorColor(const COLORREF color) noexcept override; bool SetInputMode(const ::Microsoft::Console::VirtualTerminal::TerminalInput::Mode mode, const bool enabled) noexcept override; diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index d6ed7fde91f..52463987a38 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -69,14 +69,6 @@ COORD Terminal::GetCursorPosition() noexcept return newPos; } -bool Terminal::SetCursorColor(const COLORREF color) noexcept -try -{ - _buffer->GetCursor().SetColor(color); - return true; -} -CATCH_RETURN_FALSE() - // Method Description: // - Moves the cursor down one line, and possibly also to the leftmost column. // Arguments: diff --git a/src/cascadia/TerminalCore/TerminalDispatch.cpp b/src/cascadia/TerminalCore/TerminalDispatch.cpp index 02f27e3d6e1..215e495da19 100644 --- a/src/cascadia/TerminalCore/TerminalDispatch.cpp +++ b/src/cascadia/TerminalCore/TerminalDispatch.cpp @@ -227,7 +227,7 @@ CATCH_LOG_RETURN_FALSE() bool TerminalDispatch::SetCursorColor(const DWORD color) noexcept try { - return _terminalApi.SetCursorColor(color); + return _terminalApi.SetColorTableEntry(TextColor::CURSOR_COLOR, color); } CATCH_LOG_RETURN_FALSE() diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp index a5a5d40e493..4396f8d8a9b 100644 --- a/src/cascadia/TerminalCore/terminalrenderdata.cpp +++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp @@ -135,7 +135,7 @@ CursorType Terminal::GetCursorStyle() const noexcept COLORREF Terminal::GetCursorColor() const noexcept { - return _buffer->GetCursor().GetColor(); + return _colorTable.at(TextColor::CURSOR_COLOR); } bool Terminal::IsCursorDoubleWidth() const diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 5b32c90fb47..7e894c161f4 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -1499,12 +1499,6 @@ void DoSrvSetCursorStyle(SCREEN_INFORMATION& screenInfo, screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor().SetType(cursorType); } -void DoSrvSetCursorColor(SCREEN_INFORMATION& screenInfo, - const COLORREF cursorColor) -{ - screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor().SetColor(cursorColor); -} - void DoSrvAddHyperlink(SCREEN_INFORMATION& screenInfo, const std::wstring_view uri, const std::wstring_view params) diff --git a/src/host/getset.h b/src/host/getset.h index cff83e9223c..3ffe0a56130 100644 --- a/src/host/getset.h +++ b/src/host/getset.h @@ -36,8 +36,6 @@ void DoSrvPrivateUseMainScreenBuffer(SCREEN_INFORMATION& screenInfo); void DoSrvSetCursorStyle(SCREEN_INFORMATION& screenInfo, const CursorType cursorType); -void DoSrvSetCursorColor(SCREEN_INFORMATION& screenInfo, - const COLORREF cursorColor); void DoSrvAddHyperlink(SCREEN_INFORMATION& screenInfo, const std::wstring_view uri, diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index e049238771a..cfeaa42455b 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -552,21 +552,6 @@ bool ConhostInternalGetSet::PrivateSuppressResizeRepaint() return SUCCEEDED(DoSrvPrivateSuppressResizeRepaint()); } -// Routine Description: -// - Connects the SetCursorStyle call directly into our Driver Message servicing call inside Conhost.exe -// SetCursorStyle is an internal-only "API" call that the vt commands can execute, -// but it is not represented as a function call on our public API surface. -// Arguments: -// - cursorColor: The color to change the cursor to. INVALID_COLOR will revert -// it to the legacy inverting behavior. -// Return Value: -// - true if successful (see DoSrvSetCursorStyle). false otherwise. -bool ConhostInternalGetSet::SetCursorColor(const COLORREF cursorColor) -{ - DoSrvSetCursorColor(_io.GetActiveOutputBuffer(), cursorColor); - return true; -} - // Routine Description: // - Connects the IsConsolePty call directly into our Driver Message servicing call inside Conhost.exe // - NOTE: This ONE method behaves differently! The rest of the methods on this diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index 7c7f0a6c7b3..70d98001103 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -101,7 +101,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: bool GetUserDefaultCursorStyle(CursorType& style) override; bool SetCursorStyle(CursorType const style) override; - bool SetCursorColor(COLORREF const color) override; bool PrivateRefreshWindow() override; diff --git a/src/host/registry.cpp b/src/host/registry.cpp index f7f3b7bcb64..b630931d543 100644 --- a/src/host/registry.cpp +++ b/src/host/registry.cpp @@ -338,6 +338,18 @@ void Registry::LoadFromRegistry(_In_ PCWSTR const pwszConsoleTitle) _pSettings->SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, dwValue); } + // Cursor color + Status = RegistrySerialization::s_QueryValue(hTitleKey, + CONSOLE_REGISTRY_CURSORCOLOR, + sizeof(dwValue), + REG_DWORD, + (PBYTE)&dwValue, + nullptr); + if (NT_SUCCESS(Status)) + { + _pSettings->SetColorTableEntry(TextColor::CURSOR_COLOR, dwValue); + } + GetEditKeys(hConsoleKey); // Close the registry keys diff --git a/src/host/renderData.cpp b/src/host/renderData.cpp index c860b3da3c8..8bb8f9dafcc 100644 --- a/src/host/renderData.cpp +++ b/src/host/renderData.cpp @@ -224,8 +224,7 @@ ULONG RenderData::GetCursorPixelWidth() const noexcept COLORREF RenderData::GetCursorColor() const noexcept { const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - const auto& cursor = gci.GetActiveOutputBuffer().GetTextBuffer().GetCursor(); - return cursor.GetColor(); + return gci.GetColorTableEntry(TextColor::CURSOR_COLOR); } // Routine Description: diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index cc06df4ca45..eb5a8b6e29a 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -123,7 +123,6 @@ SCREEN_INFORMATION::~SCREEN_INFORMATION() pScreen->_renderTarget); const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - pScreen->_textBuffer->GetCursor().SetColor(gci.GetCursorColor()); pScreen->_textBuffer->GetCursor().SetType(gci.GetCursorType()); const NTSTATUS status = pScreen->_InitializeOutputStateMachine(); @@ -1630,29 +1629,6 @@ void SCREEN_INFORMATION::SetCursorInformation(const ULONG Size, } } -// Routine Description: -// - This routine sets the cursor color. Also updates the cursor information of -// this buffer's main buffer, if this buffer is an alt buffer. -// Arguments: -// - Color - The new color to set the cursor to -// - setMain - If true, propagate change to main buffer as well. -// Return Value: -// - None -void SCREEN_INFORMATION::SetCursorColor(const unsigned int Color, const bool setMain) noexcept -{ - Cursor& cursor = _textBuffer->GetCursor(); - - cursor.SetColor(Color); - - // If we're an alt buffer, DON'T propagate this setting up to the main buffer. - // We don't want to pollute that buffer with this state, - // UNLESS we're getting called from the propsheet, then we DO want to update this. - if (_psiMainBuffer && setMain) - { - _psiMainBuffer->SetCursorColor(Color); - } -} - // Routine Description: // - This routine sets the cursor shape both in the data // structures and on the screen. Also updates the cursor information of @@ -1908,7 +1884,7 @@ const SCREEN_INFORMATION& SCREEN_INFORMATION::GetMainBuffer() const auto& myCursor = GetTextBuffer().GetCursor(); auto* const createdBuffer = *ppsiNewScreenBuffer; auto& altCursor = createdBuffer->GetTextBuffer().GetCursor(); - altCursor.SetStyle(myCursor.GetSize(), myCursor.GetColor(), myCursor.GetType()); + altCursor.SetStyle(myCursor.GetSize(), myCursor.GetType()); altCursor.SetIsVisible(myCursor.IsVisible()); altCursor.SetBlinkingAllowed(myCursor.IsBlinkingAllowed()); // The new position should match the viewport-relative position of the main buffer. @@ -2006,7 +1982,7 @@ void SCREEN_INFORMATION::UseMainScreenBuffer() // Copy the alt buffer's cursor style and visibility back to the main buffer. const auto& altCursor = psiAlt->GetTextBuffer().GetCursor(); auto& mainCursor = psiMain->GetTextBuffer().GetCursor(); - mainCursor.SetStyle(altCursor.GetSize(), altCursor.GetColor(), altCursor.GetType()); + mainCursor.SetStyle(altCursor.GetSize(), altCursor.GetType()); mainCursor.SetIsVisible(altCursor.IsVisible()); mainCursor.SetBlinkingAllowed(altCursor.IsBlinkingAllowed()); diff --git a/src/host/screenInfo.hpp b/src/host/screenInfo.hpp index e3b1eed4df2..740dbb53fcd 100644 --- a/src/host/screenInfo.hpp +++ b/src/host/screenInfo.hpp @@ -189,8 +189,6 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console void SetCursorInformation(const ULONG Size, const bool Visible) noexcept; - void SetCursorColor(const unsigned int Color, const bool setMain = false) noexcept; - void SetCursorType(const CursorType Type, const bool setMain = false) noexcept; void SetCursorDBMode(const bool DoubleCursor); diff --git a/src/host/selection.cpp b/src/host/selection.cpp index 90bb5668f3e..0b1e0c5f570 100644 --- a/src/host/selection.cpp +++ b/src/host/selection.cpp @@ -16,7 +16,6 @@ Selection::Selection() : _fSelectionVisible(false), _ulSavedCursorSize(0), _fSavedCursorVisible(false), - _savedCursorColor(INVALID_COLOR), _savedCursorType(CursorType::Legacy), _dwSelectionFlags(0), _fLineSelection(true), diff --git a/src/host/selection.hpp b/src/host/selection.hpp index 34dc872d8cd..9f786521497 100644 --- a/src/host/selection.hpp +++ b/src/host/selection.hpp @@ -174,7 +174,6 @@ class Selection COORD _coordSavedCursorPosition; ULONG _ulSavedCursorSize; bool _fSavedCursorVisible; - COLORREF _savedCursorColor; CursorType _savedCursorType; #ifdef UNIT_TESTING diff --git a/src/host/selectionState.cpp b/src/host/selectionState.cpp index af82bd764df..8a8705fee63 100644 --- a/src/host/selectionState.cpp +++ b/src/host/selectionState.cpp @@ -168,7 +168,6 @@ void Selection::_SaveCursorData(const Cursor& cursor) noexcept _coordSavedCursorPosition = cursor.GetPosition(); _ulSavedCursorSize = cursor.GetSize(); _fSavedCursorVisible = cursor.IsVisible(); - _savedCursorColor = cursor.GetColor(); _savedCursorType = cursor.GetType(); } @@ -182,7 +181,6 @@ void Selection::_RestoreDataToCursor(Cursor& cursor) noexcept { cursor.SetSize(_ulSavedCursorSize); cursor.SetIsVisible(_fSavedCursorVisible); - cursor.SetColor(_savedCursorColor); cursor.SetType(_savedCursorType); cursor.SetIsOn(true); cursor.SetPosition(_coordSavedCursorPosition); diff --git a/src/host/settings.cpp b/src/host/settings.cpp index 35836cb1e0b..cda78119dd4 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -78,7 +78,6 @@ Settings::Settings() : ZeroMemory((void*)&_FaceName, sizeof(_FaceName)); wcscpy_s(_FaceName, DEFAULT_TT_FONT_FACENAME); - _CursorColor = Cursor::s_InvertCursorColor; _CursorType = CursorType::Legacy; gsl::span tableView = { _colorTable.data(), _colorTable.size() }; @@ -86,6 +85,7 @@ Settings::Settings() : _colorTable.at(TextColor::DEFAULT_FOREGROUND) = INVALID_COLOR; _colorTable.at(TextColor::DEFAULT_BACKGROUND) = INVALID_COLOR; + _colorTable.at(TextColor::CURSOR_COLOR) = INVALID_COLOR; } // Routine Description: @@ -232,11 +232,11 @@ void Settings::InitFromStateInfo(_In_ PCONSOLE_STATE_INFO pStateInfo) _fCtrlKeyShortcutsDisabled = pStateInfo->fCtrlKeyShortcutsDisabled; _bLineSelection = pStateInfo->fLineSelection; _bWindowAlpha = pStateInfo->bWindowTransparency; - _CursorColor = pStateInfo->CursorColor; _CursorType = static_cast(pStateInfo->CursorType); _fInterceptCopyPaste = pStateInfo->InterceptCopyPaste; _colorTable.at(TextColor::DEFAULT_FOREGROUND) = pStateInfo->DefaultForeground; _colorTable.at(TextColor::DEFAULT_BACKGROUND) = pStateInfo->DefaultBackground; + _colorTable.at(TextColor::CURSOR_COLOR) = pStateInfo->CursorColor; _TerminalScrolling = pStateInfo->TerminalScrolling; } @@ -277,11 +277,11 @@ CONSOLE_STATE_INFO Settings::CreateConsoleStateInfo() const csi.fCtrlKeyShortcutsDisabled = _fCtrlKeyShortcutsDisabled; csi.fLineSelection = _bLineSelection; csi.bWindowTransparency = _bWindowAlpha; - csi.CursorColor = _CursorColor; csi.CursorType = static_cast(_CursorType); csi.InterceptCopyPaste = _fInterceptCopyPaste; csi.DefaultForeground = _colorTable.at(TextColor::DEFAULT_FOREGROUND); csi.DefaultBackground = _colorTable.at(TextColor::DEFAULT_BACKGROUND); + csi.CursorColor = _colorTable.at(TextColor::CURSOR_COLOR); csi.TerminalScrolling = _TerminalScrolling; return csi; } @@ -335,11 +335,13 @@ void Settings::Validate() const auto defaultForeground = _colorTable.at(TextColor::DEFAULT_FOREGROUND); const auto defaultBackground = _colorTable.at(TextColor::DEFAULT_BACKGROUND); + const auto cursorColor = _colorTable.at(TextColor::CURSOR_COLOR); // If the extended color options are set to invalid values (all the same color), reset them. - if (_CursorColor != Cursor::s_InvertCursorColor && _CursorColor == defaultBackground) + if (cursorColor != INVALID_COLOR && cursorColor == defaultBackground) { - _CursorColor = Cursor::s_InvertCursorColor; + // INVALID_COLOR is used to represent "Invert Colors" + _colorTable.at(TextColor::CURSOR_COLOR) = INVALID_COLOR; } if (defaultForeground != INVALID_COLOR && defaultForeground == defaultBackground) @@ -769,21 +771,11 @@ COLORREF Settings::GetLegacyColorTableEntry(const size_t index) const return _colorTable.at(TextColor::TransposeLegacyIndex(index)); } -COLORREF Settings::GetCursorColor() const noexcept -{ - return _CursorColor; -} - CursorType Settings::GetCursorType() const noexcept { return _CursorType; } -void Settings::SetCursorColor(const COLORREF CursorColor) noexcept -{ - _CursorColor = CursorColor; -} - void Settings::SetCursorType(const CursorType cursorType) noexcept { _CursorType = cursorType; diff --git a/src/host/settings.hpp b/src/host/settings.hpp index 5ba1efebd4b..3929b4df920 100644 --- a/src/host/settings.hpp +++ b/src/host/settings.hpp @@ -177,10 +177,7 @@ class Settings void SetLegacyColorTableEntry(const size_t index, const COLORREF ColorValue); COLORREF GetLegacyColorTableEntry(const size_t index) const; - COLORREF GetCursorColor() const noexcept; CursorType GetCursorType() const noexcept; - - void SetCursorColor(const COLORREF CursorColor) noexcept; void SetCursorType(const CursorType cursorType) noexcept; bool GetInterceptCopyPaste() const noexcept; @@ -248,8 +245,6 @@ class Settings bool _fUseWindowSizePixels; COORD _dwWindowSizePixels; - // Technically a COLORREF, but using INVALID_COLOR as "Invert Colors" - unsigned int _CursorColor; CursorType _CursorType; bool _fInterceptCopyPaste; diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 71c1cf1b2ab..4d9381309d9 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -360,12 +360,11 @@ void ScreenBufferTests::AlternateBufferCursorInheritanceTest() auto mainCursorPos = COORD{ 3, 5 }; auto mainCursorVisible = false; auto mainCursorSize = 33u; - auto mainCursorColor = RGB(1, 2, 3); auto mainCursorType = CursorType::DoubleUnderscore; auto mainCursorBlinking = false; mainCursor.SetPosition(mainCursorPos); mainCursor.SetIsVisible(mainCursorVisible); - mainCursor.SetStyle(mainCursorSize, mainCursorColor, mainCursorType); + mainCursor.SetStyle(mainCursorSize, mainCursorType); mainCursor.SetBlinkingAllowed(mainCursorBlinking); Log::Comment(L"Switch to the alternate buffer."); @@ -380,7 +379,6 @@ void ScreenBufferTests::AlternateBufferCursorInheritanceTest() VERIFY_ARE_EQUAL(mainCursorVisible, altCursor.IsVisible()); Log::Comment(L"Confirm the cursor style is inherited from the main buffer."); VERIFY_ARE_EQUAL(mainCursorSize, altCursor.GetSize()); - VERIFY_ARE_EQUAL(mainCursorColor, altCursor.GetColor()); VERIFY_ARE_EQUAL(mainCursorType, altCursor.GetType()); VERIFY_ARE_EQUAL(mainCursorBlinking, altCursor.IsBlinkingAllowed()); @@ -388,12 +386,11 @@ void ScreenBufferTests::AlternateBufferCursorInheritanceTest() auto altCursorPos = COORD{ 5, 3 }; auto altCursorVisible = true; auto altCursorSize = 66u; - auto altCursorColor = RGB(3, 2, 1); auto altCursorType = CursorType::EmptyBox; auto altCursorBlinking = true; altCursor.SetPosition(altCursorPos); altCursor.SetIsVisible(altCursorVisible); - altCursor.SetStyle(altCursorSize, altCursorColor, altCursorType); + altCursor.SetStyle(altCursorSize, altCursorType); altCursor.SetBlinkingAllowed(altCursorBlinking); Log::Comment(L"Switch back to the main buffer."); @@ -407,7 +404,6 @@ void ScreenBufferTests::AlternateBufferCursorInheritanceTest() VERIFY_ARE_EQUAL(altCursorVisible, mainCursor.IsVisible()); Log::Comment(L"Confirm the cursor style is inherited from the alt buffer."); VERIFY_ARE_EQUAL(altCursorSize, mainCursor.GetSize()); - VERIFY_ARE_EQUAL(altCursorColor, mainCursor.GetColor()); VERIFY_ARE_EQUAL(altCursorType, mainCursor.GetType()); VERIFY_ARE_EQUAL(altCursorBlinking, mainCursor.IsBlinkingAllowed()); } @@ -1743,7 +1739,6 @@ void ScreenBufferTests::ResizeCursorUnchanged() // Get initial cursor values const CursorType initialType = initialCursor.GetType(); const auto initialSize = initialCursor.GetSize(); - const COLORREF initialColor = initialCursor.GetColor(); // set our wrap mode accordingly - ResizeScreenBuffer will be smart enough // to call the appropriate implementation @@ -1758,10 +1753,8 @@ void ScreenBufferTests::ResizeCursorUnchanged() const auto& finalCursor = si.GetTextBuffer().GetCursor(); const CursorType finalType = finalCursor.GetType(); const auto finalSize = finalCursor.GetSize(); - const COLORREF finalColor = finalCursor.GetColor(); VERIFY_ARE_EQUAL(initialType, finalType); - VERIFY_ARE_EQUAL(initialColor, finalColor); VERIFY_ARE_EQUAL(initialSize, finalSize); } @@ -2124,7 +2117,6 @@ void ScreenBufferTests::TestAltBufferCursorState() // Validate that the cursor state was copied appropriately into the // alternate buffer VERIFY_ARE_EQUAL(mainCursor.GetSize(), altCursor.GetSize()); - VERIFY_ARE_EQUAL(mainCursor.GetColor(), altCursor.GetColor()); VERIFY_ARE_EQUAL(mainCursor.GetType(), altCursor.GetType()); } } diff --git a/src/interactivity/win32/menu.cpp b/src/interactivity/win32/menu.cpp index 4c56ce8678d..b09eacf3b40 100644 --- a/src/interactivity/win32/menu.cpp +++ b/src/interactivity/win32/menu.cpp @@ -315,7 +315,7 @@ void Menu::s_ShowPropertiesDialog(HWND const hwnd, BOOL const Defaults) const Cursor& cursor = ScreenInfo.GetTextBuffer().GetCursor(); pStateInfo->CursorSize = cursor.GetSize(); - pStateInfo->CursorColor = cursor.GetColor(); + pStateInfo->CursorColor = gci.GetColorTableEntry(TextColor::CURSOR_COLOR); pStateInfo->CursorType = static_cast(cursor.GetType()); // Retrieve small icon for use in displaying the dialog @@ -461,13 +461,12 @@ void Menu::s_PropertiesUpdate(PCONSOLE_STATE_INFO pStateInfo) // Set the cursor properties in the Settings const auto cursorType = static_cast(pStateInfo->CursorType); - gci.SetCursorColor(pStateInfo->CursorColor); gci.SetCursorType(cursorType); + gci.SetColorTableEntry(TextColor::CURSOR_COLOR, pStateInfo->CursorColor); // Then also apply them to the buffer's cursor ScreenInfo.SetCursorInformation(pStateInfo->CursorSize, ScreenInfo.GetTextBuffer().GetCursor().IsVisible()); - ScreenInfo.SetCursorColor(pStateInfo->CursorColor, true); ScreenInfo.SetCursorType(cursorType, true); gci.SetTerminalScrolling(pStateInfo->TerminalScrolling); diff --git a/src/propslib/RegistrySerialization.cpp b/src/propslib/RegistrySerialization.cpp index e5a4610f91d..0c40fecb741 100644 --- a/src/propslib/RegistrySerialization.cpp +++ b/src/propslib/RegistrySerialization.cpp @@ -57,7 +57,6 @@ const RegistrySerialization::_RegPropertyMap RegistrySerialization::s_PropertyMa { _RegPropertyType::Boolean, CONSOLE_REGISTRY_TRIMZEROHEADINGS, SET_FIELD_AND_SIZE(_fTrimLeadingZeros) }, { _RegPropertyType::Boolean, CONSOLE_REGISTRY_ENABLE_COLOR_SELECTION, SET_FIELD_AND_SIZE(_fEnableColorSelection) }, { _RegPropertyType::Coordinate, CONSOLE_REGISTRY_WINDOWPOS, SET_FIELD_AND_SIZE(_dwWindowOrigin) }, - { _RegPropertyType::Dword, CONSOLE_REGISTRY_CURSORCOLOR, SET_FIELD_AND_SIZE(_CursorColor) }, { _RegPropertyType::Dword, CONSOLE_REGISTRY_CURSORTYPE, SET_FIELD_AND_SIZE(_CursorType) }, { _RegPropertyType::Boolean, CONSOLE_REGISTRY_INTERCEPTCOPYPASTE, SET_FIELD_AND_SIZE(_fInterceptCopyPaste) }, { _RegPropertyType::Boolean, CONSOLE_REGISTRY_TERMINALSCROLLING, SET_FIELD_AND_SIZE(_TerminalScrolling) }, diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index dc7b161f990..18d81fd56f7 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -2250,7 +2250,7 @@ bool AdaptDispatch::SetCursorColor(const COLORREF cursorColor) return false; } - return _pConApi->SetCursorColor(cursorColor); + return _pConApi->SetColorTableEntry(TextColor::CURSOR_COLOR, cursorColor); } // Routine Description: diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp index 59dd4c11aa1..1c975e2d42d 100644 --- a/src/terminal/adapter/conGetSet.hpp +++ b/src/terminal/adapter/conGetSet.hpp @@ -72,7 +72,6 @@ namespace Microsoft::Console::VirtualTerminal virtual bool PrivateClearBuffer() = 0; virtual bool GetUserDefaultCursorStyle(CursorType& style) = 0; virtual bool SetCursorStyle(const CursorType style) = 0; - virtual bool SetCursorColor(const COLORREF color) = 0; virtual bool PrivateWriteConsoleControlInput(const KeyEvent key) = 0; virtual bool PrivateRefreshWindow() = 0; diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index 1bf7a31c25d..2f45b32a7b4 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -370,16 +370,6 @@ class TestGetSet final : public ConGetSet return _setCursorStyleResult; } - bool SetCursorColor(const COLORREF cursorColor) override - { - Log::Comment(L"SetCursorColor MOCK called..."); - if (_setCursorColorResult) - { - VERIFY_ARE_EQUAL(_expectedCursorColor, cursorColor); - } - return _setCursorColorResult; - } - bool PrivateRefreshWindow() override { Log::Comment(L"PrivateRefreshWindow MOCK called..."); @@ -717,8 +707,6 @@ class TestGetSet final : public ConGetSet std::wstring_view _expectedWindowTitle{}; bool _setCursorStyleResult = false; CursorType _expectedCursorStyle; - bool _setCursorColorResult = false; - COLORREF _expectedCursorColor = 0; bool _setConsoleOutputCPResult = false; bool _getConsoleOutputCPResult = false; bool _moveToBottomResult = false; From a1e98f56bc0664239460ecefe04379d98019b1df Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 14 Nov 2021 20:48:58 +0000 Subject: [PATCH 7/8] Simplify registry queries for special colors. --- src/host/registry.cpp | 36 -------------------------- src/propslib/RegistrySerialization.cpp | 7 +++-- 2 files changed, 5 insertions(+), 38 deletions(-) diff --git a/src/host/registry.cpp b/src/host/registry.cpp index b630931d543..49bd0642354 100644 --- a/src/host/registry.cpp +++ b/src/host/registry.cpp @@ -314,42 +314,6 @@ void Registry::LoadFromRegistry(_In_ PCWSTR const pwszConsoleTitle) } } - // Default foreground color - Status = RegistrySerialization::s_QueryValue(hTitleKey, - CONSOLE_REGISTRY_DEFAULTFOREGROUND, - sizeof(dwValue), - REG_DWORD, - (PBYTE)&dwValue, - nullptr); - if (NT_SUCCESS(Status)) - { - _pSettings->SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, dwValue); - } - - // Default background color - Status = RegistrySerialization::s_QueryValue(hTitleKey, - CONSOLE_REGISTRY_DEFAULTBACKGROUND, - sizeof(dwValue), - REG_DWORD, - (PBYTE)&dwValue, - nullptr); - if (NT_SUCCESS(Status)) - { - _pSettings->SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, dwValue); - } - - // Cursor color - Status = RegistrySerialization::s_QueryValue(hTitleKey, - CONSOLE_REGISTRY_CURSORCOLOR, - sizeof(dwValue), - REG_DWORD, - (PBYTE)&dwValue, - nullptr); - if (NT_SUCCESS(Status)) - { - _pSettings->SetColorTableEntry(TextColor::CURSOR_COLOR, dwValue); - } - GetEditKeys(hConsoleKey); // Close the registry keys diff --git a/src/propslib/RegistrySerialization.cpp b/src/propslib/RegistrySerialization.cpp index 0c40fecb741..4c45ba05c77 100644 --- a/src/propslib/RegistrySerialization.cpp +++ b/src/propslib/RegistrySerialization.cpp @@ -7,7 +7,7 @@ #pragma hdrstop -#define SET_FIELD_AND_SIZE(x) FIELD_OFFSET(Settings, x), RTL_FIELD_SIZE(Settings, x) +#define SET_FIELD_AND_SIZE(x) UFIELD_OFFSET(Settings, x), RTL_FIELD_SIZE(Settings, x) #define NT_TESTNULL(var) (((var) == nullptr) ? STATUS_NO_MEMORY : STATUS_SUCCESS) @@ -61,7 +61,10 @@ const RegistrySerialization::_RegPropertyMap RegistrySerialization::s_PropertyMa { _RegPropertyType::Boolean, CONSOLE_REGISTRY_INTERCEPTCOPYPASTE, SET_FIELD_AND_SIZE(_fInterceptCopyPaste) }, { _RegPropertyType::Boolean, CONSOLE_REGISTRY_TERMINALSCROLLING, SET_FIELD_AND_SIZE(_TerminalScrolling) }, { _RegPropertyType::Dword, CONSOLE_REGISTRY_USEDX, SET_FIELD_AND_SIZE(_fUseDx) }, - { _RegPropertyType::Boolean, CONSOLE_REGISTRY_COPYCOLOR, SET_FIELD_AND_SIZE(_fCopyColor) } + { _RegPropertyType::Boolean, CONSOLE_REGISTRY_COPYCOLOR, SET_FIELD_AND_SIZE(_fCopyColor) }, + { _RegPropertyType::Dword, CONSOLE_REGISTRY_DEFAULTFOREGROUND, SET_FIELD_AND_SIZE(_colorTable[TextColor::DEFAULT_FOREGROUND]) }, + { _RegPropertyType::Dword, CONSOLE_REGISTRY_DEFAULTBACKGROUND, SET_FIELD_AND_SIZE(_colorTable[TextColor::DEFAULT_BACKGROUND]) }, + { _RegPropertyType::Dword, CONSOLE_REGISTRY_CURSORCOLOR, SET_FIELD_AND_SIZE(_colorTable[TextColor::CURSOR_COLOR]) } }; const size_t RegistrySerialization::s_PropertyMappingsSize = ARRAYSIZE(s_PropertyMappings); From 69f2b4f3a1fa281b7eddeb8ca23a874e98263784 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 18 Nov 2021 15:54:14 +0000 Subject: [PATCH 8/8] Silence the spelling bot. --- .github/actions/spelling/allow/apis.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/spelling/allow/apis.txt b/.github/actions/spelling/allow/apis.txt index 83ca55178bc..572acd0239c 100644 --- a/.github/actions/spelling/allow/apis.txt +++ b/.github/actions/spelling/allow/apis.txt @@ -169,6 +169,7 @@ toupper TTask TVal UChar +UFIELD ULARGE UPDATEINIFILE userenv