Skip to content

Commit

Permalink
Re-enable the setting to adjust the colors of indistinguishable text (#…
Browse files Browse the repository at this point in the history
…13343)

## Summary of the Pull Request
Re-enable the setting to adjust indistinguishable text, with some changes:

- We now pre-calculate the nudged values for 'default bright foreground' as well
- When a color table entry is manually set (i.e. via VT sequences) we re-calculate the nudged values

## References
#11095 #12160

## PR Checklist
* [x] Closes #11917
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.

## Validation Steps Performed
Indistinguishable text gets adjusted again
  • Loading branch information
PankajBhojwani authored Jul 14, 2022
1 parent a17f18a commit 2c6cdc2
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/MTSMSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ Author(s):
X(hstring, ColorSchemeName, "colorScheme", L"Campbell") \
X(hstring, BackgroundImagePath, "backgroundImage") \
X(Model::IntenseStyle, IntenseTextStyle, "intenseTextStyle", Model::IntenseStyle::Bright) \
X(bool, AdjustIndistinguishableColors, "adjustIndistinguishableColors", true)
X(bool, AdjustIndistinguishableColors, "adjustIndistinguishableColors", false)

// Intentionally omitted Appearance settings:
// * ForegroundKey, BackgroundKey, SelectionBackgroundKey, CursorColorKey: all optional colors
Expand Down
5 changes: 1 addition & 4 deletions src/features.xml
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,7 @@
<feature>
<name>Feature_AdjustIndistinguishableText</name>
<description>If enabled, the foreground color will, when necessary, be automatically adjusted to make it more visible.</description>
<stage>AlwaysDisabled</stage>
<alwaysEnabledBrandingTokens>
<brandingToken>Dev</brandingToken>
</alwaysEnabledBrandingTokens>
<stage>AlwaysEnabled</stage>
</feature>

<feature>
Expand Down
29 changes: 21 additions & 8 deletions src/renderer/base/RenderSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ using Microsoft::Console::Utils::InitializeColorTable;

static constexpr size_t AdjustedFgIndex{ 16 };
static constexpr size_t AdjustedBgIndex{ 17 };
static constexpr size_t AdjustedBrightFgIndex{ 18 };

RenderSettings::RenderSettings() noexcept
{
Expand Down Expand Up @@ -77,17 +78,21 @@ void RenderSettings::ResetColorTable() noexcept
// color pair to the adjusted foreground for that color pair
void RenderSettings::MakeAdjustedColorArray() noexcept
{
// The color table has 16 colors, but the adjusted color table needs to be 18
// to include the default background and default foreground colors
std::array<COLORREF, 18> colorTableWithDefaults;
// The color table has 16 colors, but the adjusted color table needs to be 19
// to include the default background, default foreground and bright default foreground colors
std::array<COLORREF, 19> colorTableWithDefaults;
std::copy_n(std::begin(_colorTable), 16, std::begin(colorTableWithDefaults));
colorTableWithDefaults[AdjustedFgIndex] = GetColorAlias(ColorAlias::DefaultForeground);
colorTableWithDefaults[AdjustedBgIndex] = GetColorAlias(ColorAlias::DefaultBackground);

for (auto fgIndex = 0; fgIndex < 18; ++fgIndex)
// We need to use TextColor to calculate the bright default fg
TextColor defaultFg;
colorTableWithDefaults[AdjustedBrightFgIndex] = defaultFg.GetColor(_colorTable, GetColorAliasIndex(ColorAlias::DefaultForeground), true);

for (auto fgIndex = 0; fgIndex < 19; ++fgIndex)
{
const auto fg = til::at(colorTableWithDefaults, fgIndex);
for (auto bgIndex = 0; bgIndex < 18; ++bgIndex)
for (auto bgIndex = 0; bgIndex < 19; ++bgIndex)
{
if (fgIndex == bgIndex)
{
Expand Down Expand Up @@ -196,22 +201,30 @@ std::pair<COLORREF, COLORREF> RenderSettings::GetAttributeColors(const TextAttri
if (Feature_AdjustIndistinguishableText::IsEnabled() &&
GetRenderMode(Mode::DistinguishableColors) &&
!dimFg &&
!attr.IsInvisible() &&
(fgTextColor.IsDefault() || fgTextColor.IsLegacy()) &&
(bgTextColor.IsDefault() || bgTextColor.IsLegacy()))
{
const auto bgIndex = bgTextColor.IsDefault() ? AdjustedBgIndex : bgTextColor.GetIndex();
auto fgIndex = fgTextColor.IsDefault() ? AdjustedFgIndex : fgTextColor.GetIndex();

if (fgTextColor.IsIndex16() && (fgIndex < 8) && brightenFg)
if (brightenFg)
{
// There is a special case for intense here - we need to get the bright version of the foreground color
fgIndex += 8;
if (fgTextColor.IsIndex16() && (fgIndex < 8))
{
fgIndex += 8;
}
else if (fgTextColor.IsDefault())
{
fgIndex = AdjustedBrightFgIndex;
}
}

if (swapFgAndBg)
{
const auto fg = _adjustedForegroundColors[fgIndex][bgIndex];
const auto bg = fgTextColor.GetColor(_colorTable, defaultFgIndex);
const auto bg = fgTextColor.GetColor(_colorTable, defaultFgIndex, brightenFg);
return { fg, bg };
}
else
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/inc/RenderSettings.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ namespace Microsoft::Console::Render
til::enumset<Mode> _renderMode{ Mode::BlinkAllowed, Mode::IntenseIsBright };
std::array<COLORREF, TextColor::TABLE_SIZE> _colorTable;
std::array<size_t, static_cast<size_t>(ColorAlias::ENUM_COUNT)> _colorAliasIndices;
std::array<std::array<COLORREF, 18>, 18> _adjustedForegroundColors;
std::array<std::array<COLORREF, 19>, 19> _adjustedForegroundColors;
size_t _blinkCycle = 0;
mutable bool _blinkIsInUse = false;
bool _blinkShouldBeFaint = false;
Expand Down
10 changes: 10 additions & 0 deletions src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2221,6 +2221,11 @@ bool AdaptDispatch::SetClipboard(const std::wstring_view content)
bool AdaptDispatch::SetColorTableEntry(const size_t tableIndex, const DWORD dwColor)
{
_renderSettings.SetColorTableEntry(tableIndex, dwColor);
if (_renderSettings.GetRenderMode(RenderSettings::Mode::DistinguishableColors))
{
// Re-calculate the adjusted colors now that one of the entries has been changed
_renderSettings.MakeAdjustedColorArray();
}

// 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
Expand Down Expand Up @@ -2286,6 +2291,11 @@ bool AdaptDispatch::AssignColor(const DispatchTypes::ColorItem item, const VTInt
case DispatchTypes::ColorItem::NormalText:
_renderSettings.SetColorAliasIndex(ColorAlias::DefaultForeground, fgIndex);
_renderSettings.SetColorAliasIndex(ColorAlias::DefaultBackground, bgIndex);
if (_renderSettings.GetRenderMode(RenderSettings::Mode::DistinguishableColors))
{
// Re-calculate the adjusted colors now that these aliases have been changed
_renderSettings.MakeAdjustedColorArray();
}
break;
case DispatchTypes::ColorItem::WindowFrame:
_renderSettings.SetColorAliasIndex(ColorAlias::FrameForeground, fgIndex);
Expand Down

0 comments on commit 2c6cdc2

Please sign in to comment.