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

Automatically enable AdjustIndistinguishableColors if High Contrast mode enabled #17346

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,11 @@ namespace winrt::TerminalApp::implementation
globals.MinimizeToNotificationArea();
}

void AppLogic::SetHighContrast(bool highContrast)
{
_settings.HighContrastMode(highContrast);
}

bool AppLogic::AllowHeadless()
{
if (!_loadedInitialSettings)
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ namespace winrt::TerminalApp::implementation
bool IsolatedMode();
bool AllowHeadless();
bool RequestsTrayIcon();
void SetHighContrast(bool highContrast);

TerminalApp::TerminalWindow CreateNewWindow();

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/AppLogic.idl
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ namespace TerminalApp
Boolean IsolatedMode { get; };
Boolean AllowHeadless { get; };
Boolean RequestsTrayIcon { get; };
void SetHighContrast(Boolean highContrast);

FindTargetWindowResult FindTargetWindow(String[] args);

Expand Down
8 changes: 5 additions & 3 deletions src/cascadia/TerminalApp/TerminalPaneContent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,11 +340,13 @@ namespace winrt::TerminalApp::implementation
RestartTerminalRequested.raise(*this, nullptr);
}

void TerminalPaneContent::UpdateSettings(const CascadiaSettings& /*settings*/)
void TerminalPaneContent::UpdateSettings(const CascadiaSettings& settings)
{
if (const auto& settings{ _cache.TryLookup(_profile) })
if (const auto& cachedSettings{ _cache.TryLookup(_profile) })
{
_control.UpdateControlSettings(settings.DefaultSettings(), settings.UnfocusedSettings());
auto cachedDefaultSettings = cachedSettings.DefaultSettings();
cachedDefaultSettings.HighContrastMode(settings.HighContrastMode());
_control.UpdateControlSettings(cachedDefaultSettings, cachedSettings.UnfocusedSettings());
}
}

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_cellHeight = CSSLengthPercentage::FromString(_settings->CellHeight().c_str());
_runtimeOpacity = std::nullopt;
_runtimeFocusedOpacity = std::nullopt;
_terminal->SetHighContrastInfo(settings.HighContrastMode());

// Manually turn off acrylic if they turn off transparency.
_runtimeUseAcrylic = _settings->Opacity() < 1.0 && _settings->UseAcrylic();
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalControl/ControlSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
CONTROL_SETTINGS(SETTINGS_GEN)
#undef SETTINGS_GEN

WINRT_PROPERTY(bool, HighContrastMode, false);

private:
winrt::com_ptr<ControlAppearance> _unfocusedAppearance{ nullptr };
winrt::com_ptr<ControlAppearance> _focusedAppearance{ nullptr };
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalControl/IControlSettings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,8 @@ namespace Microsoft.Terminal.Control
Boolean UseBackgroundImageForWindow { get; };
Boolean RightClickContextMenu { get; };
Boolean RepositionCursorWithMouse { get; };

// Non-serialized settings
Boolean HighContrastMode;
};
}
3 changes: 0 additions & 3 deletions src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
};
bool _InitializeTerminal(const InitializeReason reason);
winrt::fire_and_forget _restoreInBackground();
void _SetFontSize(int fontSize);
void _TappedHandler(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& e);
void _KeyDownHandler(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::Input::KeyRoutedEventArgs& e);
void _KeyUpHandler(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::Input::KeyRoutedEventArgs& e);
Expand Down Expand Up @@ -349,8 +348,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void _SwapChainSizeChanged(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::SizeChangedEventArgs& e);
void _SwapChainScaleChanged(const Windows::UI::Xaml::Controls::SwapChainPanel& sender, const Windows::Foundation::IInspectable& args);

void _TerminalTabColorChanged(const std::optional<til::color> color);

void _ScrollPositionChanged(const IInspectable& sender, const Control::ScrollPositionChangedArgs& args);

bool _CapturePointer(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& e);
Expand Down
4 changes: 3 additions & 1 deletion src/cascadia/TerminalCore/ICoreAppearance.idl
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ namespace Microsoft.Terminal.Core
{
Never,
Indexed,
Always
Always,
AutomaticIndexed,
AutomaticAlways
};

// TerminalCore declares its own Color struct to avoid depending
Expand Down
22 changes: 21 additions & 1 deletion src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,22 @@ void Terminal::UpdateAppearance(const ICoreAppearance& appearance)
renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBold, appearance.IntenseIsBold());
renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, appearance.IntenseIsBright());

switch (appearance.AdjustIndistinguishableColors())
// If we're in high contrast mode and AIC is set to Automatic Indexed/Always,
// fallback to Indexed/Always respectively.
const auto deducedAIC = [this, &appearance]() {
const auto initialAIC = appearance.AdjustIndistinguishableColors();
switch (initialAIC)
{
case AdjustTextMode::AutomaticIndexed:
return _highContrastMode ? AdjustTextMode::Indexed : AdjustTextMode::Never;
case AdjustTextMode::AutomaticAlways:
return _highContrastMode ? AdjustTextMode::Always : AdjustTextMode::Never;
default:
return initialAIC;
}
}();

switch (deducedAIC)
{
case AdjustTextMode::Always:
renderSettings.SetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors, false);
Expand Down Expand Up @@ -207,6 +222,11 @@ void Terminal::UpdateAppearance(const ICoreAppearance& appearance)
_NotifyScrollEvent();
}

void Terminal::SetHighContrastInfo(bool hc)
{
_highContrastMode = hc;
}

void Terminal::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle)
{
auto& engine = reinterpret_cast<OutputStateMachineEngine&>(_stateMachine->Engine());
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class Microsoft::Terminal::Core::Terminal final :

void UpdateSettings(winrt::Microsoft::Terminal::Core::ICoreSettings settings);
void UpdateAppearance(const winrt::Microsoft::Terminal::Core::ICoreAppearance& appearance);
void SetHighContrastInfo(bool hc);
void SetFontInfo(const FontInfo& fontInfo);
void SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle);
void EraseScrollback();
Expand Down Expand Up @@ -366,6 +367,7 @@ class Microsoft::Terminal::Core::Terminal final :
size_t _hyperlinkPatternId = 0;

std::wstring _workingDirectory;
bool _highContrastMode = false;

// This default fake font value is only used to check if the font is a raster font.
// Otherwise, the font is changed to a real value with the renderer via TriggerFontChange.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,14 @@
<value>Always</value>
<comment>An option to choose from for the "adjust indistinguishable colors" setting. When selected, we will adjust the text colors for visibility.</comment>
</data>
<data name="Profile_AdjustIndistinguishableColorsAutomaticIndexed.Content" xml:space="preserve">
<value>When High Contrast is enabled, only for colors in the color scheme</value>
<comment>An option to choose from for the "adjust indistinguishable colors" setting. When selected, we will adjust the text colors for visibility only when the colors are part of this profile's color scheme's color table if and only if high contrast mode is enabled.</comment>
</data>
<data name="Profile_AdjustIndistinguishableColorsAutomaticAlways.Content" xml:space="preserve">
<value>When High Contrast Mode is enabled, all colors</value>
<comment>An option to choose from for the "adjust indistinguishable colors" setting. When selected, we will adjust the text colors for visibility if and only if high contrast mode is enabled.</comment>
</data>
<data name="Profile_CursorShapeBar.Content" xml:space="preserve">
<value>Bar ( ┃ )</value>
<comment>{Locked="┃"} An option to choose from for the "cursor shape" setting. When selected, the cursor will look like a vertical bar. The character in the parentheses is used to show what it looks like.</comment>
Expand Down
10 changes: 10 additions & 0 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ std::string_view Model::implementation::LoadStringResource(int resourceID)
return { reinterpret_cast<const char*>(ptr), sz };
}

bool CascadiaSettings::HighContrastMode()
{
return _highContrastMode;
}

void CascadiaSettings::HighContrastMode(bool value)
{
_highContrastMode = value;
}

winrt::hstring CascadiaSettings::Hash() const noexcept
{
return _hash;
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
static winrt::hstring ApplicationDisplayName();
static winrt::hstring ApplicationVersion();
static bool IsPortableMode();
static bool HighContrastMode();
static void HighContrastMode(bool value);
static void ExportFile(winrt::hstring path, winrt::hstring content);

CascadiaSettings() noexcept = default;
Expand Down Expand Up @@ -155,6 +157,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
static const std::filesystem::path& _settingsPath();
static const std::filesystem::path& _releaseSettingsPath();
static winrt::hstring _calculateHash(std::string_view settings, const FILETIME& lastWriteTime);
static bool _highContrastMode;

winrt::com_ptr<implementation::Profile> _createNewProfile(const std::wstring_view& name) const;
Model::Profile _getProfileForCommandLine(const winrt::hstring& commandLine) const;
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ namespace Microsoft.Terminal.Settings.Model
static String SettingsPath { get; };
static String DefaultSettingsPath { get; };
static Boolean IsPortableMode { get; };
static Boolean HighContrastMode;

static String ApplicationDisplayName { get; };
static String ApplicationVersion { get; };
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalSettingsModel/TerminalSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

INHERITABLE_SETTING(Model::TerminalSettings, bool, ReloadEnvironmentVariables, true);

WINRT_PROPERTY(bool, HighContrastMode, false);

private:
std::optional<std::array<Microsoft::Terminal::Core::Color, COLOR_TABLE_SIZE>> _ColorTable;
std::span<Microsoft::Terminal::Core::Color> _getColorTableImpl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Core::CursorStyle)
// - Helper for converting a user-specified adjustTextMode value to its corresponding enum
JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Core::AdjustTextMode)
{
JSON_MAPPINGS(3) = {
JSON_MAPPINGS(5) = {
pair_type{ "never", ValueType::Never },
pair_type{ "indexed", ValueType::Indexed },
pair_type{ "always", ValueType::Always },
pair_type{ "automaticIndexed", ValueType::AutomaticIndexed },
pair_type{ "automaticAlways", ValueType::AutomaticAlways },
};

// Override mapping parser to add boolean parsing
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1506,7 +1506,7 @@ void AppHost::_handleSendContent(const winrt::Windows::Foundation::IInspectable&
// Bubble the update settings request up to the emperor. We're being called on
// the Window thread, but the Emperor needs to update the settings on the _main_
// thread.
void AppHost::_requestUpdateSettings()
void AppHost::_requestUpdateSettings(bool highContrastEnabled)
{
UpdateSettingsRequested.raise();
UpdateSettingsRequested.raise(highContrastEnabled);
}
4 changes: 2 additions & 2 deletions src/cascadia/WindowsTerminal/AppHost.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class AppHost : public std::enable_shared_from_this<AppHost>

static void s_DisplayMessageBox(const winrt::TerminalApp::ParseCommandlineResult& message);

til::event<winrt::delegate<void()>> UpdateSettingsRequested;
til::event<winrt::delegate<void(bool)>> UpdateSettingsRequested;

private:
std::unique_ptr<IslandWindow> _window;
Expand Down Expand Up @@ -146,7 +146,7 @@ class AppHost : public std::enable_shared_from_this<AppHost>
void _handleAttach(const winrt::Windows::Foundation::IInspectable& sender,
winrt::Microsoft::Terminal::Remoting::AttachRequest args);

void _requestUpdateSettings();
void _requestUpdateSettings(bool highContrastEnabled);

// Page -> us -> monarch
void _handleReceiveContent(const winrt::Windows::Foundation::IInspectable& sender,
Expand Down
25 changes: 24 additions & 1 deletion src/cascadia/WindowsTerminal/IslandWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,8 @@
// themes, color schemes that might depend on the OS theme
if (param == L"ImmersiveColorSet")
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
{
bool updateSettings = false;

// GH#15732: Don't update the settings, unless the theme
// _actually_ changed. ImmersiveColorSet gets sent more often
// than just on a theme change. It notably gets sent when the PC
Expand All @@ -757,7 +759,28 @@
if (isCurrentlyDark != _currentSystemThemeIsDark)
{
_currentSystemThemeIsDark = isCurrentlyDark;
UpdateSettingsRequested.raise();
updateSettings = true;
}

bool isCurrentlyHighContrast = []() {
HIGHCONTRAST hc = { 0 };

Check failure

Code scanning / check-spelling

Unrecognized Spelling Error

HIGHCONTRAST is not a recognized word. (unrecognized-spelling)
hc.cbSize = sizeof(HIGHCONTRAST);

Check failure

Code scanning / check-spelling

Unrecognized Spelling Error

HIGHCONTRAST is not a recognized word. (unrecognized-spelling)
if (SystemParametersInfo(SPI_GETHIGHCONTRAST, sizeof(hc), &hc, 0))
{
return (bool)(hc.dwFlags & HCF_HIGHCONTRASTON);
}
return false;
}();

if (isCurrentlyHighContrast != _currentHighContrastModeState)
{
_currentHighContrastModeState = isCurrentlyHighContrast;
updateSettings = true;
}

if (updateSettings)
{
UpdateSettingsRequested.raise(_currentHighContrastModeState);
Copy link
Member

Choose a reason for hiding this comment

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

Why does the IslandWindow of all things need to remember whether we were in dark mode or not? Also, why does it result in ReloadSettings when none of the settings changed? An alternative idea:

  • Add a bool CascadiaSettings::ReloadingSystemConfig() function (or similar).
    • It reads the dark-mode-ness and the high-contrast-ness of the OS.
    • It caches the 2 results inside (for instance) GlobalAppSettings.
    • If checks if either of 2 have changed and returns true if they did, false otherwise.
    • The cached values replace any use of IsSystemInDarkTheme() inside the settings code (currently in 2 locations).
  • Add an alternative to ReloadSettings(), e.g. ReloadingSystemConfigRelatedSettings() (lol) and call that in WindowEmperor::_windowRequestUpdateSettings().
  • In ReloadingSystemConfigRelatedSettings() call _settings.ReloadingSystemConfig() and check if it returned true. If it did: Raise a SettingsChanged event, just like we do in ReloadSettings.

}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/WindowsTerminal/IslandWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class IslandWindow :

til::event<winrt::delegate<void()>> WindowMoved;
til::event<winrt::delegate<void(bool)>> WindowVisibilityChanged;
til::event<winrt::delegate<void()>> UpdateSettingsRequested;
til::event<winrt::delegate<void(bool)>> UpdateSettingsRequested;

protected:
void ForceResize()
Expand Down Expand Up @@ -118,6 +118,7 @@ class IslandWindow :
RECT _rcWorkBeforeFullscreen{};
UINT _dpiBeforeFullscreen{ 96 };
bool _currentSystemThemeIsDark{ true };
bool _currentHighContrastModeState{ false };

void _coldInitialize();
void _warmInitialize();
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/WindowsTerminal/WindowEmperor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -850,9 +850,10 @@ winrt::fire_and_forget WindowEmperor::_windowIsQuakeWindowChanged(winrt::Windows
co_await wil::resume_foreground(this->_dispatcher);
_checkWindowsForNotificationIcon();
}
winrt::fire_and_forget WindowEmperor::_windowRequestUpdateSettings()
winrt::fire_and_forget WindowEmperor::_windowRequestUpdateSettings(bool highContrastEnabled)
{
// We MUST be on the main thread to update the settings. We will crash when trying to enumerate fragment extensions otherwise.
co_await wil::resume_foreground(this->_dispatcher);
_app.Logic().SetHighContrastMode();
_app.Logic().ReloadSettings();
}
2 changes: 1 addition & 1 deletion src/cascadia/WindowsTerminal/WindowEmperor.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class WindowEmperor : public std::enable_shared_from_this<WindowEmperor>
void _numberOfWindowsChanged(const winrt::Windows::Foundation::IInspectable&, const winrt::Windows::Foundation::IInspectable&);

winrt::fire_and_forget _windowIsQuakeWindowChanged(winrt::Windows::Foundation::IInspectable sender, winrt::Windows::Foundation::IInspectable args);
winrt::fire_and_forget _windowRequestUpdateSettings();
winrt::fire_and_forget _windowRequestUpdateSettings(bool highContrastEnabled);

void _createMessageWindow();

Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/WindowsTerminal/WindowThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void WindowThread::CreateHost()
_manager,
_peasant);

_UpdateSettingsRequestedToken = _host->UpdateSettingsRequested([this]() { UpdateSettingsRequested.raise(); });
_UpdateSettingsRequestedToken = _host->UpdateSettingsRequested([this](bool highContrastEnabled) { UpdateSettingsRequested.raise(highContrastEnabled); });

winrt::init_apartment(winrt::apartment_type::single_threaded);

Expand Down Expand Up @@ -111,7 +111,7 @@ bool WindowThread::KeepWarm()
// state transitions. In this case, the window is actually being woken up.
if (msg.message == AppHost::WM_REFRIGERATE)
{
_UpdateSettingsRequestedToken = _host->UpdateSettingsRequested([this]() { UpdateSettingsRequested.raise(); });
_UpdateSettingsRequestedToken = _host->UpdateSettingsRequested([this](bool highContrastEnabled) { UpdateSettingsRequested.raise(highContrastEnabled); });
// Re-initialize the host here, on the window thread
_host->Initialize();
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/WindowsTerminal/WindowThread.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class WindowThread : public std::enable_shared_from_this<WindowThread>

uint64_t PeasantID();

til::event<winrt::delegate<void()>> UpdateSettingsRequested;
til::event<winrt::delegate<void(bool)>> UpdateSettingsRequested;

private:
winrt::Microsoft::Terminal::Remoting::Peasant _peasant{ nullptr };
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/inc/ControlProperties.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
X(uint32_t, CursorHeight, DEFAULT_CURSOR_HEIGHT) \
X(bool, IntenseIsBold) \
X(bool, IntenseIsBright, true) \
X(winrt::Microsoft::Terminal::Core::AdjustTextMode, AdjustIndistinguishableColors, winrt::Microsoft::Terminal::Core::AdjustTextMode::Never)
X(winrt::Microsoft::Terminal::Core::AdjustTextMode, AdjustIndistinguishableColors, winrt::Microsoft::Terminal::Core::AdjustTextMode::AutomaticIndexed)

// --------------------------- Control Appearance ---------------------------
// All of these settings are defined in IControlAppearance.
Expand Down
Loading