Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify the handling of alpha values in the color table #11900

Merged
5 commits merged into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/cascadia/LocalTests_SettingsModel/ColorSchemeTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ namespace SettingsModelLocalTests
std::array<COLORREF, COLOR_TABLE_SIZE> expectedCampbellTable;
const auto campbellSpan = gsl::make_span(expectedCampbellTable);
Utils::InitializeColorTable(campbellSpan);
Utils::SetColorTableAlpha(campbellSpan, 0);
Comment on lines 81 to -84
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary, because the COLORREFs are automatically initialized with zero alpha when cast from til::color (which is what happens in the InitializeColorTable method).


for (size_t i = 0; i < expectedCampbellTable.size(); i++)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're no longer including argb.h, the standard RGB macro will create COLORREFs with a zero alpha, which is no different from the previous ARGB usage here.

}

Expand Down
1 change: 0 additions & 1 deletion src/cascadia/LocalTests_SettingsModel/pch.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/LocalTests_TerminalApp/pch.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalControl/ControlInteractivity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

#include "pch.h"
#include "ControlInteractivity.h"
#include <argb.h>
#include <DefaultSettings.h>
#include <unicode.hpp>
#include <Utf16Parser.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 6 additions & 14 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -137,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)
Expand Down Expand Up @@ -186,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() };
Expand Down Expand Up @@ -1226,8 +1225,6 @@ try
const gsl::span<COLORREF> 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()

Expand Down Expand Up @@ -1311,9 +1308,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) };
Expand All @@ -1340,10 +1335,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 };
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/UnitTests_Control/pch.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/UnitTests_Remoting/pch.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/UnitTests_TerminalCore/ScrollTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include "pch.h"
#include <WexTestClass.h>

#include <argb.h>
#include <DefaultSettings.h>

#include "../renderer/inc/DummyRenderTarget.hpp"
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/ut_app/precomp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
26 changes: 0 additions & 26 deletions src/inc/argb.h

This file was deleted.

3 changes: 1 addition & 2 deletions src/renderer/dx/CustomTextRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
16 changes: 0 additions & 16 deletions src/types/inc/colorTable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,4 @@ namespace Microsoft::Console::Utils
gsl::span<const til::color> CampbellColorTable();

std::optional<til::color> 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:
// - <none>
constexpr void SetColorTableAlpha(const gsl::span<COLORREF> table, const BYTE newAlpha) noexcept
{
const auto shiftedAlpha = newAlpha << 24;
for (auto& color : table)
{
WI_UpdateFlagsInMask(color, 0xff000000, shiftedAlpha);
}
}
}