From 3388a486dce5a75ec9900b968d6b245be14b1f11 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Fri, 10 Jul 2020 23:26:34 +0100 Subject: [PATCH] Refactor the renderer color calculations (#6853) This is a refactoring of the renderer color calculations to simplify the implementation, and to make it easier to support additional color-altering rendition attributes in the future (e.g. _faint_ and _conceal_). ## References * This is a followup to PRs #3817 and #6809, which introduced additional complexity in the color calculations, and which suggested the need for refactoring. ## Detailed Description of the Pull Request / Additional comments When we added support for `DECSCNM`, that required the foreground and background color lookup methods to be able to return the opposite of what was requested when the reversed mode was set. That made those methods unnecessarily complicated, and I thought we could simplify them considerably just by combining the calculations into a single method that derived both colors at the same time. And since both conhost and Windows Terminal needed to perform the same calculations, it also made sense to move that functionality into the `TextAttribute` class, where it could easily be shared. In general this way of doing things is a bit more efficient. However, it does result in some unnecessary work when only one of the colors is required, as is the case for the gridline painter. So to make that less of an issue, I've reordered the gridline code a bit so it at least avoids looking up the colors when no gridlines are needed. ## Validation Steps Performed Because of the API changes, quite a lot of the unit tests had to be updated. For example instead of verifying colors with two separate calls to `LookupForegroundColor` and `LookupBackgroundColor`, that's now achieved with a single `LookupAttributeColors` call, comparing against a pair of values. The specifics of the tests haven't changed though, and they're all still working as expected. I've also manually confirmed that the various color sequences and rendition attributes are rendering correctly with the new refactoring. --- src/buffer/out/TextAttribute.cpp | 61 +++------ src/buffer/out/TextAttribute.hpp | 15 +-- src/buffer/out/TextColor.h | 2 +- src/buffer/out/textBuffer.cpp | 13 +- src/buffer/out/textBuffer.hpp | 3 +- .../out/ut_textbuffer/TextAttributeTests.cpp | 48 +++---- .../PublicTerminalCore/HwndTerminal.cpp | 2 +- src/cascadia/TerminalCore/Terminal.hpp | 3 +- .../TerminalCore/TerminalSelection.cpp | 6 +- .../TerminalCore/terminalrenderdata.cpp | 34 ++--- src/host/renderData.cpp | 19 +-- src/host/renderData.hpp | 3 +- src/host/settings.cpp | 41 ++---- src/host/settings.hpp | 3 +- src/host/ut_host/ScreenBufferTests.cpp | 88 +++++-------- src/host/ut_host/TextBufferTests.cpp | 124 +++++++----------- src/interactivity/win32/Clipboard.cpp | 6 +- src/renderer/base/renderer.cpp | 13 +- src/renderer/dx/DxRenderer.cpp | 3 +- src/renderer/gdi/paint.cpp | 3 - src/renderer/gdi/state.cpp | 3 +- src/renderer/inc/IRenderData.hpp | 3 +- 22 files changed, 169 insertions(+), 327 deletions(-) diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index fe47a9108b..cd1f17aa97 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -81,54 +81,27 @@ bool TextAttribute::IsLegacy() const noexcept return _foreground.IsLegacy() && _background.IsLegacy(); } -// Arguments: -// - None -// Return Value: -// - color that should be displayed as the foreground color -COLORREF TextAttribute::CalculateRgbForeground(std::basic_string_view colorTable, - COLORREF defaultFgColor, - COLORREF defaultBgColor) const noexcept -{ - return IsReverseVideo() ? _GetRgbBackground(colorTable, defaultBgColor) : _GetRgbForeground(colorTable, defaultFgColor); -} - // Routine Description: -// - Calculates rgb background color based off of current color table and active modification attributes +// - Calculates rgb colors based off of current color table and active modification attributes. // Arguments: -// - None +// - colorTable: the current color table rgb values. +// - defaultFgColor: the default foreground color rgb value. +// - defaultBgColor: the default background color rgb value. +// - reverseScreenMode: true if the screen mode is reversed. // Return Value: -// - color that should be displayed as the background color -COLORREF TextAttribute::CalculateRgbBackground(std::basic_string_view colorTable, - COLORREF defaultFgColor, - COLORREF defaultBgColor) const noexcept +// - the foreground and background colors that should be displayed. +std::pair TextAttribute::CalculateRgbColors(const std::basic_string_view colorTable, + const COLORREF defaultFgColor, + const COLORREF defaultBgColor, + const bool reverseScreenMode) const noexcept { - return IsReverseVideo() ? _GetRgbForeground(colorTable, defaultFgColor) : _GetRgbBackground(colorTable, defaultBgColor); -} - -// Routine Description: -// - gets rgb foreground color, possibly based off of current color table. Does not take active modification -// attributes into account -// Arguments: -// - None -// Return Value: -// - color that is stored as the foreground color -COLORREF TextAttribute::_GetRgbForeground(std::basic_string_view colorTable, - COLORREF defaultColor) const noexcept -{ - return _foreground.GetColor(colorTable, defaultColor, IsBold()); -} - -// Routine Description: -// - gets rgb background color, possibly based off of current color table. Does not take active modification -// attributes into account -// Arguments: -// - None -// Return Value: -// - color that is stored as the background color -COLORREF TextAttribute::_GetRgbBackground(std::basic_string_view colorTable, - COLORREF defaultColor) const noexcept -{ - return _background.GetColor(colorTable, defaultColor, false); + auto fg = _foreground.GetColor(colorTable, defaultFgColor, IsBold()); + auto bg = _background.GetColor(colorTable, defaultBgColor); + if (IsReverseVideo() ^ reverseScreenMode) + { + std::swap(fg, bg); + } + return { fg, bg }; } TextColor TextAttribute::GetForeground() const noexcept diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index 58db242ec3..9719a4f607 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -63,12 +63,10 @@ public: static TextAttribute StripErroneousVT16VersionsOfLegacyDefaults(const TextAttribute& attribute) noexcept; WORD GetLegacyAttributes() const noexcept; - COLORREF CalculateRgbForeground(std::basic_string_view colorTable, - COLORREF defaultFgColor, - COLORREF defaultBgColor) const noexcept; - COLORREF CalculateRgbBackground(std::basic_string_view colorTable, - COLORREF defaultFgColor, - COLORREF defaultBgColor) const noexcept; + std::pair CalculateRgbColors(const std::basic_string_view colorTable, + const COLORREF defaultFgColor, + const COLORREF defaultBgColor, + const bool reverseScreenMode = false) const noexcept; bool IsLeadingByte() const noexcept; bool IsTrailingByte() const noexcept; @@ -154,11 +152,6 @@ public: } private: - COLORREF _GetRgbForeground(std::basic_string_view colorTable, - COLORREF defaultColor) const noexcept; - COLORREF _GetRgbBackground(std::basic_string_view colorTable, - COLORREF defaultColor) const noexcept; - static constexpr TextColor s_LegacyIndexOrDefault(const BYTE requestedIndex, const BYTE defaultIndex) { return requestedIndex == defaultIndex ? TextColor{} : TextColor{ requestedIndex, true }; diff --git a/src/buffer/out/TextColor.h b/src/buffer/out/TextColor.h index 16698fbd94..6085dbd338 100644 --- a/src/buffer/out/TextColor.h +++ b/src/buffer/out/TextColor.h @@ -90,7 +90,7 @@ public: COLORREF GetColor(std::basic_string_view colorTable, const COLORREF defaultColor, - const bool brighten) const noexcept; + const bool brighten = false) const noexcept; BYTE GetLegacyIndex(const BYTE defaultIndex) const noexcept; diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 5462ffceea..faee76caf0 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1461,18 +1461,16 @@ void TextBuffer::_ExpandTextRow(SMALL_RECT& textRow) const // - includeCRLF - inject CRLF pairs to the end of each line // - trimTrailingWhitespace - remove the trailing whitespace at the end of each line // - textRects - the rectangular regions from which the data will be extracted from the buffer (i.e.: selection rects) -// - GetForegroundColor - function used to map TextAttribute to RGB COLORREF for foreground color. If null, only extract the text. -// - GetBackgroundColor - function used to map TextAttribute to RGB COLORREF for background color. If null, only extract the text. +// - GetAttributeColors - function used to map TextAttribute to RGB COLORREFs. If null, only extract the text. // Return Value: // - The text, background color, and foreground color data of the selected region of the text buffer. const TextBuffer::TextAndColor TextBuffer::GetText(const bool includeCRLF, const bool trimTrailingWhitespace, const std::vector& selectionRects, - std::function GetForegroundColor, - std::function GetBackgroundColor) const + std::function(const TextAttribute&)> GetAttributeColors) const { TextAndColor data; - const bool copyTextColor = GetForegroundColor && GetBackgroundColor; + const bool copyTextColor = GetAttributeColors != nullptr; // preallocate our vectors to reduce reallocs size_t const rows = selectionRects.size(); @@ -1517,9 +1515,8 @@ const TextBuffer::TextAndColor TextBuffer::GetText(const bool includeCRLF, if (copyTextColor) { - auto cellData = cell.TextAttr(); - COLORREF const CellFgAttr = GetForegroundColor(cellData); - COLORREF const CellBkAttr = GetBackgroundColor(cellData); + const auto cellData = cell.TextAttr(); + const auto [CellFgAttr, CellBkAttr] = GetAttributeColors(cellData); for (const wchar_t wch : cell.Chars()) { selectionFgAttr.push_back(CellFgAttr); diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 3f19e3a759..269bb6911e 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -152,8 +152,7 @@ public: const TextAndColor GetText(const bool lineSelection, const bool trimTrailingWhitespace, const std::vector& textRects, - std::function GetForegroundColor = nullptr, - std::function GetBackgroundColor = nullptr) const; + std::function(const TextAttribute&)> GetAttributeColors = nullptr) const; static std::string GenHTML(const TextAndColor& rows, const int fontHeightPoints, diff --git a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp index f9dfcc76f5..c94fe7f052 100644 --- a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp +++ b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp @@ -138,21 +138,17 @@ void TextAttributeTests::TestTextAttributeColorGetters() // values when reverse video is not set VERIFY_IS_FALSE(attr.IsReverseVideo()); - VERIFY_ARE_EQUAL(red, attr._GetRgbForeground(view, _defaultFg)); - VERIFY_ARE_EQUAL(red, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); - - VERIFY_ARE_EQUAL(green, attr._GetRgbBackground(view, _defaultBg)); - VERIFY_ARE_EQUAL(green, attr.CalculateRgbBackground(view, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(view, _defaultFg)); + VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(view, _defaultBg)); + VERIFY_ARE_EQUAL(std::make_pair(red, green), attr.CalculateRgbColors(view, _defaultFg, _defaultBg)); // with reverse video set, calculated foreground/background values should be // switched while getters stay the same attr.SetReverseVideo(true); - VERIFY_ARE_EQUAL(red, attr._GetRgbForeground(view, _defaultFg)); - VERIFY_ARE_EQUAL(green, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); - - VERIFY_ARE_EQUAL(green, attr._GetRgbBackground(view, _defaultBg)); - VERIFY_ARE_EQUAL(red, attr.CalculateRgbBackground(view, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(view, _defaultFg)); + VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(view, _defaultBg)); + VERIFY_ARE_EQUAL(std::make_pair(green, red), attr.CalculateRgbColors(view, _defaultFg, _defaultBg)); } void TextAttributeTests::TestReverseDefaultColors() @@ -166,42 +162,34 @@ void TextAttributeTests::TestReverseDefaultColors() // values when reverse video is not set VERIFY_IS_FALSE(attr.IsReverseVideo()); - VERIFY_ARE_EQUAL(_defaultFg, attr._GetRgbForeground(view, _defaultFg)); - VERIFY_ARE_EQUAL(_defaultFg, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); - - VERIFY_ARE_EQUAL(_defaultBg, attr._GetRgbBackground(view, _defaultBg)); - VERIFY_ARE_EQUAL(_defaultBg, attr.CalculateRgbBackground(view, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(view, _defaultFg)); + VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(view, _defaultBg)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), attr.CalculateRgbColors(view, _defaultFg, _defaultBg)); // 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._GetRgbForeground(view, _defaultFg)); - VERIFY_ARE_EQUAL(_defaultBg, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); - - VERIFY_ARE_EQUAL(_defaultBg, attr._GetRgbBackground(view, _defaultBg)); - VERIFY_ARE_EQUAL(_defaultFg, attr.CalculateRgbBackground(view, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(view, _defaultFg)); + VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(view, _defaultBg)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultBg, _defaultFg), attr.CalculateRgbColors(view, _defaultFg, _defaultBg)); attr.SetForeground(red); VERIFY_IS_TRUE(attr.IsReverseVideo()); - VERIFY_ARE_EQUAL(red, attr._GetRgbForeground(view, _defaultFg)); - VERIFY_ARE_EQUAL(_defaultBg, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); - - VERIFY_ARE_EQUAL(_defaultBg, attr._GetRgbBackground(view, _defaultBg)); - VERIFY_ARE_EQUAL(red, attr.CalculateRgbBackground(view, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(view, _defaultFg)); + VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(view, _defaultBg)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultBg, red), attr.CalculateRgbColors(view, _defaultFg, _defaultBg)); attr.Invert(); VERIFY_IS_FALSE(attr.IsReverseVideo()); attr.SetDefaultForeground(); attr.SetBackground(green); - VERIFY_ARE_EQUAL(_defaultFg, attr._GetRgbForeground(view, _defaultFg)); - VERIFY_ARE_EQUAL(_defaultFg, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); - - VERIFY_ARE_EQUAL(green, attr._GetRgbBackground(view, _defaultBg)); - VERIFY_ARE_EQUAL(green, attr.CalculateRgbBackground(view, _defaultFg, _defaultBg)); + VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(view, _defaultFg)); + VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(view, _defaultBg)); + VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, green), attr.CalculateRgbColors(view, _defaultFg, _defaultBg)); } void TextAttributeTests::TestRoundtripDefaultColors() diff --git a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp index cb31e1860f..d5a151f3bf 100644 --- a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp +++ b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp @@ -732,7 +732,7 @@ try { const auto& fontData = _actualFont; int const iFontHeightPoints = fontData.GetUnscaledSize().Y; // this renderer uses points already - const COLORREF bgColor = _terminal->GetBackgroundColor(_terminal->GetDefaultBrushColors()); + const COLORREF bgColor = _terminal->GetAttributeColors(_terminal->GetDefaultBrushColors()).second; std::string HTMLToPlaceOnClip = TextBuffer::GenHTML(rows, iFontHeightPoints, fontData.GetFaceName(), bgColor); _CopyToSystemClipboard(HTMLToPlaceOnClip, L"HTML Format"); diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index d2f60e1886..64232ef963 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -139,8 +139,7 @@ public: #pragma region IRenderData // These methods are defined in TerminalRenderData.cpp const TextAttribute GetDefaultBrushColors() noexcept override; - const COLORREF GetForegroundColor(const TextAttribute& attr) const noexcept override; - const COLORREF GetBackgroundColor(const TextAttribute& attr) const noexcept override; + std::pair GetAttributeColors(const TextAttribute& attr) const noexcept override; COORD GetCursorPosition() const noexcept override; bool IsCursorVisible() const noexcept override; bool IsCursorOn() const noexcept override; diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 7d38370277..32c45a92a9 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -253,14 +253,12 @@ const TextBuffer::TextAndColor Terminal::RetrieveSelectedTextFromBuffer(bool sin { const auto selectionRects = _GetSelectionRects(); - std::function GetForegroundColor = std::bind(&Terminal::GetForegroundColor, this, std::placeholders::_1); - std::function GetBackgroundColor = std::bind(&Terminal::GetBackgroundColor, this, std::placeholders::_1); + const auto GetAttributeColors = std::bind(&Terminal::GetAttributeColors, this, std::placeholders::_1); return _buffer->GetText(!singleLine, !singleLine, selectionRects, - GetForegroundColor, - GetBackgroundColor); + GetAttributeColors); } // Method Description: diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp index 294ac78027..65e9105034 100644 --- a/src/cascadia/TerminalCore/terminalrenderdata.cpp +++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp @@ -50,38 +50,20 @@ const TextAttribute Terminal::GetDefaultBrushColors() noexcept return TextAttribute{}; } -const COLORREF Terminal::GetForegroundColor(const TextAttribute& attr) const noexcept +std::pair Terminal::GetAttributeColors(const TextAttribute& attr) const noexcept { - COLORREF fgColor{}; - if (_screenReversed) - { - fgColor = attr.CalculateRgbBackground({ _colorTable.data(), _colorTable.size() }, _defaultFg, _defaultBg); - } - else - { - fgColor = attr.CalculateRgbForeground({ _colorTable.data(), _colorTable.size() }, _defaultFg, _defaultBg); - } - return 0xff000000 | fgColor; -} - -const COLORREF Terminal::GetBackgroundColor(const TextAttribute& attr) const noexcept -{ - COLORREF bgColor{}; - if (_screenReversed) - { - bgColor = attr.CalculateRgbForeground({ _colorTable.data(), _colorTable.size() }, _defaultFg, _defaultBg); - } - else - { - bgColor = attr.CalculateRgbBackground({ _colorTable.data(), _colorTable.size() }, _defaultFg, _defaultBg); - } + auto colors = attr.CalculateRgbColors({ _colorTable.data(), _colorTable.size() }, + _defaultFg, + _defaultBg, + _screenReversed); + colors.first |= 0xff000000; // We only care about alpha for the default BG (which enables acrylic) // If the bg isn't the default bg color, or reverse video is enabled, make it fully opaque. if (!attr.BackgroundIsDefault() || (attr.IsReverseVideo() ^ _screenReversed)) { - return 0xff000000 | bgColor; + colors.second |= 0xff000000; } - return bgColor; + return colors; } COORD Terminal::GetCursorPosition() const noexcept diff --git a/src/host/renderData.cpp b/src/host/renderData.cpp index 3398b11eae..b8397cbbb0 100644 --- a/src/host/renderData.cpp +++ b/src/host/renderData.cpp @@ -325,25 +325,14 @@ const std::wstring RenderData::GetConsoleTitle() const noexcept } // Routine Description: -// - Converts a text attribute into the foreground RGB value that should be presented, applying +// - Converts a text attribute into the RGB values that should be presented, applying // relevant table translation information and preferences. // Return Value: -// - ARGB color value -const COLORREF RenderData::GetForegroundColor(const TextAttribute& attr) const noexcept +// - ARGB color values for the foreground and background +std::pair RenderData::GetAttributeColors(const TextAttribute& attr) const noexcept { const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - return gci.LookupForegroundColor(attr); -} - -// Routine Description: -// - Converts a text attribute into the background RGB value that should be presented, applying -// relevant table translation information and preferences. -// Return Value: -// - ARGB color value -const COLORREF RenderData::GetBackgroundColor(const TextAttribute& attr) const noexcept -{ - const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - return gci.LookupBackgroundColor(attr); + return gci.LookupAttributeColors(attr); } #pragma endregion diff --git a/src/host/renderData.hpp b/src/host/renderData.hpp index 0c0cab3f94..4b2bf2e3b5 100644 --- a/src/host/renderData.hpp +++ b/src/host/renderData.hpp @@ -37,8 +37,7 @@ public: #pragma region IRenderData const TextAttribute GetDefaultBrushColors() noexcept override; - const COLORREF GetForegroundColor(const TextAttribute& attr) const noexcept override; - const COLORREF GetBackgroundColor(const TextAttribute& attr) const noexcept override; + std::pair GetAttributeColors(const TextAttribute& attr) const noexcept override; COORD GetCursorPosition() const noexcept override; bool IsCursorVisible() const noexcept override; diff --git a/src/host/settings.cpp b/src/host/settings.cpp index 1eb534aa0b..4e91e50c5a 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -862,43 +862,18 @@ COLORREF Settings::CalculateDefaultBackground() const noexcept } // Method Description: -// - Get the foreground color of a particular text attribute, using our color -// table, and our configured default attributes. +// - Get the colors of a particular text attribute, using our color table, +// and our configured default attributes. // Arguments: // - attr: the TextAttribute to retrieve the foreground color of. // Return Value: -// - The color value of the attribute's foreground TextColor. -COLORREF Settings::LookupForegroundColor(const TextAttribute& attr) const noexcept +// - The color values of the attribute's foreground and background. +std::pair Settings::LookupAttributeColors(const TextAttribute& attr) const noexcept { - const auto tableView = Get256ColorTable(); - if (_fScreenReversed) - { - return attr.CalculateRgbBackground(tableView, CalculateDefaultForeground(), CalculateDefaultBackground()); - } - else - { - return attr.CalculateRgbForeground(tableView, CalculateDefaultForeground(), CalculateDefaultBackground()); - } -} - -// Method Description: -// - Get the background color of a particular text attribute, using our color -// table, and our configured default attributes. -// Arguments: -// - attr: the TextAttribute to retrieve the background color of. -// Return Value: -// - The color value of the attribute's background TextColor. -COLORREF Settings::LookupBackgroundColor(const TextAttribute& attr) const noexcept -{ - const auto tableView = Get256ColorTable(); - if (_fScreenReversed) - { - return attr.CalculateRgbForeground(tableView, CalculateDefaultForeground(), CalculateDefaultBackground()); - } - else - { - return attr.CalculateRgbBackground(tableView, CalculateDefaultForeground(), CalculateDefaultBackground()); - } + return attr.CalculateRgbColors(Get256ColorTable(), + CalculateDefaultForeground(), + CalculateDefaultBackground(), + _fScreenReversed); } bool Settings::GetCopyColor() const noexcept diff --git a/src/host/settings.hpp b/src/host/settings.hpp index a7d719ff20..ed13b65f6e 100644 --- a/src/host/settings.hpp +++ b/src/host/settings.hpp @@ -187,8 +187,7 @@ public: COLORREF CalculateDefaultForeground() const noexcept; COLORREF CalculateDefaultBackground() const noexcept; - COLORREF LookupForegroundColor(const TextAttribute& attr) const noexcept; - COLORREF LookupBackgroundColor(const TextAttribute& attr) const noexcept; + std::pair LookupAttributeColors(const TextAttribute& attr) const noexcept; private: DWORD _dwHotKey; diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 22fc29c820..8a25ca2de6 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -1362,8 +1362,7 @@ void ScreenBufferTests::VtScrollMarginsNewlineColor() const auto& attr = attrs[x]; VERIFY_ARE_EQUAL(false, attr.IsLegacy()); VERIFY_ARE_EQUAL(defaultAttrs, attr); - VERIFY_ARE_EQUAL(yellow, gci.LookupForegroundColor(attr)); - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attr)); + VERIFY_ARE_EQUAL(std::make_pair(yellow, magenta), gci.LookupAttributeColors(attr)); } } } @@ -2270,19 +2269,12 @@ void ScreenBufferTests::SetDefaultsIndividuallyBothDefault() VERIFY_ARE_EQUAL(expectedTwo, attrE); VERIFY_ARE_EQUAL(expectedSix, attrF); - VERIFY_ARE_EQUAL(yellow, gci.LookupForegroundColor(attrA)); - VERIFY_ARE_EQUAL(brightGreen, gci.LookupForegroundColor(attrB)); - VERIFY_ARE_EQUAL(yellow, gci.LookupForegroundColor(attrC)); - VERIFY_ARE_EQUAL(yellow, gci.LookupForegroundColor(attrD)); - VERIFY_ARE_EQUAL(brightGreen, gci.LookupForegroundColor(attrE)); - VERIFY_ARE_EQUAL(brightGreen, gci.LookupForegroundColor(attrF)); - - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrA)); - VERIFY_ARE_EQUAL(darkBlue, gci.LookupBackgroundColor(attrB)); - VERIFY_ARE_EQUAL(darkBlue, gci.LookupBackgroundColor(attrC)); - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrD)); - VERIFY_ARE_EQUAL(darkBlue, gci.LookupBackgroundColor(attrE)); - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrF)); + VERIFY_ARE_EQUAL(std::make_pair(yellow, magenta), gci.LookupAttributeColors(attrA)); + VERIFY_ARE_EQUAL(std::make_pair(brightGreen, darkBlue), gci.LookupAttributeColors(attrB)); + VERIFY_ARE_EQUAL(std::make_pair(yellow, darkBlue), gci.LookupAttributeColors(attrC)); + VERIFY_ARE_EQUAL(std::make_pair(yellow, magenta), gci.LookupAttributeColors(attrD)); + VERIFY_ARE_EQUAL(std::make_pair(brightGreen, darkBlue), gci.LookupAttributeColors(attrE)); + VERIFY_ARE_EQUAL(std::make_pair(brightGreen, magenta), gci.LookupAttributeColors(attrF)); } void ScreenBufferTests::SetDefaultsTogether() @@ -2354,13 +2346,9 @@ void ScreenBufferTests::SetDefaultsTogether() VERIFY_ARE_EQUAL(expectedTwo, attrB); VERIFY_ARE_EQUAL(expectedDefaults, attrC); - VERIFY_ARE_EQUAL(yellow, gci.LookupForegroundColor(attrA)); - VERIFY_ARE_EQUAL(yellow, gci.LookupForegroundColor(attrB)); - VERIFY_ARE_EQUAL(yellow, gci.LookupForegroundColor(attrC)); - - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrA)); - VERIFY_ARE_EQUAL(color250, gci.LookupBackgroundColor(attrB)); - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrC)); + VERIFY_ARE_EQUAL(std::make_pair(yellow, magenta), gci.LookupAttributeColors(attrA)); + VERIFY_ARE_EQUAL(std::make_pair(yellow, color250), gci.LookupAttributeColors(attrB)); + VERIFY_ARE_EQUAL(std::make_pair(yellow, magenta), gci.LookupAttributeColors(attrC)); } void ScreenBufferTests::ReverseResetWithDefaultBackground() @@ -2424,9 +2412,9 @@ void ScreenBufferTests::ReverseResetWithDefaultBackground() VERIFY_ARE_EQUAL(expectedReversed, attrB); VERIFY_ARE_EQUAL(expectedDefaults, attrC); - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrA)); - VERIFY_ARE_EQUAL(magenta, gci.LookupForegroundColor(attrB)); - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrC)); + VERIFY_ARE_EQUAL(magenta, gci.LookupAttributeColors(attrA).second); + VERIFY_ARE_EQUAL(magenta, gci.LookupAttributeColors(attrB).first); + VERIFY_ARE_EQUAL(magenta, gci.LookupAttributeColors(attrC).second); } void ScreenBufferTests::BackspaceDefaultAttrs() @@ -2477,8 +2465,8 @@ void ScreenBufferTests::BackspaceDefaultAttrs() VERIFY_ARE_EQUAL(expectedDefaults, attrA); VERIFY_ARE_EQUAL(expectedDefaults, attrB); - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrA)); - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrB)); + VERIFY_ARE_EQUAL(magenta, gci.LookupAttributeColors(attrA).second); + VERIFY_ARE_EQUAL(magenta, gci.LookupAttributeColors(attrB).second); } void ScreenBufferTests::BackspaceDefaultAttrsWriteCharsLegacy() @@ -2554,8 +2542,8 @@ void ScreenBufferTests::BackspaceDefaultAttrsWriteCharsLegacy() VERIFY_ARE_EQUAL(expectedDefaults, attrA); VERIFY_ARE_EQUAL(expectedDefaults, attrB); - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrA)); - VERIFY_ARE_EQUAL(magenta, gci.LookupBackgroundColor(attrB)); + VERIFY_ARE_EQUAL(magenta, gci.LookupAttributeColors(attrA).second); + VERIFY_ARE_EQUAL(magenta, gci.LookupAttributeColors(attrB).second); } void ScreenBufferTests::BackspaceDefaultAttrsInPrompt() @@ -2665,7 +2653,7 @@ void ScreenBufferTests::SetGlobalColorTable() const std::vector attrs{ attrRow->begin(), attrRow->end() }; const auto attrA = attrs[0]; LOG_ATTR(attrA); - VERIFY_ARE_EQUAL(originalRed, gci.LookupBackgroundColor(attrA)); + VERIFY_ARE_EQUAL(originalRed, gci.LookupAttributeColors(attrA).second); } Log::Comment(NoThrowString().Format(L"Create an alt buffer")); @@ -2691,7 +2679,7 @@ void ScreenBufferTests::SetGlobalColorTable() const std::vector attrs{ attrRow->begin(), attrRow->end() }; const auto attrA = attrs[0]; LOG_ATTR(attrA); - VERIFY_ARE_EQUAL(originalRed, gci.LookupBackgroundColor(attrA)); + VERIFY_ARE_EQUAL(originalRed, gci.LookupAttributeColors(attrA).second); } Log::Comment(NoThrowString().Format(L"Change the value of red to RGB(0x11, 0x22, 0x33)")); @@ -2708,8 +2696,8 @@ void ScreenBufferTests::SetGlobalColorTable() const auto attrB = attrs[1]; LOG_ATTR(attrA); LOG_ATTR(attrB); - VERIFY_ARE_EQUAL(testColor, gci.LookupBackgroundColor(attrA)); - VERIFY_ARE_EQUAL(testColor, gci.LookupBackgroundColor(attrB)); + VERIFY_ARE_EQUAL(testColor, gci.LookupAttributeColors(attrA).second); + VERIFY_ARE_EQUAL(testColor, gci.LookupAttributeColors(attrB).second); } Log::Comment(NoThrowString().Format(L"Switch back to the main buffer")); @@ -2731,8 +2719,8 @@ void ScreenBufferTests::SetGlobalColorTable() const auto attrB = attrs[1]; LOG_ATTR(attrA); LOG_ATTR(attrB); - VERIFY_ARE_EQUAL(testColor, gci.LookupBackgroundColor(attrA)); - VERIFY_ARE_EQUAL(testColor, gci.LookupBackgroundColor(attrB)); + VERIFY_ARE_EQUAL(testColor, gci.LookupAttributeColors(attrA).second); + VERIFY_ARE_EQUAL(testColor, gci.LookupAttributeColors(attrB).second); } } @@ -2771,7 +2759,7 @@ void ScreenBufferTests::SetColorTableThreeDigits() const std::vector attrs{ attrRow->begin(), attrRow->end() }; const auto attrA = attrs[0]; LOG_ATTR(attrA); - VERIFY_ARE_EQUAL(originalRed, gci.LookupBackgroundColor(attrA)); + VERIFY_ARE_EQUAL(originalRed, gci.LookupAttributeColors(attrA).second); } Log::Comment(NoThrowString().Format(L"Create an alt buffer")); @@ -2797,7 +2785,7 @@ void ScreenBufferTests::SetColorTableThreeDigits() const std::vector attrs{ attrRow->begin(), attrRow->end() }; const auto attrA = attrs[0]; LOG_ATTR(attrA); - VERIFY_ARE_EQUAL(originalRed, gci.LookupBackgroundColor(attrA)); + VERIFY_ARE_EQUAL(originalRed, gci.LookupAttributeColors(attrA).second); } Log::Comment(NoThrowString().Format(L"Change the value of red to RGB(0x11, 0x22, 0x33)")); @@ -2816,7 +2804,7 @@ void ScreenBufferTests::SetColorTableThreeDigits() const auto attrB = attrs[1]; // TODO MSFT:20105972 - attrA and attrB should both be the same color now LOG_ATTR(attrB); - VERIFY_ARE_EQUAL(testColor, gci.LookupBackgroundColor(attrB)); + VERIFY_ARE_EQUAL(testColor, gci.LookupAttributeColors(attrB).second); } } @@ -3195,11 +3183,9 @@ void ScreenBufferTests::DontResetColorsAboveVirtualBottom() const auto attrB = attrs[1]; LOG_ATTR(attrA); LOG_ATTR(attrB); - VERIFY_ARE_EQUAL(darkRed, gci.LookupForegroundColor(attrA)); - VERIFY_ARE_EQUAL(darkBlue, gci.LookupBackgroundColor(attrA)); + VERIFY_ARE_EQUAL(std::make_pair(darkRed, darkBlue), gci.LookupAttributeColors(attrA)); - VERIFY_ARE_EQUAL(darkWhite, gci.LookupForegroundColor(attrB)); - VERIFY_ARE_EQUAL(darkBlack, gci.LookupBackgroundColor(attrB)); + VERIFY_ARE_EQUAL(std::make_pair(darkWhite, darkBlack), gci.LookupAttributeColors(attrB)); } Log::Comment(NoThrowString().Format(L"Emulate scrolling up with the mouse")); @@ -3230,14 +3216,11 @@ void ScreenBufferTests::DontResetColorsAboveVirtualBottom() LOG_ATTR(attrA); LOG_ATTR(attrB); LOG_ATTR(attrC); - VERIFY_ARE_EQUAL(darkRed, gci.LookupForegroundColor(attrA)); - VERIFY_ARE_EQUAL(darkBlue, gci.LookupBackgroundColor(attrA)); + VERIFY_ARE_EQUAL(std::make_pair(darkRed, darkBlue), gci.LookupAttributeColors(attrA)); - VERIFY_ARE_EQUAL(darkWhite, gci.LookupForegroundColor(attrB)); - VERIFY_ARE_EQUAL(darkBlack, gci.LookupBackgroundColor(attrB)); + VERIFY_ARE_EQUAL(std::make_pair(darkWhite, darkBlack), gci.LookupAttributeColors(attrB)); - VERIFY_ARE_EQUAL(darkWhite, gci.LookupForegroundColor(attrC)); - VERIFY_ARE_EQUAL(darkBlack, gci.LookupBackgroundColor(attrC)); + VERIFY_ARE_EQUAL(std::make_pair(darkWhite, darkBlack), gci.LookupAttributeColors(attrC)); } } @@ -4601,20 +4584,17 @@ void ScreenBufferTests::SetScreenMode() Log::Comment(L"By default the screen mode is normal."); VERIFY_IS_FALSE(gci.IsScreenReversed()); - VERIFY_ARE_EQUAL(rgbForeground, gci.LookupForegroundColor(testAttr)); - VERIFY_ARE_EQUAL(rgbBackground, gci.LookupBackgroundColor(testAttr)); + VERIFY_ARE_EQUAL(std::make_pair(rgbForeground, rgbBackground), gci.LookupAttributeColors(testAttr)); Log::Comment(L"When DECSCNM is set, background and foreground colors are switched."); stateMachine.ProcessString(L"\x1B[?5h"); VERIFY_IS_TRUE(gci.IsScreenReversed()); - VERIFY_ARE_EQUAL(rgbBackground, gci.LookupForegroundColor(testAttr)); - VERIFY_ARE_EQUAL(rgbForeground, gci.LookupBackgroundColor(testAttr)); + VERIFY_ARE_EQUAL(std::make_pair(rgbBackground, rgbForeground), gci.LookupAttributeColors(testAttr)); Log::Comment(L"When DECSCNM is reset, the colors are normal again."); stateMachine.ProcessString(L"\x1B[?5l"); VERIFY_IS_FALSE(gci.IsScreenReversed()); - VERIFY_ARE_EQUAL(rgbForeground, gci.LookupForegroundColor(testAttr)); - VERIFY_ARE_EQUAL(rgbBackground, gci.LookupBackgroundColor(testAttr)); + VERIFY_ARE_EQUAL(std::make_pair(rgbForeground, rgbBackground), gci.LookupAttributeColors(testAttr)); } void ScreenBufferTests::SetOriginMode() diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp index 8da7470566..6755cc7b76 100644 --- a/src/host/ut_host/TextBufferTests.cpp +++ b/src/host/ut_host/TextBufferTests.cpp @@ -665,11 +665,11 @@ void TextBufferTests::TestMixedRgbAndLegacyForeground() VERIFY_ARE_EQUAL(attrA.IsLegacy(), false); VERIFY_ARE_EQUAL(attrB.IsLegacy(), false); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), RGB(64, 128, 255)); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrA), gci.LookupBackgroundColor(si.GetAttributes())); + const auto fgColor = RGB(64, 128, 255); + const auto bgColor = gci.LookupAttributeColors(si.GetAttributes()).second; - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), RGB(64, 128, 255)); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrB), gci.LookupBackgroundColor(si.GetAttributes())); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA), std::make_pair(fgColor, bgColor)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB), std::make_pair(fgColor, bgColor)); wchar_t* reset = L"\x1b[0m"; stateMachine.ProcessString(reset); @@ -709,11 +709,11 @@ void TextBufferTests::TestMixedRgbAndLegacyBackground() VERIFY_ARE_EQUAL(attrA.IsLegacy(), false); VERIFY_ARE_EQUAL(attrB.IsLegacy(), false); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrA), RGB(64, 128, 255)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), gci.LookupForegroundColor(si.GetAttributes())); + const auto bgColor = RGB(64, 128, 255); + const auto fgColor = gci.LookupAttributeColors(si.GetAttributes()).first; - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrB), RGB(64, 128, 255)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), gci.LookupForegroundColor(si.GetAttributes())); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA), std::make_pair(fgColor, bgColor)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB), std::make_pair(fgColor, bgColor)); wchar_t* reset = L"\x1b[0m"; stateMachine.ProcessString(reset); @@ -751,11 +751,11 @@ void TextBufferTests::TestMixedRgbAndLegacyUnderline() VERIFY_ARE_EQUAL(attrA.IsLegacy(), false); VERIFY_ARE_EQUAL(attrB.IsLegacy(), false); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrA), RGB(64, 128, 255)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), gci.LookupForegroundColor(si.GetAttributes())); + const auto bgColor = RGB(64, 128, 255); + const auto fgColor = gci.LookupAttributeColors(si.GetAttributes()).first; - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrB), RGB(64, 128, 255)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), gci.LookupForegroundColor(si.GetAttributes())); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA), std::make_pair(fgColor, bgColor)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB), std::make_pair(fgColor, bgColor)); VERIFY_ARE_EQUAL(attrA.GetLegacyAttributes() & COMMON_LVB_UNDERSCORE, 0); VERIFY_ARE_EQUAL(attrB.GetLegacyAttributes() & COMMON_LVB_UNDERSCORE, COMMON_LVB_UNDERSCORE); @@ -799,8 +799,8 @@ void TextBufferTests::TestMixedRgbAndLegacyBrightness() VERIFY_ARE_EQUAL(attrA.IsLegacy(), false); VERIFY_ARE_EQUAL(attrB.IsLegacy(), false); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), dark_green); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), bright_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA).first, dark_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB).first, bright_green); wchar_t* reset = L"\x1b[0m"; stateMachine.ProcessString(reset); @@ -850,14 +850,14 @@ void TextBufferTests::TestRgbEraseLine() const auto attr0 = attrs[0]; VERIFY_ARE_EQUAL(attr0.IsLegacy(), false); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attr0), RGB(64, 128, 255)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attr0).second, RGB(64, 128, 255)); for (auto i = 1; i < len; i++) { const auto attr = attrs[i]; LOG_ATTR(attr); VERIFY_ARE_EQUAL(attr.IsLegacy(), false); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attr), RGB(128, 128, 255)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attr).second, RGB(128, 128, 255)); } std::wstring reset = L"\x1b[0m"; stateMachine.ProcessString(reset); @@ -908,8 +908,8 @@ void TextBufferTests::TestUnBold() LOG_ATTR(attrA); LOG_ATTR(attrB); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), bright_green); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), dark_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA).first, bright_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB).first, dark_green); std::wstring reset = L"\x1b[0m"; stateMachine.ProcessString(reset); @@ -963,8 +963,8 @@ void TextBufferTests::TestUnBoldRgb() VERIFY_ARE_EQUAL(attrA.IsLegacy(), false); VERIFY_ARE_EQUAL(attrB.IsLegacy(), false); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), bright_green); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), dark_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA).first, bright_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB).first, dark_green); std::wstring reset = L"\x1b[0m"; stateMachine.ProcessString(reset); @@ -1036,28 +1036,22 @@ void TextBufferTests::TestComplexUnBold() VERIFY_ARE_EQUAL(attrE.IsLegacy(), false); VERIFY_ARE_EQUAL(attrF.IsLegacy(), false); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), bright_green); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrA), RGB(1, 2, 3)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA), std::make_pair(bright_green, RGB(1, 2, 3))); VERIFY_IS_TRUE(attrA.IsBold()); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), dark_green); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrB), RGB(1, 2, 3)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB), std::make_pair(dark_green, RGB(1, 2, 3))); VERIFY_IS_FALSE(attrB.IsBold()); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrC), RGB(32, 32, 32)); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrC), RGB(1, 2, 3)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrC), std::make_pair(RGB(32, 32, 32), RGB(1, 2, 3))); VERIFY_IS_FALSE(attrC.IsBold()); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrD), gci.LookupForegroundColor(attrC)); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrD), gci.LookupBackgroundColor(attrC)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrD), gci.LookupAttributeColors(attrC)); VERIFY_IS_TRUE(attrD.IsBold()); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrE), RGB(64, 64, 64)); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrE), RGB(1, 2, 3)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrE), std::make_pair(RGB(64, 64, 64), RGB(1, 2, 3))); VERIFY_IS_TRUE(attrE.IsBold()); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrF), RGB(64, 64, 64)); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrF), RGB(1, 2, 3)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrF), std::make_pair(RGB(64, 64, 64), RGB(1, 2, 3))); VERIFY_IS_FALSE(attrF.IsBold()); std::wstring reset = L"\x1b[0m"; @@ -1109,8 +1103,8 @@ void TextBufferTests::CopyAttrs() LOG_ATTR(attrA); LOG_ATTR(attrB); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), dark_blue); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), dark_magenta); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA).first, dark_blue); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB).first, dark_magenta); } void TextBufferTests::EmptySgrTest() @@ -1127,8 +1121,7 @@ void TextBufferTests::EmptySgrTest() std::wstring reset = L"\x1b[0m"; stateMachine.ProcessString(reset); - const COLORREF defaultFg = gci.LookupForegroundColor(si.GetAttributes()); - const COLORREF defaultBg = gci.LookupBackgroundColor(si.GetAttributes()); + const auto [defaultFg, defaultBg] = gci.LookupAttributeColors(si.GetAttributes()); // Case 1 - // Write '\x1b[0mX\x1b[31mX\x1b[31;m' @@ -1164,14 +1157,11 @@ void TextBufferTests::EmptySgrTest() LOG_ATTR(attrB); LOG_ATTR(attrC); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), defaultFg); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrA), defaultBg); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA), std::make_pair(defaultFg, defaultBg)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), darkRed); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrB), defaultBg); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB), std::make_pair(darkRed, defaultBg)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrC), defaultFg); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrC), defaultBg); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrC), std::make_pair(defaultFg, defaultBg)); stateMachine.ProcessString(reset); } @@ -1191,8 +1181,7 @@ void TextBufferTests::TestReverseReset() std::wstring reset = L"\x1b[0m"; stateMachine.ProcessString(reset); - const COLORREF defaultFg = gci.LookupForegroundColor(si.GetAttributes()); - const COLORREF defaultBg = gci.LookupBackgroundColor(si.GetAttributes()); + const auto [defaultFg, defaultBg] = gci.LookupAttributeColors(si.GetAttributes()); // Case 1 - // Write '\E[42m\E[38;2;128;5;255mX\E[7mX\E[27mX' @@ -1230,14 +1219,11 @@ void TextBufferTests::TestReverseReset() LOG_ATTR(attrB); LOG_ATTR(attrC); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), rgbColor); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrA), dark_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA), std::make_pair(rgbColor, dark_green)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), dark_green); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrB), rgbColor); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB), std::make_pair(dark_green, rgbColor)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrC), rgbColor); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrC), dark_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrC), std::make_pair(rgbColor, dark_green)); stateMachine.ProcessString(reset); } @@ -1259,8 +1245,7 @@ void TextBufferTests::CopyLastAttr() std::wstring reset = L"\x1b[0m"; stateMachine.ProcessString(reset); - const COLORREF defaultFg = gci.LookupForegroundColor(si.GetAttributes()); - const COLORREF defaultBg = gci.LookupBackgroundColor(si.GetAttributes()); + const auto [defaultFg, defaultBg] = gci.LookupAttributeColors(si.GetAttributes()); const COLORREF solFg = RGB(101, 123, 131); const COLORREF solBg = RGB(0, 43, 54); @@ -1348,23 +1333,17 @@ void TextBufferTests::CopyLastAttr() LOG_ATTR(attr3B); LOG_ATTR(attr3C); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attr1A), solFg); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attr1A), solBg); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attr1A), std::make_pair(solFg, solBg)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attr2A), solFg); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attr2A), solBg); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attr2A), std::make_pair(solFg, solBg)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attr2B), solCyan); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attr2B), solBg); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attr2B), std::make_pair(solCyan, solBg)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attr3A), solFg); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attr3A), solBg); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attr3A), std::make_pair(solFg, solBg)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attr3B), solCyan); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attr3B), solBg); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attr3B), std::make_pair(solCyan, solBg)); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attr3C), solFg); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attr3C), solBg); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attr3C), std::make_pair(solFg, solBg)); stateMachine.ProcessString(reset); } @@ -1405,10 +1384,8 @@ void TextBufferTests::TestRgbThenBold() VERIFY_ARE_EQUAL(attrA.IsLegacy(), false); VERIFY_ARE_EQUAL(attrB.IsLegacy(), false); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), foreground); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrA), background); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), foreground); - VERIFY_ARE_EQUAL(gci.LookupBackgroundColor(attrB), background); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA), std::make_pair(foreground, background)); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB), std::make_pair(foreground, background)); wchar_t* reset = L"\x1b[0m"; stateMachine.ProcessString(reset); @@ -1430,8 +1407,7 @@ void TextBufferTests::TestResetClearsBoldness() TextAttribute defaultAttribute; si.SetAttributes(defaultAttribute); - const COLORREF defaultFg = gci.LookupForegroundColor(si.GetAttributes()); - const COLORREF defaultBg = gci.LookupBackgroundColor(si.GetAttributes()); + const auto [defaultFg, defaultBg] = gci.LookupAttributeColors(si.GetAttributes()); const auto dark_green = gci.GetColorTableEntry(2); const auto bright_green = gci.GetColorTableEntry(10); @@ -1460,10 +1436,10 @@ void TextBufferTests::TestResetClearsBoldness() LOG_ATTR(attrC); LOG_ATTR(attrD); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrA), dark_green); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrB), bright_green); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrC), defaultFg); - VERIFY_ARE_EQUAL(gci.LookupForegroundColor(attrD), dark_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA).first, dark_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB).first, bright_green); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrC).first, defaultFg); + VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrD).first, dark_green); VERIFY_IS_FALSE(attrA.IsBold()); VERIFY_IS_TRUE(attrB.IsBold()); diff --git a/src/interactivity/win32/Clipboard.cpp b/src/interactivity/win32/Clipboard.cpp index 6103d57353..ba22fb50ae 100644 --- a/src/interactivity/win32/Clipboard.cpp +++ b/src/interactivity/win32/Clipboard.cpp @@ -207,8 +207,7 @@ void Clipboard::StoreSelectionToClipboard(bool const copyFormatting) const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); const auto& buffer = gci.GetActiveOutputBuffer().GetTextBuffer(); - std::function GetForegroundColor = std::bind(&CONSOLE_INFORMATION::LookupForegroundColor, &gci, std::placeholders::_1); - std::function GetBackgroundColor = std::bind(&CONSOLE_INFORMATION::LookupBackgroundColor, &gci, std::placeholders::_1); + const auto GetAttributeColors = std::bind(&CONSOLE_INFORMATION::LookupAttributeColors, &gci, std::placeholders::_1); bool includeCRLF, trimTrailingWhitespace; if (WI_IsFlagSet(GetKeyState(VK_SHIFT), KEY_PRESSED)) @@ -224,8 +223,7 @@ void Clipboard::StoreSelectionToClipboard(bool const copyFormatting) const auto text = buffer.GetText(includeCRLF, trimTrailingWhitespace, selectionRects, - GetForegroundColor, - GetBackgroundColor); + GetAttributeColors); CopyTextToSystemClipboard(text, copyFormatting); } diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 74e6df2ba5..f382dbd7c0 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -879,13 +879,16 @@ void Renderer::_PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngin const size_t cchLine, const COORD coordTarget) { - const COLORREF rgb = _pData->GetForegroundColor(textAttribute); - // Convert console grid line representations into rendering engine enum representations. IRenderEngine::GridLines lines = Renderer::s_GetGridlines(textAttribute); - - // Draw the lines - LOG_IF_FAILED(pEngine->PaintBufferGridLines(lines, rgb, cchLine, coordTarget)); + // Return early if there are no lines to paint. + if (lines != IRenderEngine::GridLines::None) + { + // Get the current foreground color to render the lines. + const COLORREF rgb = _pData->GetAttributeColors(textAttribute).first; + // Draw the lines + LOG_IF_FAILED(pEngine->PaintBufferGridLines(lines, rgb, cchLine, coordTarget)); + } } // Routine Description: diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 333d4b0997..5d36ff3527 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1631,8 +1631,7 @@ CATCH_RETURN() const bool usingTransparency = _defaultTextBackgroundOpacity != 1.0f; const bool forceOpaqueBG = usingCleartype && !usingTransparency; - const COLORREF colorForeground = pData->GetForegroundColor(textAttributes); - const COLORREF colorBackground = pData->GetBackgroundColor(textAttributes); + const auto [colorForeground, colorBackground] = pData->GetAttributeColors(textAttributes); _foregroundColor = _ColorFFromColorRef(OPACITY_OPAQUE | colorForeground); _backgroundColor = _ColorFFromColorRef((forceOpaqueBG ? OPACITY_OPAQUE : 0) | colorBackground); diff --git a/src/renderer/gdi/paint.cpp b/src/renderer/gdi/paint.cpp index 7682a23252..7654790df0 100644 --- a/src/renderer/gdi/paint.cpp +++ b/src/renderer/gdi/paint.cpp @@ -442,9 +442,6 @@ using namespace Microsoft::Console::Render; // - S_OK or suitable GDI HRESULT error or E_FAIL for GDI errors in functions that don't reliably return a specific error code. [[nodiscard]] HRESULT GdiEngine::PaintBufferGridLines(const GridLines lines, const COLORREF color, const size_t cchLine, const COORD coordTarget) noexcept { - // Return early if there are no lines to paint. - RETURN_HR_IF(S_OK, GridLines::None == lines); - LOG_IF_FAILED(_FlushBufferLines()); // Convert the target from characters to pixels. diff --git a/src/renderer/gdi/state.cpp b/src/renderer/gdi/state.cpp index 10be2c6a5f..65eec1aaa4 100644 --- a/src/renderer/gdi/state.cpp +++ b/src/renderer/gdi/state.cpp @@ -188,8 +188,7 @@ GdiEngine::~GdiEngine() RETURN_HR_IF_NULL(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), _hdcMemoryContext); // Set the colors for painting text - const COLORREF colorForeground = pData->GetForegroundColor(textAttributes); - const COLORREF colorBackground = pData->GetBackgroundColor(textAttributes); + const auto [colorForeground, colorBackground] = pData->GetAttributeColors(textAttributes); if (colorForeground != _lastFg) { diff --git a/src/renderer/inc/IRenderData.hpp b/src/renderer/inc/IRenderData.hpp index c504ae2dbb..5e0f1c77d8 100644 --- a/src/renderer/inc/IRenderData.hpp +++ b/src/renderer/inc/IRenderData.hpp @@ -48,8 +48,7 @@ namespace Microsoft::Console::Render virtual const TextAttribute GetDefaultBrushColors() noexcept = 0; - virtual const COLORREF GetForegroundColor(const TextAttribute& attr) const noexcept = 0; - virtual const COLORREF GetBackgroundColor(const TextAttribute& attr) const noexcept = 0; + virtual std::pair GetAttributeColors(const TextAttribute& attr) const noexcept = 0; virtual COORD GetCursorPosition() const noexcept = 0; virtual bool IsCursorVisible() const noexcept = 0;