Repository: microsoft/terminal (language: cpp)

Issue:
Opacity is not correctly restored when cancelling the command palette preview
### Windows Terminal version

1.13.10181.0

### Windows build number

10.0.22540.0

### Steps to reproduce

* Select `Set background opacity...` action in the command palette
* Cycle through some options
* Press Escape

### Actual Behavior

![jAL8iT3unYWAyLDw](https://user-images.githubusercontent.com/2256941/150711966-c4314dd1-7169-45a8-beb0-873dbbba5d46.gif)

### Fixed Behavior

![au9Qve43gAqYHbSn](https://user-images.githubusercontent.com/2256941/150712644-0b04963d-ed33-4e20-81b6-3c0864100c8e.gif)


Discussion / hints:

Fix patch:
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<int>(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<AdjustOpacityArgs>())
             {
                 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<double>(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


Test patch: