From a48c3cf7d52c657f6a3f1bfcc8195abaaf3b8441 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 24 Jan 2022 03:17:44 +0100 Subject: [PATCH] Fix opacity restore for command palette previews --- .../TerminalApp/ActionPreviewHandlers.cpp | 44 +++++++++++-------- .../TerminalApp/AppActionHandlers.cpp | 2 +- src/cascadia/TerminalControl/ControlCore.cpp | 3 +- src/cascadia/TerminalControl/ControlCore.h | 2 +- src/cascadia/TerminalControl/ControlCore.idl | 2 +- src/cascadia/TerminalControl/TermControl.cpp | 2 +- src/cascadia/TerminalControl/TermControl.h | 2 +- src/cascadia/TerminalControl/TermControl.idl | 2 +- 8 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/cascadia/TerminalApp/ActionPreviewHandlers.cpp b/src/cascadia/TerminalApp/ActionPreviewHandlers.cpp index c43dcdf399a..84aba407f74 100644 --- a/src/cascadia/TerminalApp/ActionPreviewHandlers.cpp +++ b/src/cascadia/TerminalApp/ActionPreviewHandlers.cpp @@ -70,12 +70,14 @@ namespace winrt::TerminalApp::implementation // Apply the reverts in reverse order - If we had multiple previews // stacked on top of each other, then this will ensure the first one in // is the last one out. - for (auto i{ _restorePreviewFuncs.rbegin() }; i < _restorePreviewFuncs.rend(); i++) + const auto cleanup = wil::scope_exit([this]() { + _restorePreviewFuncs.clear(); + }); + + for (const auto& f : _restorePreviewFuncs) { - auto f = *i; f(); } - _restorePreviewFuncs.clear(); } // Method Description: @@ -94,6 +96,8 @@ namespace winrt::TerminalApp::implementation { if (const auto& scheme{ _settings.GlobalSettings().ColorSchemes().TryLookup(args.SchemeName()) }) { + const auto backup = _restorePreviewFuncs.empty(); + _ApplyToActiveControls([&](const auto& control) { // Stash a copy of the current scheme. auto originalScheme{ control.ColorScheme() }; @@ -101,34 +105,38 @@ namespace winrt::TerminalApp::implementation // Apply the new scheme. control.ColorScheme(scheme.ToCoreScheme()); - // Each control will emplace a revert into the - // _restorePreviewFuncs for itself. - _restorePreviewFuncs.emplace_back([=]() { - // On dismiss, restore the original scheme. - control.ColorScheme(originalScheme); - }); + if (backup) + { + // Each control will emplace a revert into the + // _restorePreviewFuncs for itself. + _restorePreviewFuncs.emplace_back([=]() { + // On dismiss, restore the original scheme. + control.ColorScheme(originalScheme); + }); + } }); } } void TerminalPage::_PreviewAdjustOpacity(const Settings::Model::AdjustOpacityArgs& args) { - // Clear the saved preview funcs because we don't need to add a restore each time - // the preview changes, we only need to be able to restore the last one. - _restorePreviewFuncs.clear(); + const auto backup = _restorePreviewFuncs.empty(); _ApplyToActiveControls([&](const auto& control) { // Stash a copy of the original opacity. auto originalOpacity{ control.BackgroundOpacity() }; // Apply the new opacity - control.AdjustOpacity(args.Opacity(), args.Relative()); + control.AdjustOpacity(args.Opacity() / 100.0, args.Relative()); - _restorePreviewFuncs.emplace_back([=]() { - // On dismiss: - // Don't adjust relatively, just set outright. - control.AdjustOpacity(::base::saturated_cast(originalOpacity * 100), false); - }); + if (backup) + { + _restorePreviewFuncs.emplace_back([=]() { + // On dismiss: + // Don't adjust relatively, just set outright. + control.AdjustOpacity(originalOpacity, false); + }); + } }); } diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index 2088ad2c464..e5a8f720290 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -970,7 +970,7 @@ namespace winrt::TerminalApp::implementation if (const auto& realArgs = args.ActionArgs().try_as()) { const auto res = _ApplyToActiveControls([&](auto& control) { - control.AdjustOpacity(realArgs.Opacity(), realArgs.Relative()); + control.AdjustOpacity(realArgs.Opacity() / 100.0, realArgs.Relative()); }); args.Handled(res); } diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index f8e0d436830..a7590024a42 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1718,9 +1718,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation return _settings->HasUnfocusedAppearance(); } - void ControlCore::AdjustOpacity(const int32_t& opacity, const bool& relative) + void ControlCore::AdjustOpacity(const double opacityAdjust, const bool relative) { - const double opacityAdjust = static_cast(opacity) / 100.0; if (relative) { AdjustOpacity(opacityAdjust); diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 7c9a668c3a5..4b4b5b2d5a7 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -166,7 +166,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation static bool IsVintageOpacityAvailable() noexcept; - void AdjustOpacity(const int32_t& opacity, const bool& relative); + void AdjustOpacity(const double opacity, const bool relative); RUNTIME_SETTING(double, Opacity, _settings->Opacity()); RUNTIME_SETTING(bool, UseAcrylic, _settings->UseAcrylic()); diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl index c23461412e2..4feef419afa 100644 --- a/src/cascadia/TerminalControl/ControlCore.idl +++ b/src/cascadia/TerminalControl/ControlCore.idl @@ -98,7 +98,7 @@ namespace Microsoft.Terminal.Control String ReadEntireBuffer(); - void AdjustOpacity(Int32 Opacity, Boolean relative); + void AdjustOpacity(Double Opacity, Boolean relative); event FontSizeChangedEventArgs FontSizeChanged; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index aa6f3567e80..8f6fea7063e 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -2713,7 +2713,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _core.ColorScheme(scheme); } - void TermControl::AdjustOpacity(const int32_t& opacity, const bool& relative) + void TermControl::AdjustOpacity(const double opacity, const bool relative) { _core.AdjustOpacity(opacity, relative); } diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 4c360913b4c..89d31889a5d 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -110,7 +110,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation winrt::Microsoft::Terminal::Core::Scheme ColorScheme() const noexcept; void ColorScheme(const winrt::Microsoft::Terminal::Core::Scheme& scheme) const noexcept; - void AdjustOpacity(const int32_t& opacity, const bool& relative); + void AdjustOpacity(const double opacity, const bool relative); // -------------------------------- WinRT Events --------------------------------- // clang-format off diff --git a/src/cascadia/TerminalControl/TermControl.idl b/src/cascadia/TerminalControl/TermControl.idl index 4862c408b3d..307dd965530 100644 --- a/src/cascadia/TerminalControl/TermControl.idl +++ b/src/cascadia/TerminalControl/TermControl.idl @@ -73,7 +73,7 @@ namespace Microsoft.Terminal.Control String ReadEntireBuffer(); - void AdjustOpacity(Int32 Opacity, Boolean relative); + void AdjustOpacity(Double Opacity, Boolean relative); // You'd think this should just be "Opacity", but UIElement already // defines an "Opacity", which we're actually not setting at all. We're