From 04e3eb10c0bb09a4a003d448a4d7aaea0ca437a5 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 5 Dec 2021 15:12:23 +0000 Subject: [PATCH 1/5] Remove all usage of argb.h --- .../TerminalSettingsTests.cpp | 8 +++--- src/cascadia/LocalTests_SettingsModel/pch.h | 1 - src/cascadia/LocalTests_TerminalApp/pch.h | 1 - .../TerminalControl/ControlInteractivity.cpp | 1 - src/cascadia/TerminalCore/Terminal.cpp | 3 +-- src/cascadia/UnitTests_Control/pch.h | 1 - src/cascadia/UnitTests_Remoting/pch.h | 1 - .../UnitTests_TerminalCore/ScrollTest.cpp | 1 - src/cascadia/ut_app/precomp.h | 1 - src/inc/argb.h | 26 ------------------- 10 files changed, 5 insertions(+), 39 deletions(-) delete mode 100644 src/inc/argb.h diff --git a/src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp b/src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp index d5ced6f3e17..9570edcee76 100644 --- a/src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp @@ -732,11 +732,11 @@ namespace SettingsModelLocalTests const auto terminalSettings4 = createTerminalSettings(activeProfiles.GetAt(4), colorSchemes); const auto terminalSettings5 = createTerminalSettings(activeProfiles.GetAt(5), colorSchemes); - VERIFY_ARE_EQUAL(ARGB(0, 0x12, 0x34, 0x56), terminalSettings0->CursorColor()); // from color scheme + VERIFY_ARE_EQUAL(RGB(0x12, 0x34, 0x56), terminalSettings0->CursorColor()); // from color scheme VERIFY_ARE_EQUAL(DEFAULT_CURSOR_COLOR, terminalSettings1->CursorColor()); // default - VERIFY_ARE_EQUAL(ARGB(0, 0x23, 0x45, 0x67), terminalSettings2->CursorColor()); // from profile (trumps color scheme) - VERIFY_ARE_EQUAL(ARGB(0, 0x34, 0x56, 0x78), terminalSettings3->CursorColor()); // from profile (not set in color scheme) - VERIFY_ARE_EQUAL(ARGB(0, 0x45, 0x67, 0x89), terminalSettings4->CursorColor()); // from profile (no color scheme) + VERIFY_ARE_EQUAL(RGB(0x23, 0x45, 0x67), terminalSettings2->CursorColor()); // from profile (trumps color scheme) + VERIFY_ARE_EQUAL(RGB(0x34, 0x56, 0x78), terminalSettings3->CursorColor()); // from profile (not set in color scheme) + VERIFY_ARE_EQUAL(RGB(0x45, 0x67, 0x89), terminalSettings4->CursorColor()); // from profile (no color scheme) VERIFY_ARE_EQUAL(DEFAULT_CURSOR_COLOR, terminalSettings5->CursorColor()); // default } diff --git a/src/cascadia/LocalTests_SettingsModel/pch.h b/src/cascadia/LocalTests_SettingsModel/pch.h index 5520f953a55..e93c7f6fc0e 100644 --- a/src/cascadia/LocalTests_SettingsModel/pch.h +++ b/src/cascadia/LocalTests_SettingsModel/pch.h @@ -62,7 +62,6 @@ Author(s): #include "til.h" // Common includes for most tests: -#include "../../inc/argb.h" #include "../../inc/conattrs.hpp" #include "../../types/inc/utils.hpp" #include "../../inc/DefaultSettings.h" diff --git a/src/cascadia/LocalTests_TerminalApp/pch.h b/src/cascadia/LocalTests_TerminalApp/pch.h index ab702f8922f..75aabc570b8 100644 --- a/src/cascadia/LocalTests_TerminalApp/pch.h +++ b/src/cascadia/LocalTests_TerminalApp/pch.h @@ -69,7 +69,6 @@ Author(s): #include "til.h" // Common includes for most tests: -#include "../../inc/argb.h" #include "../../inc/conattrs.hpp" #include "../../types/inc/utils.hpp" #include "../../inc/DefaultSettings.h" diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp index d24d443d1f7..70876af7a42 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -3,7 +3,6 @@ #include "pch.h" #include "ControlInteractivity.h" -#include #include #include #include diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 29c613dc000..73ad2a46c4e 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -6,7 +6,6 @@ #include "../../terminal/parser/OutputStateMachineEngine.hpp" #include "TerminalDispatch.hpp" #include "../../inc/unicode.hpp" -#include "../../inc/argb.h" #include "../../types/inc/utils.hpp" #include "../../types/inc/colorTable.hpp" @@ -81,7 +80,7 @@ Terminal::Terminal() : _InitializeColorTable(); _colorTable.at(TextColor::DEFAULT_FOREGROUND) = RGB(255, 255, 255); - _colorTable.at(TextColor::DEFAULT_BACKGROUND) = ARGB(0, 0, 0, 0); + _colorTable.at(TextColor::DEFAULT_BACKGROUND) = RGB(0, 0, 0); _colorTable.at(TextColor::CURSOR_COLOR) = INVALID_COLOR; } diff --git a/src/cascadia/UnitTests_Control/pch.h b/src/cascadia/UnitTests_Control/pch.h index 0dd68050e11..f5aabcccc7f 100644 --- a/src/cascadia/UnitTests_Control/pch.h +++ b/src/cascadia/UnitTests_Control/pch.h @@ -50,7 +50,6 @@ Licensed under the MIT license. #include "ThrottledFunc.h" // Common includes for most tests: -#include "../../inc/argb.h" #include "../../inc/conattrs.hpp" #include "../../types/inc/utils.hpp" #include "../../inc/DefaultSettings.h" diff --git a/src/cascadia/UnitTests_Remoting/pch.h b/src/cascadia/UnitTests_Remoting/pch.h index 457a7b32dd1..4ad6732bf6b 100644 --- a/src/cascadia/UnitTests_Remoting/pch.h +++ b/src/cascadia/UnitTests_Remoting/pch.h @@ -44,7 +44,6 @@ Licensed under the MIT license. #include "til.h" // Common includes for most tests: -#include "../../inc/argb.h" #include "../../inc/conattrs.hpp" #include "../../types/inc/utils.hpp" #include "../../inc/DefaultSettings.h" diff --git a/src/cascadia/UnitTests_TerminalCore/ScrollTest.cpp b/src/cascadia/UnitTests_TerminalCore/ScrollTest.cpp index 7d376234cef..0365f83a2a2 100644 --- a/src/cascadia/UnitTests_TerminalCore/ScrollTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ScrollTest.cpp @@ -4,7 +4,6 @@ #include "pch.h" #include -#include #include #include "../renderer/inc/DummyRenderTarget.hpp" diff --git a/src/cascadia/ut_app/precomp.h b/src/cascadia/ut_app/precomp.h index 1dfe0cb53c2..656a8a84a6e 100644 --- a/src/cascadia/ut_app/precomp.h +++ b/src/cascadia/ut_app/precomp.h @@ -53,7 +53,6 @@ Author(s): #include "til.h" // Common includes for most tests: -#include "../../inc/argb.h" #include "../../inc/conattrs.hpp" #include "../../types/inc/utils.hpp" #include "../../inc/DefaultSettings.h" diff --git a/src/inc/argb.h b/src/inc/argb.h deleted file mode 100644 index d8972b7f18a..00000000000 --- a/src/inc/argb.h +++ /dev/null @@ -1,26 +0,0 @@ -/*++ -Copyright (c) Microsoft Corporation -Licensed under the MIT license. - -Module Name: -- argb.h - -Abstract: -- Replaces the RGB macro with one that fills the highest order byte with 0xff. - We use this for the cascadia project, because it can have colors with an - alpha component. For code that is alpha-aware, include this header to make - RGB() fill the alpha byte. Otherwise, colors made with RGB will be transparent. -Author(s): -- Mike Griese (migrie) Feb 2019 ---*/ -#pragma once - -constexpr COLORREF ARGB(const BYTE a, const BYTE r, const BYTE g, const BYTE b) noexcept -{ - return (a << 24) | (b << 16) | (g << 8) | (r); -} - -#ifdef RGB -#undef RGB -#define RGB(r, g, b) (ARGB(255, (r), (g), (b))) -#endif From 775249dd78d3db14062b0329ed51564cabea4665 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 5 Dec 2021 15:18:51 +0000 Subject: [PATCH 2/5] Drop unnecessary with_alpha calls. --- src/cascadia/TerminalControl/ControlCore.cpp | 2 +- src/cascadia/TerminalCore/Terminal.cpp | 11 +++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 62ddc20244a..f597fab82ef 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1101,7 +1101,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation til::color ControlCore::BackgroundColor() const { - return til::color{ _terminal->GetColorTableEntry(TextColor::DEFAULT_BACKGROUND) }.with_alpha(0xff); + return _terminal->GetColorTableEntry(TextColor::DEFAULT_BACKGROUND); } // Method Description: diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 73ad2a46c4e..b64d08ed0a7 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -185,7 +185,7 @@ 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 const til::color newBackgroundColor{ appearance.DefaultBackground() }; - _colorTable.at(TextColor::DEFAULT_BACKGROUND) = newBackgroundColor.with_alpha(0); + _colorTable.at(TextColor::DEFAULT_BACKGROUND) = newBackgroundColor; const til::color newForegroundColor{ appearance.DefaultForeground() }; _colorTable.at(TextColor::DEFAULT_FOREGROUND) = newForegroundColor; const til::color newCursorColor{ appearance.CursorColor() }; @@ -1310,9 +1310,7 @@ Scheme Terminal::GetColorScheme() const noexcept Scheme s; s.Foreground = til::color{ _colorTable.at(TextColor::DEFAULT_FOREGROUND) }; - // Don't leak the implementation detail that our _defaultBg is stored - // internally without alpha. - s.Background = til::color{ _colorTable.at(TextColor::DEFAULT_BACKGROUND) }.with_alpha(0xff); + s.Background = til::color{ _colorTable.at(TextColor::DEFAULT_BACKGROUND) }; // SelectionBackground is stored in the ControlAppearance s.CursorColor = til::color{ _colorTable.at(TextColor::CURSOR_COLOR) }; @@ -1339,10 +1337,7 @@ Scheme Terminal::GetColorScheme() const noexcept void Terminal::ApplyScheme(const Scheme& colorScheme) { _colorTable.at(TextColor::DEFAULT_FOREGROUND) = til::color{ colorScheme.Foreground }; - // Set the default background as transparent to prevent the - // DX layer from overwriting the background image or acrylic effect - til::color newBackgroundColor{ colorScheme.Background }; - _colorTable.at(TextColor::DEFAULT_BACKGROUND) = newBackgroundColor.with_alpha(0); + _colorTable.at(TextColor::DEFAULT_BACKGROUND) = til::color{ colorScheme.Background }; _colorTable[0] = til::color{ colorScheme.Black }; _colorTable[1] = til::color{ colorScheme.Red }; From 2c3a8ed165a9c9682e8375b3a2d931219865b24e Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 5 Dec 2021 18:08:41 +0000 Subject: [PATCH 3/5] Drop unnecessary OPACITY_OPAQUE. --- src/renderer/dx/CustomTextRenderer.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index b1f7b76387f..a75f14bc2fa 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -348,8 +348,7 @@ try if (!fInvert) { // Make sure to make the cursor opaque - RETURN_IF_FAILED(d2dContext->CreateSolidColorBrush(til::color{ OPACITY_OPAQUE | options.cursorColor }, - &brush)); + RETURN_IF_FAILED(d2dContext->CreateSolidColorBrush(til::color{ options.cursorColor }, &brush)); } else { From 7f7c029356b7076086f412bc42e3c91d51a08ccf Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 5 Dec 2021 19:04:11 +0000 Subject: [PATCH 4/5] Drop more unnecessary with_alpha calls. --- src/cascadia/TerminalControl/TermControl.cpp | 2 +- src/cascadia/TerminalCore/Terminal.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 14324e8565c..4f14aba020e 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -450,7 +450,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void TermControl::_InitializeBackgroundBrush() { auto settings{ _core.Settings() }; - auto bgColor = til::color{ _core.FocusedAppearance().DefaultBackground() }.with_alpha(0xff); + auto bgColor = til::color{ _core.FocusedAppearance().DefaultBackground() }; if (settings.UseAcrylic()) { // See if we've already got an acrylic background brush diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index b64d08ed0a7..0534c1ec50b 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -136,12 +136,12 @@ void Terminal::UpdateSettings(ICoreSettings settings) } else { - _tabColor = til::color{ settings.TabColor().Value() }.with_alpha(0xff); + _tabColor = settings.TabColor().Value(); } if (!_startingTabColor && settings.StartingTabColor()) { - _startingTabColor = til::color{ settings.StartingTabColor().Value() }.with_alpha(0xff); + _startingTabColor = settings.StartingTabColor().Value(); } if (_pfnTabColorChanged) From 3f6591916c9cb06644f4f88db5edcec1ff025246 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 5 Dec 2021 19:13:08 +0000 Subject: [PATCH 5/5] Get rid of the SetColorTableAlpha function. --- .../ColorSchemeTests.cpp | 1 - src/cascadia/TerminalCore/Terminal.cpp | 2 -- src/types/inc/colorTable.hpp | 16 ---------------- 3 files changed, 19 deletions(-) diff --git a/src/cascadia/LocalTests_SettingsModel/ColorSchemeTests.cpp b/src/cascadia/LocalTests_SettingsModel/ColorSchemeTests.cpp index 682bee5f2af..c13bcbdc8b6 100644 --- a/src/cascadia/LocalTests_SettingsModel/ColorSchemeTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/ColorSchemeTests.cpp @@ -81,7 +81,6 @@ namespace SettingsModelLocalTests std::array expectedCampbellTable; const auto campbellSpan = gsl::make_span(expectedCampbellTable); Utils::InitializeColorTable(campbellSpan); - Utils::SetColorTableAlpha(campbellSpan, 0); for (size_t i = 0; i < expectedCampbellTable.size(); i++) { diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 0534c1ec50b..4eaa385e2d4 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -1225,8 +1225,6 @@ try const gsl::span tableView = { _colorTable.data(), _colorTable.size() }; // First set up the basic 256 colors Utils::InitializeColorTable(tableView); - // Then make sure all the values have an alpha of 255 - Utils::SetColorTableAlpha(tableView, 0xff); } CATCH_LOG() diff --git a/src/types/inc/colorTable.hpp b/src/types/inc/colorTable.hpp index 9999c3768dd..f68284aaa01 100644 --- a/src/types/inc/colorTable.hpp +++ b/src/types/inc/colorTable.hpp @@ -16,20 +16,4 @@ namespace Microsoft::Console::Utils gsl::span CampbellColorTable(); std::optional ColorFromXOrgAppColorName(const std::wstring_view wstr) noexcept; - - // Function Description: - // - Fill the alpha byte of the colors in a given color table with the given value. - // Arguments: - // - table: a color table - // - newAlpha: the new value to use as the alpha for all the entries in that table. - // Return Value: - // - - constexpr void SetColorTableAlpha(const gsl::span table, const BYTE newAlpha) noexcept - { - const auto shiftedAlpha = newAlpha << 24; - for (auto& color : table) - { - WI_UpdateFlagsInMask(color, 0xff000000, shiftedAlpha); - } - } }