From 86a71339b86d2e82f717a3badc12bc28cd44b27f Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Mon, 20 Jun 2022 11:44:43 -0700 Subject: [PATCH 1/8] ignore brighten/invisible --- src/renderer/base/RenderSettings.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/renderer/base/RenderSettings.cpp b/src/renderer/base/RenderSettings.cpp index fab1f7dc10e..74e9f56dc75 100644 --- a/src/renderer/base/RenderSettings.cpp +++ b/src/renderer/base/RenderSettings.cpp @@ -192,22 +192,18 @@ std::pair RenderSettings::GetAttributeColors(const TextAttri const auto swapFgAndBg = attr.IsReverseVideo() ^ GetRenderMode(Mode::ScreenReversed); // We want to nudge the foreground color to make it more perceivable only for the - // default color pairs within the color table + // default color pairs within the color table, and only if there's no additional text attributes if (Feature_AdjustIndistinguishableText::IsEnabled() && GetRenderMode(Mode::DistinguishableColors) && !dimFg && + !brightenFg && + !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) - { - // There is a special case for intense here - we need to get the bright version of the foreground color - fgIndex += 8; - } - if (swapFgAndBg) { const auto fg = _adjustedForegroundColors[fgIndex][bgIndex]; From d86700f786f50430fe609e7407759cdbd76f56c0 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Mon, 20 Jun 2022 15:42:46 -0700 Subject: [PATCH 2/8] enable in feature, default setting is false --- src/cascadia/TerminalSettingsModel/MTSMSettings.h | 2 +- src/features.xml | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/MTSMSettings.h b/src/cascadia/TerminalSettingsModel/MTSMSettings.h index e425bdfd877..02b0e788742 100644 --- a/src/cascadia/TerminalSettingsModel/MTSMSettings.h +++ b/src/cascadia/TerminalSettingsModel/MTSMSettings.h @@ -109,7 +109,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 diff --git a/src/features.xml b/src/features.xml index a8db865ddc7..00ca5a8bea0 100644 --- a/src/features.xml +++ b/src/features.xml @@ -79,10 +79,7 @@ Feature_AdjustIndistinguishableText If enabled, the foreground color will, when necessary, be automatically adjusted to make it more visible. - AlwaysDisabled - - Dev - + AlwaysEnabled From 230c2aa2f484c7af4bd6e99ad512c2609c32ec84 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Tue, 21 Jun 2022 15:19:07 -0700 Subject: [PATCH 3/8] add comment --- src/renderer/base/RenderSettings.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/renderer/base/RenderSettings.cpp b/src/renderer/base/RenderSettings.cpp index 74e9f56dc75..33cbc4c8274 100644 --- a/src/renderer/base/RenderSettings.cpp +++ b/src/renderer/base/RenderSettings.cpp @@ -193,6 +193,13 @@ std::pair RenderSettings::GetAttributeColors(const TextAttri // We want to nudge the foreground color to make it more perceivable only for the // default color pairs within the color table, and only if there's no additional text attributes + + // The reason we don't want to nudge when there's additional attributes is because of certain + // interactions like "bright" + "reverse". We cannot nudge after reversing because we, as of now, + // have no easy way to calculate what the bright background is when the background is default. We + // also cannot nudge before reversing because then we are reversing the resultant background rather + // than the resultant foreground, and this can lead to weird situations where certain regions of + // text have different backgrounds. if (Feature_AdjustIndistinguishableText::IsEnabled() && GetRenderMode(Mode::DistinguishableColors) && !dimFg && From 39ee965eaf8b33cd58ad78c7c0f9f700430776a3 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 24 Jun 2022 15:02:45 -0700 Subject: [PATCH 4/8] fixes --- .../TerminalSettingsModel/MTSMSettings.h | 2 +- src/renderer/base/RenderSettings.cpp | 27 +++++++++++++++---- src/renderer/inc/RenderSettings.hpp | 2 +- src/terminal/adapter/adaptDispatch.cpp | 5 ++++ 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/MTSMSettings.h b/src/cascadia/TerminalSettingsModel/MTSMSettings.h index 02b0e788742..e425bdfd877 100644 --- a/src/cascadia/TerminalSettingsModel/MTSMSettings.h +++ b/src/cascadia/TerminalSettingsModel/MTSMSettings.h @@ -109,7 +109,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", false) + X(bool, AdjustIndistinguishableColors, "adjustIndistinguishableColors", true) // Intentionally omitted Appearance settings: // * ForegroundKey, BackgroundKey, SelectionBackgroundKey, CursorColorKey: all optional colors diff --git a/src/renderer/base/RenderSettings.cpp b/src/renderer/base/RenderSettings.cpp index 33cbc4c8274..8cc67e1c1d0 100644 --- a/src/renderer/base/RenderSettings.cpp +++ b/src/renderer/base/RenderSettings.cpp @@ -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 { @@ -79,15 +80,19 @@ 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 colorTableWithDefaults; + std::array 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) { @@ -203,7 +208,6 @@ std::pair RenderSettings::GetAttributeColors(const TextAttri if (Feature_AdjustIndistinguishableText::IsEnabled() && GetRenderMode(Mode::DistinguishableColors) && !dimFg && - !brightenFg && !attr.IsInvisible() && (fgTextColor.IsDefault() || fgTextColor.IsLegacy()) && (bgTextColor.IsDefault() || bgTextColor.IsLegacy())) @@ -211,10 +215,23 @@ std::pair RenderSettings::GetAttributeColors(const TextAttri const auto bgIndex = bgTextColor.IsDefault() ? AdjustedBgIndex : bgTextColor.GetIndex(); auto fgIndex = fgTextColor.IsDefault() ? AdjustedFgIndex : fgTextColor.GetIndex(); + if (brightenFg) + { + // There is a special case for intense here - we need to get the bright version of the foreground color + 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 diff --git a/src/renderer/inc/RenderSettings.hpp b/src/renderer/inc/RenderSettings.hpp index 110075b2969..a201fad263d 100644 --- a/src/renderer/inc/RenderSettings.hpp +++ b/src/renderer/inc/RenderSettings.hpp @@ -47,7 +47,7 @@ namespace Microsoft::Console::Render til::enumset _renderMode{ Mode::BlinkAllowed, Mode::IntenseIsBright }; std::array _colorTable; std::array(ColorAlias::ENUM_COUNT)> _colorAliasIndices; - std::array, 18> _adjustedForegroundColors; + std::array, 19> _adjustedForegroundColors; size_t _blinkCycle = 0; mutable bool _blinkIsInUse = false; bool _blinkShouldBeFaint = false; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 6b2a7488ad7..a241383ad27 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -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 From 38f34010d29164b21aebaef85cfcb3e45f95d20e Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 24 Jun 2022 15:03:32 -0700 Subject: [PATCH 5/8] fix comment --- src/renderer/base/RenderSettings.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/renderer/base/RenderSettings.cpp b/src/renderer/base/RenderSettings.cpp index 8cc67e1c1d0..59e500f263b 100644 --- a/src/renderer/base/RenderSettings.cpp +++ b/src/renderer/base/RenderSettings.cpp @@ -197,14 +197,7 @@ std::pair RenderSettings::GetAttributeColors(const TextAttri const auto swapFgAndBg = attr.IsReverseVideo() ^ GetRenderMode(Mode::ScreenReversed); // We want to nudge the foreground color to make it more perceivable only for the - // default color pairs within the color table, and only if there's no additional text attributes - - // The reason we don't want to nudge when there's additional attributes is because of certain - // interactions like "bright" + "reverse". We cannot nudge after reversing because we, as of now, - // have no easy way to calculate what the bright background is when the background is default. We - // also cannot nudge before reversing because then we are reversing the resultant background rather - // than the resultant foreground, and this can lead to weird situations where certain regions of - // text have different backgrounds. + // default color pairs within the color table if (Feature_AdjustIndistinguishableText::IsEnabled() && GetRenderMode(Mode::DistinguishableColors) && !dimFg && From 7a843d73adff786911637e5fb866653512151010 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 24 Jun 2022 15:04:42 -0700 Subject: [PATCH 6/8] fix other comment --- src/renderer/base/RenderSettings.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderer/base/RenderSettings.cpp b/src/renderer/base/RenderSettings.cpp index 59e500f263b..1e3aaed19c4 100644 --- a/src/renderer/base/RenderSettings.cpp +++ b/src/renderer/base/RenderSettings.cpp @@ -78,8 +78,8 @@ 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 + // 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 colorTableWithDefaults; std::copy_n(std::begin(_colorTable), 16, std::begin(colorTableWithDefaults)); colorTableWithDefaults[AdjustedFgIndex] = GetColorAlias(ColorAlias::DefaultForeground); From 35c80693881a7b7303a595c3193234f48bea76ab Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Wed, 29 Jun 2022 13:50:44 -0700 Subject: [PATCH 7/8] back to default off --- src/cascadia/TerminalSettingsModel/MTSMSettings.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalSettingsModel/MTSMSettings.h b/src/cascadia/TerminalSettingsModel/MTSMSettings.h index e425bdfd877..02b0e788742 100644 --- a/src/cascadia/TerminalSettingsModel/MTSMSettings.h +++ b/src/cascadia/TerminalSettingsModel/MTSMSettings.h @@ -109,7 +109,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 From d69e1be14404350cfb412d9624a6c151920e1610 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Wed, 29 Jun 2022 13:54:01 -0700 Subject: [PATCH 8/8] assign color --- src/terminal/adapter/adaptDispatch.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index a241383ad27..3404f3b3c25 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -2291,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);