Allow ThrottledFunc to work on different types of dispatcher

src/cascadia/TerminalApp/AppLogic.cpp

@@ -203,7 +203,7 @@ namespace winrt::TerminalApp::implementation
         _isElevated = _isUserAdmin();
         _root = winrt::make_self<TerminalPage>();
 
-        _reloadSettings = std::make_shared<ThrottledFuncTrailing<>>(_root->Dispatcher(), std::chrono::milliseconds(100), [weakSelf = get_weak()]() {
+        _reloadSettings = std::make_shared<ThrottledFuncTrailing<>>(winrt::Windows::System::DispatcherQueue::GetForCurrentThread(), std::chrono::milliseconds(100), [weakSelf = get_weak()]() {
             if (auto self{ weakSelf.get() })
             {
                 self->_ReloadSettings();

src/cascadia/TerminalControl/ControlCore.cpp

@@ -23,6 +23,16 @@ using namespace winrt::Windows::Graphics::Display;
 using namespace winrt::Windows::System;
 using namespace winrt::Windows::ApplicationModel::DataTransfer;
 
+// The minimum delay between updates to the scroll bar's values.
+// The updates are throttled to limit power usage.
+constexpr const auto ScrollBarUpdateInterval = std::chrono::milliseconds(8);
+
+// The minimum delay between updating the TSF input control.
+constexpr const auto TsfRedrawInterval = std::chrono::milliseconds(100);
+
+// The minimum delay between updating the locations of regex patterns
+constexpr const auto UpdatePatternLocationsInterval = std::chrono::milliseconds(500);
+
 namespace winrt::Microsoft::Terminal::Control::implementation
 {
     // Helper static function to ensure that all ambiguous-width glyphs are reported as narrow.
@@ -94,6 +104,61 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         auto pfnTerminalTaskbarProgressChanged = std::bind(&ControlCore::_terminalTaskbarProgressChanged, this);
         _terminal->TaskbarProgressChangedCallback(pfnTerminalTaskbarProgressChanged);
 
+        // Get our dispatcher. If we're hosted in-proc with XAML, this will get
+        // us the same dispatcher as TermControl::Dispatcher(). If we're out of
+        // proc, this'll return null. We'll need to instead make a new
+        // DispatcherQueue (on a new thread), so we can use that for throttled
+        // functions.
+        _dispatcher = winrt::Windows::System::DispatcherQueue::GetForCurrentThread();
+        if (!_dispatcher)
+        {
+            auto controller{ winrt::Windows::System::DispatcherQueueController::CreateOnDedicatedThread() };
+            _dispatcher = controller.DispatcherQueue();
+        }
+
+        // A few different events should be throttled, so they don't fire absolutely all the time:
+        // * _tsfTryRedrawCanvas: When the cursor position moves, we need to
+        //   inform TSF, so it can move the canvas for the composition. We
+        //   throttle this so that we're not hopping across the process boundary
+        //   every time that the cursor moves.
+        // * _updatePatternLocations: When there's new output, or we scroll the
+        //   viewport, we should re-check if there are any visible hyperlinks.
+        //   But we don't really need to do this every single time text is
+        //   output, we can limit this update to once every 500ms.
+        // * _updateScrollBar: Same idea as the TSF update - we don't _really_
+        //   need to hop across the process boundary every time text is output.
+        //   We can throttle this to once every 8ms, which will get us out of
+        //   the way of the main output & rendering threads.
+        _tsfTryRedrawCanvas = std::make_shared<ThrottledFuncTrailing<>>(
+            _dispatcher,
+            TsfRedrawInterval,
+            [weakThis = get_weak()]() {
+                if (auto core{ weakThis.get() }; !core->_IsClosing())
+                {
+                    core->_CursorPositionChangedHandlers(*core, nullptr);
+                }
+            });
+
+        _updatePatternLocations = std::make_shared<ThrottledFuncTrailing<>>(
+            _dispatcher,
+            UpdatePatternLocationsInterval,
+            [weakThis = get_weak()]() {
+                if (auto core{ weakThis.get() }; !core->_IsClosing())
+                {
+                    core->UpdatePatternLocations();
+                }
+            });
+
+        _updateScrollBar = std::make_shared<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>>(
+            _dispatcher,
+            ScrollBarUpdateInterval,
+            [weakThis = get_weak()](const auto& update) {
+                if (auto core{ weakThis.get() }; !core->_IsClosing())
+                {
+                    core->_ScrollPositionChangedHandlers(*core, update);
+                }
+            });
+
         UpdateSettings(settings);
     }
 
@@ -1103,15 +1168,28 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // TODO GH#9617: refine locking around pattern tree
         _terminal->ClearPatternTree();
 
-        _ScrollPositionChangedHandlers(*this,
-                                       winrt::make<ScrollPositionChangedArgs>(viewTop,
-                                                                              viewHeight,
-                                                                              bufferSize));
+        // Start the throttled update of our scrollbar.
+        auto update{ winrt::make<ScrollPositionChangedArgs>(viewTop,
+                                                            viewHeight,
+                                                            bufferSize) };
+        if (!_inUnitTests)
+        {
+            _updateScrollBar->Run(update);
+        }
+        else
+        {
+            _ScrollPositionChangedHandlers(*this, update);
+        }
+
+        // Additionally, start the throttled update of where our links are.
+        _updatePatternLocations->Run();
     }
 
     void ControlCore::_terminalCursorPositionChanged()
     {
-        _CursorPositionChangedHandlers(*this, nullptr);
+        // When the buffer's cursor moves, start the throttled func to
+        // eventually dispatch a CursorPositionChanged event.
+        _tsfTryRedrawCanvas->Run();
     }
 
     void ControlCore::_terminalTaskbarProgressChanged()
@@ -1221,8 +1299,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
     void ControlCore::Close()
     {
-        if (!_closing.exchange(true))
+        if (!_IsClosing())
         {
+            _closing = true;
+
             // Stop accepting new output and state changes before we disconnect everything.
             _connection.TerminalOutput(_connectionOutputEventToken);
             _connectionStateChangedRevoker.revoke();
@@ -1400,18 +1480,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         _terminal->Write(hstr);
 
-        // NOTE: We're raising an event here to inform the TermControl that
-        // output has been received, so it can queue up a throttled
-        // UpdatePatternLocations call. In the future, we should have the
-        // _updatePatternLocations ThrottledFunc internal to this class, and
-        // run on this object's dispatcher queue.
-        //
-        // We're not doing that quite yet, because the Core will eventually
-        // be out-of-proc from the UI thread, and won't be able to just use
-        // the UI thread as the dispatcher queue thread.
-        //
-        // See TODO: https://github.com/microsoft/terminal/projects/5#card-50760282
-        _ReceivedOutputHandlers(*this, nullptr);
+        // Start the throttled update of where our hyperlinks are.
+        _updatePatternLocations->Run();
     }
 
 }

src/cascadia/TerminalControl/ControlCore.h

@@ -168,7 +168,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
     private:
         bool _initializedTerminal{ false };
-        std::atomic<bool> _closing{ false };
+        bool _closing{ false };
 
         TerminalConnection::ITerminalConnection _connection{ nullptr };
         event_token _connectionOutputEventToken;
@@ -206,6 +206,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         double _panelHeight{ 0 };
         double _compositionScale{ 0 };
 
+        winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr };
+        std::shared_ptr<ThrottledFuncTrailing<>> _tsfTryRedrawCanvas;
+        std::shared_ptr<ThrottledFuncTrailing<>> _updatePatternLocations;
+        std::shared_ptr<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>> _updateScrollBar;
+
         winrt::fire_and_forget _asyncCloseConnection();
 
         void _setFontSize(int fontSize);
@@ -239,8 +244,24 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void _connectionOutputHandler(const hstring& hstr);
         void _updateHoveredCell(const std::optional<til::point> terminalPosition);
 
+        inline bool _IsClosing() const noexcept
+        {
+#ifndef NDEBUG
+            if (_dispatcher)
+            {
+                // _closing isn't atomic and may only be accessed from the main thread.
+                //
+                // Though, the unit tests don't actually run in TAEF's main
+                // thread, so we don't care when we're running in tests.
+                assert(_inUnitTests || _dispatcher.HasThreadAccess());
+            }
+#endif
+            return _closing;
+        }
+
         friend class ControlUnitTests::ControlCoreTests;
         friend class ControlUnitTests::ControlInteractivityTests;
+        bool _inUnitTests{ false };
     };
 }
 

src/cascadia/TerminalControl/TermControl.cpp

@@ -33,7 +33,8 @@ using namespace winrt::Windows::ApplicationModel::DataTransfer;
 constexpr const auto ScrollBarUpdateInterval = std::chrono::milliseconds(8);
 
 // The minimum delay between updating the TSF input control.
-constexpr const auto TsfRedrawInterval = std::chrono::milliseconds(100);
+// This is already throttled primarily in the ControlCore, with a timeout of 100ms. We're adding another smaller one here, as the (potentially x-proc) call will come in off the UI thread
+constexpr const auto TsfRedrawInterval = std::chrono::milliseconds(8);
 
 // The minimum delay between updating the locations of regex patterns
 constexpr const auto UpdatePatternLocationsInterval = std::chrono::milliseconds(500);
@@ -64,10 +65,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _interactivity = winrt::make<implementation::ControlInteractivity>(settings, connection);
         _core = _interactivity.Core();
 
-        // Use a manual revoker on the output event, so we can immediately stop
-        // worrying about it on destruction.
-        _coreOutputEventToken = _core.ReceivedOutput({ this, &TermControl::_coreReceivedOutput });
-
         // These events might all be triggered by the connection, but that
         // should be drained and closed before we complete destruction. So these
         // are safe.
@@ -104,37 +101,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             }
         });
 
-        // Many of these ThrottledFunc's should be inside ControlCore. However,
-        // currently they depend on the Dispatcher() of the UI thread, which the
-        // Core eventually won't have access to. When we get to
-        // https://github.com/microsoft/terminal/projects/5#card-50760282
-        // then we'll move the applicable ones.
-        //
-        // These four throttled functions are triggered by terminal output and interact with the UI.
+        // Get our dispatcher. This will get us the same dispatcher as
+        // TermControl::Dispatcher().
+        auto dispatcher = winrt::Windows::System::DispatcherQueue::GetForCurrentThread();
+
+        // These three throttled functions are triggered by terminal output and interact with the UI.
         // Since Close() is the point after which we are removed from the UI, but before the
         // destructor has run, we MUST check control->_IsClosing() before actually doing anything.
-        _tsfTryRedrawCanvas = std::make_shared<ThrottledFuncTrailing<>>(
-            Dispatcher(),
-            TsfRedrawInterval,
-            [weakThis = get_weak()]() {
-                if (auto control{ weakThis.get() }; !control->_IsClosing())
-                {
-                    control->TSFInputControl().TryRedrawCanvas();
-                }
-            });
-
-        _updatePatternLocations = std::make_shared<ThrottledFuncTrailing<>>(
-            Dispatcher(),
-            UpdatePatternLocationsInterval,
-            [weakThis = get_weak()]() {
-                if (auto control{ weakThis.get() }; !control->_IsClosing())
-                {
-                    control->_core.UpdatePatternLocations();
-                }
-            });
-
         _playWarningBell = std::make_shared<ThrottledFuncLeading>(
-            Dispatcher(),
+            dispatcher,
             TerminalWarningBellInterval,
             [weakThis = get_weak()]() {
                 if (auto control{ weakThis.get() }; !control->_IsClosing())
@@ -144,7 +119,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             });
 
         _updateScrollBar = std::make_shared<ThrottledFuncTrailing<ScrollBarUpdate>>(
-            Dispatcher(),
+            dispatcher,
             ScrollBarUpdateInterval,
             [weakThis = get_weak()](const auto& update) {
                 if (auto control{ weakThis.get() }; !control->_IsClosing())
@@ -540,7 +515,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         {
             // create a custom automation peer with this code pattern:
             // (https://docs.microsoft.com/en-us/windows/uwp/design/accessibility/custom-automation-peers)
-            if (const auto& interactivityAutoPeer = _interactivity.OnCreateAutomationPeer())
+            if (const auto& interactivityAutoPeer{ _interactivity.OnCreateAutomationPeer() })
             {
                 _automationPeer = winrt::make<implementation::TermControlAutomationPeer>(this, interactivityAutoPeer);
                 return _automationPeer;
@@ -1276,23 +1251,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         CATCH_LOG();
     }
 
-    void TermControl::_coreReceivedOutput(const IInspectable& /*sender*/,
-                                          const IInspectable& /*args*/)
-    {
-        // Queue up a throttled UpdatePatternLocations call. In the future, we
-        // should have the _updatePatternLocations ThrottledFunc internal to
-        // ControlCore, and run on that object's dispatcher queue.
-        //
-        // We're not doing that quite yet, because the Core will eventually
-        // be out-of-proc from the UI thread, and won't be able to just use
-        // the UI thread as the dispatcher queue thread.
-        //
-        // THIS IS CALLED ON EVERY STRING OF TEXT OUTPUT TO THE TERMINAL. Think
-        // twice before adding anything here.
-
-        _updatePatternLocations->Run();
-    }
-
     // Method Description:
     // - Reset the font size of the terminal to its default size.
     // Arguments:
@@ -1330,8 +1288,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _updateScrollBar->ModifyPending([](auto& update) {
             update.newValue.reset();
         });
-
-        _updatePatternLocations->Run();
     }
 
     // Method Description:
@@ -1665,18 +1621,27 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         update.newValue = args.ViewTop();
 
         _updateScrollBar->Run(update);
-        _updatePatternLocations->Run();
     }
 
     // Method Description:
     // - Tells TSFInputControl to redraw the Canvas/TextBlock so it'll update
     //   to be where the current cursor position is.
     // Arguments:
     // - N/A
-    void TermControl::_CursorPositionChanged(const IInspectable& /*sender*/,
-                                             const IInspectable& /*args*/)
-    {
-        _tsfTryRedrawCanvas->Run();
+    winrt::fire_and_forget TermControl::_CursorPositionChanged(const IInspectable& /*sender*/,
+                                                               const IInspectable& /*args*/)
+    {
+        // Prior to GH#10187, this fired a trailing throttled func to update the
+        // TSF canvas only every 100ms. Now, the throttling occurs on the
+        // ControlCore side. If we're told to update the cursor position, we can
+        // just go ahead and do it.
+        // This can come in off the COM thread - hop back to the UI thread.
+        auto weakThis{ get_weak() };
+        co_await resume_foreground(Dispatcher());
+        if (auto control{ weakThis.get() }; !control->_IsClosing())
+        {
+            control->TSFInputControl().TryRedrawCanvas();
+        }
     }
 
     hstring TermControl::Title()
@@ -1730,8 +1695,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         {
             _closing = true;
 
-            _core.ReceivedOutput(_coreOutputEventToken);
             _RestorePointerCursorHandlers(*this, nullptr);
+
             // Disconnect the TSF input control so it doesn't receive EditContext events.
             TSFInputControl().Close();
             _autoScrollTimer.Stop();