From 634b6854dcd0bf5e4dc0ee53763bb86389cc607c Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 26 Oct 2021 10:23:26 -0500 Subject: [PATCH] fix a bug with toggling the retro effects from the palette --- src/cascadia/TerminalApp/Pane.cpp | 15 +-------- src/cascadia/TerminalApp/TerminalPage.cpp | 4 --- src/cascadia/TerminalControl/ControlCore.cpp | 5 +++ src/cascadia/TerminalControl/ControlCore.h | 2 ++ src/cascadia/TerminalControl/ControlCore.idl | 4 +-- .../TerminalControl/ControlSettings.h | 13 ++++++++ src/cascadia/TerminalControl/TermControl.cpp | 17 +++++----- .../TerminalSettings.cpp | 32 ------------------- .../TerminalSettingsModel/TerminalSettings.h | 2 -- .../TerminalSettings.idl | 1 - 10 files changed, 32 insertions(+), 63 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 8c25cea00..0343cf138 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1464,20 +1464,7 @@ void Pane::UpdateSettings(const TerminalSettingsCreateResult& settings, const Pr assert(_IsLeaf()); _profile = profile; - // auto controlSettings = _control.Settings().as(); - // // Update the parent of the control's settings object (and not the object itself) so - // // that any overrides made by the control don't get affected by the reload - // controlSettings.SetParent(settings.DefaultSettings()); - // auto unfocusedSettings{ settings.UnfocusedSettings() }; - // if (unfocusedSettings) - // { - // // Note: the unfocused settings needs to be entirely unchanged _except_ we need to - // // set its parent to the settings object that lives in the control. This is because - // // the overrides made by the control live in that settings object, so we want to make - // // sure the unfocused settings inherit from that. - // unfocusedSettings.SetParent(controlSettings); - // } - // // _control.UnfocusedAppearance(unfocusedSettings); + _control.UpdateControlSettings(settings.DefaultSettings(), settings.UnfocusedSettings()); } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index dfe9fc2b9..e40d8b98a 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -2161,11 +2161,7 @@ namespace winrt::TerminalApp::implementation { // Give term control a child of the settings so that any overrides go in the child // This way, when we do a settings reload we just update the parent and the overrides remain - // const auto child = TerminalSettings::CreateWithParent(settings); TermControl term{ settings.DefaultSettings(), settings.UnfocusedSettings(), connection }; - - // term.UnfocusedAppearance(child.UnfocusedSettings()); // It is okay for the unfocused settings to be null - return term; } diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 6c70371af..8d224cda0 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1638,4 +1638,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation _renderer->TriggerRedrawAll(); _BackgroundColorChangedHandlers(*this, nullptr); } + + bool ControlCore::HasUnfocusedAppearance() const + { + return _settings->HasUnfocusedAppearance(); + } } diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 1f9df968f..5c4f0feff 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -67,6 +67,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation }; Control::IControlAppearance FocusedAppearance() const { return *_settings->FocusedAppearance(); }; Control::IControlAppearance UnfocusedAppearance() const { return *_settings->UnfocusedAppearance(); }; + bool HasUnfocusedAppearance() const; + winrt::Microsoft::Terminal::Core::Scheme ColorScheme() const noexcept; void ColorScheme(const winrt::Microsoft::Terminal::Core::Scheme& scheme); diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl index 98ec088b9..2a4287f5a 100644 --- a/src/cascadia/TerminalControl/ControlCore.idl +++ b/src/cascadia/TerminalControl/ControlCore.idl @@ -41,11 +41,12 @@ namespace Microsoft.Terminal.Control Double compositionScale); void UpdateSettings(IControlSettings settings, IControlAppearance appearance); - // void UpdateAppearance(IControlAppearance appearance); void ApplyAppearance(Boolean focused); + IControlSettings Settings { get; }; IControlAppearance FocusedAppearance { get; }; IControlAppearance UnfocusedAppearance { get; }; + Boolean HasUnfocusedAppearance(); UInt64 SwapChainHandle { get; }; @@ -81,7 +82,6 @@ namespace Microsoft.Terminal.Control void BlinkAttributeTick(); void UpdatePatternLocations(); void Search(String text, Boolean goForward, Boolean caseSensitive); - // void SetBackgroundOpacity(Double opacity); Microsoft.Terminal.Core.Color BackgroundColor { get; }; Boolean HasSelection { get; }; diff --git a/src/cascadia/TerminalControl/ControlSettings.h b/src/cascadia/TerminalControl/ControlSettings.h index defba6213..f618b4537 100644 --- a/src/cascadia/TerminalControl/ControlSettings.h +++ b/src/cascadia/TerminalControl/ControlSettings.h @@ -17,6 +17,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation { struct ControlSettings : public winrt::implements { + // Getters and setters for each *Setting member. We're not using + // WINRT_PROPERTY for these, because they actually exist inside the + // _focusedAppearance member. We don't need to reserve another member to + // hold them. #define SETTINGS_GEN(type, name, ...) WINRT_PROPERTY(type, name, __VA_ARGS__); CORE_SETTINGS(SETTINGS_GEN) CONTROL_SETTINGS(SETTINGS_GEN) @@ -25,16 +29,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation private: winrt::com_ptr _unfocusedAppearance{ nullptr }; winrt::com_ptr _focusedAppearance{ nullptr }; + bool _hasUnfocusedAppearance{ false }; public: ControlSettings(const Control::IControlSettings& settings, const Control::IControlAppearance& unfocusedAppearance) { + _hasUnfocusedAppearance = unfocusedAppearance != nullptr; + _focusedAppearance = winrt::make_self(settings); _unfocusedAppearance = unfocusedAppearance ? winrt::make_self(unfocusedAppearance) : _focusedAppearance; + // Copy every value from the passed in settings, into us. #define COPY_SETTING(type, name, ...) _##name = settings.name(); CORE_SETTINGS(COPY_SETTING) CONTROL_SETTINGS(COPY_SETTING) @@ -43,7 +51,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation winrt::com_ptr UnfocusedAppearance() { return _unfocusedAppearance; } winrt::com_ptr FocusedAppearance() { return _focusedAppearance; } + bool HasUnfocusedAppearance() { return _hasUnfocusedAppearance; } + // Getters and setters for each Appearance member. We're not using + // WINRT_PROPERTY for these, because they actually exist inside the + // _focusedAppearance member. We don't need to reserve another member to + // hold them. #define APPEARANCE_GEN(type, name, ...) \ type name() const noexcept { return _focusedAppearance->name(); } \ void name(const type& value) noexcept { _focusedAppearance->name(value); } diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index b37528348..df33e0dcf 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1501,10 +1501,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation _blinkTimer->Start(); } - // Only update the appearance here if an unfocused config exists - - // if an unfocused config does not exist then we never would have switched - // appearances anyway so there's no need to switch back upon gaining focus - if (_core.UnfocusedAppearance()) + // Only update the appearance here if an unfocused config exists - if an + // unfocused config does not exist then we never would have switched + // appearances anyway so there's no need to switch back upon gaining + // focus + if (_core.HasUnfocusedAppearance()) { UpdateAppearance(_core.FocusedAppearance()); } @@ -1548,10 +1549,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Check if there is an unfocused config we should set the appearance to // upon losing focus - // if (_settings->UnfocusedAppearance()) - // { - UpdateAppearance(_core.UnfocusedAppearance()); - // } + if (_core.HasUnfocusedAppearance()) + { + UpdateAppearance(_core.UnfocusedAppearance()); + } } // Method Description: diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp b/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp index 5e4d1eeea..d345ef6cf 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp @@ -206,38 +206,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation _Opacity = appearance.Opacity(); } - // Method Description: - // - Creates a TerminalSettingsCreateResult from a parent TerminalSettingsCreateResult - // - The returned defaultSettings inherits from the parent's defaultSettings, and the - // returned unfocusedSettings inherits from the returned defaultSettings - // - Note that the unfocused settings needs to be entirely unchanged _except_ we need to - // set its parent to the other settings object that we return. This is because the overrides - // made by the control will live in that other settings object, so we want to make - // sure the unfocused settings inherit from that. - // - Another way to think about this is that initially we have UnfocusedSettings inherit - // from DefaultSettings. This function simply adds another TerminalSettings object - // in the middle of these two, so UnfocusedSettings now inherits from the new object - // and the new object inherits from the DefaultSettings. And this new object is what - // the control can put overrides in. - // Arguments: - // - parent: the TerminalSettingsCreateResult that we create a new one from - // Return Value: - // - A TerminalSettingsCreateResult object that contains a defaultSettings that inherits - // from parent's defaultSettings, and contains an unfocusedSettings that inherits from - // its defaultSettings - Model::TerminalSettingsCreateResult TerminalSettings::CreateWithParent(const Model::TerminalSettingsCreateResult& parent) - { - THROW_HR_IF_NULL(E_INVALIDARG, parent); - - auto defaultImpl{ get_self(parent.DefaultSettings()) }; - auto defaultChild = defaultImpl->CreateChild(); - if (parent.UnfocusedSettings()) - { - parent.UnfocusedSettings().SetParent(*defaultChild); - } - return winrt::make(*defaultChild, parent.UnfocusedSettings()); - } - // Method Description: // - Sets our parent to the provided TerminalSettings // Arguments: diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettings.h b/src/cascadia/TerminalSettingsModel/TerminalSettings.h index 4bc757172..f35f0d6a5 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettings.h +++ b/src/cascadia/TerminalSettingsModel/TerminalSettings.h @@ -64,8 +64,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation const Model::NewTerminalArgs& newTerminalArgs, const Control::IKeyBindings& keybindings); - static Model::TerminalSettingsCreateResult CreateWithParent(const Model::TerminalSettingsCreateResult& parent); - Model::TerminalSettings GetParent(); void SetParent(const Model::TerminalSettings& parent); diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettings.idl b/src/cascadia/TerminalSettingsModel/TerminalSettings.idl index 480f897ed..d58ae24f1 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettings.idl +++ b/src/cascadia/TerminalSettingsModel/TerminalSettings.idl @@ -28,7 +28,6 @@ namespace Microsoft.Terminal.Settings.Model static TerminalSettingsCreateResult CreateWithProfile(CascadiaSettings appSettings, Profile profile, Microsoft.Terminal.Control.IKeyBindings keybindings); static TerminalSettingsCreateResult CreateWithNewTerminalArgs(CascadiaSettings appSettings, NewTerminalArgs newTerminalArgs, Microsoft.Terminal.Control.IKeyBindings keybindings); - static TerminalSettingsCreateResult CreateWithParent(TerminalSettingsCreateResult parent); void SetParent(TerminalSettings parent); TerminalSettings GetParent();