From 2fed4c4255cc1aa4c608805c8043e3f64a85f0d6 Mon Sep 17 00:00:00 2001
From: PankajBhojwani <pankaj.d.bhoj@gmail.com>
Date: Wed, 2 Jun 2021 11:49:33 -0700
Subject: [PATCH] Cleanup from bell flash PR (#10307)

Just come cleanup I did not manage to get to before #9270 merged.

Specifically:

- We only initialize the animation and timer if we need them
- We don't repeatedly destroy/create the timer

## Validation Steps Performed
It still works
---
 src/cascadia/TerminalControl/TermControl.cpp | 39 ++++++++++++--------
 src/cascadia/TerminalControl/TermControl.h   |  4 +-
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp
index 9cec3b8100d..266b1bd3db7 100644
--- a/src/cascadia/TerminalControl/TermControl.cpp
+++ b/src/cascadia/TerminalControl/TermControl.cpp
@@ -58,8 +58,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _lastAutoScrollUpdateTime{ std::nullopt },
         _cursorTimer{},
         _blinkTimer{},
-        _searchBox{ nullptr },
-        _bellLightAnimation{ Window::Current().Compositor().CreateScalarKeyFrameAnimation() }
+        _searchBox{ nullptr }
     {
         InitializeComponent();
 
@@ -168,11 +167,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _autoScrollTimer.Interval(AutoScrollUpdateInterval);
         _autoScrollTimer.Tick({ this, &TermControl::_UpdateAutoScroll });
 
-        // Add key frames and a duration to our bell light animation
-        _bellLightAnimation.InsertKeyFrame(0.0, 2.0);
-        _bellLightAnimation.InsertKeyFrame(1.0, 1.0);
-        _bellLightAnimation.Duration(winrt::Windows::Foundation::TimeSpan(std::chrono::milliseconds(TerminalWarningBellInterval)));
-
         _ApplyUISettings(_settings);
     }
 
@@ -2386,17 +2380,33 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
     void TermControl::BellLightOn()
     {
+        // Initialize the animation if it does not exist
+        // We only initialize here instead of in the ctor because depending on the bell style setting,
+        // we may never need this animation
+        if (!_bellLightAnimation)
+        {
+            _bellLightAnimation = Window::Current().Compositor().CreateScalarKeyFrameAnimation();
+            // Add key frames and a duration to our bell light animation
+            _bellLightAnimation.InsertKeyFrame(0.0, 2.0);
+            _bellLightAnimation.InsertKeyFrame(1.0, 1.0);
+            _bellLightAnimation.Duration(winrt::Windows::Foundation::TimeSpan(std::chrono::milliseconds(TerminalWarningBellInterval)));
+        }
+
+        // Similar to the animation, only initialize the timer here
+        if (!_bellLightTimer)
+        {
+            _bellLightTimer = {};
+            _bellLightTimer.Interval(std::chrono::milliseconds(TerminalWarningBellInterval));
+            _bellLightTimer.Tick({ get_weak(), &TermControl::_BellLightOff });
+        }
+
         Windows::Foundation::Numerics::float2 zeroSize{ 0, 0 };
         // If the grid has 0 size or if the bell timer is
         // already active, do nothing
-        if (RootGrid().ActualSize() != zeroSize && !_bellLightTimer)
+        if (RootGrid().ActualSize() != zeroSize && !_bellLightTimer.IsEnabled())
         {
             // Start the timer, when the timer ticks we switch off the light
-            DispatcherTimer invertTimer;
-            invertTimer.Interval(std::chrono::milliseconds(TerminalWarningBellInterval));
-            invertTimer.Tick({ get_weak(), &TermControl::_BellLightOff });
-            invertTimer.Start();
-            _bellLightTimer.emplace(std::move(invertTimer));
+            _bellLightTimer.Start();
 
             // Switch on the light and animate the intensity to fade out
             VisualBellLight::SetIsTarget(RootGrid(), true);
@@ -2410,8 +2420,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         if (_bellLightTimer)
         {
             // Stop the timer and switch off the light
-            _bellLightTimer->Stop();
-            _bellLightTimer.reset();
+            _bellLightTimer.Stop();
 
             if (!_closing)
             {
diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h
index 684ac3a1a29..0e09cb903fc 100644
--- a/src/cascadia/TerminalControl/TermControl.h
+++ b/src/cascadia/TerminalControl/TermControl.h
@@ -174,11 +174,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         Windows::UI::Xaml::DispatcherTimer _autoScrollTimer;
         std::optional<std::chrono::high_resolution_clock::time_point> _lastAutoScrollUpdateTime;
 
-        winrt::Windows::UI::Composition::ScalarKeyFrameAnimation _bellLightAnimation;
+        winrt::Windows::UI::Composition::ScalarKeyFrameAnimation _bellLightAnimation{ nullptr };
+        Windows::UI::Xaml::DispatcherTimer _bellLightTimer{ nullptr };
 
         std::optional<Windows::UI::Xaml::DispatcherTimer> _cursorTimer;
         std::optional<Windows::UI::Xaml::DispatcherTimer> _blinkTimer;
-        std::optional<Windows::UI::Xaml::DispatcherTimer> _bellLightTimer;
 
         event_token _coreOutputEventToken;