diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 53c3fde491..0563adbd7a 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -135,12 +135,19 @@ namespace winrt::TerminalApp::implementation _isElevated = ::Microsoft::Console::Utils::IsRunningElevated(); _canDragDrop = ::Microsoft::Console::Utils::CanUwpDragDrop(); - _reloadSettings = std::make_shared>(winrt::Windows::System::DispatcherQueue::GetForCurrentThread(), std::chrono::milliseconds(100), [weakSelf = get_weak()]() { - if (auto self{ weakSelf.get() }) - { - self->ReloadSettings(); - } - }); + _reloadSettings = std::make_shared>( + DispatcherQueue::GetForCurrentThread(), + til::throttled_func_options{ + .delay = std::chrono::milliseconds{ 100 }, + .debounce = true, + .trailing = true, + }, + [weakSelf = get_weak()]() { + if (auto self{ weakSelf.get() }) + { + self->ReloadSettings(); + } + }); _languageProfileNotifier = winrt::make_self([this]() { _reloadSettings->Run(); diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h index 0b76939992..f992561632 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -64,7 +64,7 @@ namespace winrt::TerminalApp::implementation bool _hasSettingsStartupActions{ false }; ::TerminalApp::AppCommandlineArgs _settingsAppArgs; - std::shared_ptr> _reloadSettings; + std::shared_ptr> _reloadSettings; std::vector _warnings{}; diff --git a/src/cascadia/TerminalApp/MinMaxCloseControl.cpp b/src/cascadia/TerminalApp/MinMaxCloseControl.cpp index f4db97176e..541f7351ae 100644 --- a/src/cascadia/TerminalApp/MinMaxCloseControl.cpp +++ b/src/cascadia/TerminalApp/MinMaxCloseControl.cpp @@ -35,17 +35,23 @@ namespace winrt::TerminalApp::implementation // (which should be the default, see: // https://docs.microsoft.com/en-us/windows/win32/api/winuser/ns-winuser-trackmouseevent#remarks) unsigned int hoverTimeoutMillis{ 400 }; - LOG_IF_WIN32_BOOL_FALSE(SystemParametersInfoW(SPI_GETMOUSEHOVERTIME, 0, &hoverTimeoutMillis, 0)); - const auto toolTipInterval = std::chrono::milliseconds(hoverTimeoutMillis); + if (FAILED(SystemParametersInfoW(SPI_GETMOUSEHOVERTIME, 0, &hoverTimeoutMillis, 0))) + { + hoverTimeoutMillis = 400; + } // Create a ThrottledFunc for opening the tooltip after the hover // timeout. If we hover another button, we should make sure to call // Run() with the new button. Calling `_displayToolTip.Run(nullptr)`, // which will cause us to not display a tooltip, which is used when we // leave the control entirely. - _displayToolTip = std::make_shared>( + _displayToolTip = std::make_shared>( dispatcher, - toolTipInterval, + til::throttled_func_options{ + .delay = std::chrono::milliseconds{ hoverTimeoutMillis }, + .debounce = true, + .trailing = true, + }, [weakThis = get_weak()](Controls::Button button) { // If we provide a button, then open the tooltip on that button. // We can "dismiss" this throttled func by calling it with null, diff --git a/src/cascadia/TerminalApp/MinMaxCloseControl.h b/src/cascadia/TerminalApp/MinMaxCloseControl.h index 5bda6974f4..4a53674174 100644 --- a/src/cascadia/TerminalApp/MinMaxCloseControl.h +++ b/src/cascadia/TerminalApp/MinMaxCloseControl.h @@ -32,7 +32,7 @@ namespace winrt::TerminalApp::implementation til::typed_event MaximizeClick; til::typed_event CloseClick; - std::shared_ptr> _displayToolTip{ nullptr }; + std::shared_ptr> _displayToolTip{ nullptr }; std::optional _lastPressedButton{ std::nullopt }; }; } diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index b08842e3f2..bd86dcc009 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -1035,27 +1035,49 @@ namespace winrt::TerminalApp::implementation void Tab::_AttachEventHandlersToContent(const uint32_t paneId, const TerminalApp::IPaneContent& content) { auto weakThis{ get_weak() }; - auto dispatcher = TabViewItem().Dispatcher(); + auto dispatcher = DispatcherQueue::GetForCurrentThread(); ContentEventTokens events{}; + auto throttledTitleChanged = std::make_shared>( + dispatcher, + til::throttled_func_options{ + .delay = std::chrono::milliseconds{ 200 }, + .leading = true, + .trailing = true, + }, + [weakThis]() { + if (const auto tab = weakThis.get()) + { + tab->UpdateTitle(); + } + }); + events.TitleChanged = content.TitleChanged( winrt::auto_revoke, - [dispatcher, weakThis](auto&&, auto&&) -> safe_void_coroutine { - // The lambda lives in the `std::function`-style container owned by `control`. That is, when the - // `control` gets destroyed the lambda struct also gets destroyed. In other words, we need to - // copy `weakThis` onto the stack, because that's the only thing that gets captured in coroutines. - // See: https://devblogs.microsoft.com/oldnewthing/20211103-00/?p=105870 - const auto weakThisCopy = weakThis; - co_await wil::resume_foreground(dispatcher); - // Check if Tab's lifetime has expired - if (auto tab{ weakThisCopy.get() }) + [func = std::move(throttledTitleChanged)](auto&&, auto&&) { + func->Run(); + }); + + auto throttledTaskbarProgressChanged = std::make_shared>( + dispatcher, + til::throttled_func_options{ + .delay = std::chrono::milliseconds{ 200 }, + .leading = true, + .trailing = true, + }, + [weakThis]() { + if (const auto tab = weakThis.get()) { - // The title of the control changed, but not necessarily the title of the tab. - // Set the tab's text to the active panes' text. - tab->UpdateTitle(); + tab->_UpdateProgressState(); } }); + events.TaskbarProgressChanged = content.TaskbarProgressChanged( + winrt::auto_revoke, + [func = std::move(throttledTaskbarProgressChanged)](auto&&, auto&&) { + func->Run(); + }); + events.TabColorChanged = content.TabColorChanged( winrt::auto_revoke, [dispatcher, weakThis](auto&&, auto&&) -> safe_void_coroutine { @@ -1071,18 +1093,6 @@ namespace winrt::TerminalApp::implementation } }); - events.TaskbarProgressChanged = content.TaskbarProgressChanged( - winrt::auto_revoke, - [dispatcher, weakThis](auto&&, auto&&) -> safe_void_coroutine { - const auto weakThisCopy = weakThis; - co_await wil::resume_foreground(dispatcher); - // Check if Tab's lifetime has expired - if (auto tab{ weakThisCopy.get() }) - { - tab->_UpdateProgressState(); - } - }); - events.ConnectionStateChanged = content.ConnectionStateChanged( winrt::auto_revoke, [dispatcher, weakThis](auto&&, auto&&) -> safe_void_coroutine { diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 7ee4649801..dc689e906d 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -4876,8 +4876,6 @@ namespace winrt::TerminalApp::implementation safe_void_coroutine TerminalPage::_ControlCompletionsChangedHandler(const IInspectable sender, const CompletionsChangedEventArgs args) { - // This will come in on a background (not-UI, not output) thread. - // This won't even get hit if the velocity flag is disabled - we gate // registering for the event based off of // Feature_ShellCompletions::IsEnabled back in _RegisterTerminalEvents diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 6a4147637d..318f65a2a7 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -174,8 +174,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation // // NOTE: Calling UpdatePatternLocations from a background // thread is a workaround for us to hit GH#12607 less often. - shared->outputIdle = std::make_unique>( - std::chrono::milliseconds{ 100 }, + shared->outputIdle = std::make_unique>( + til::throttled_func_options{ + .delay = std::chrono::milliseconds{ 100 }, + .debounce = true, + .trailing = true, + }, [this, weakThis = get_weak(), dispatcher = _dispatcher]() { dispatcher.TryEnqueue(DispatcherQueuePriority::Normal, [weakThis]() { if (const auto self = weakThis.get(); self && !self->_IsClosing()) @@ -195,8 +199,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation // If you rapidly show/hide Windows Terminal, something about GotFocus()/LostFocus() gets broken. // We'll then receive easily 10+ such calls from WinUI the next time the application is shown. - shared->focusChanged = std::make_unique>( - std::chrono::milliseconds{ 25 }, + shared->focusChanged = std::make_unique>( + til::throttled_func_options{ + .delay = std::chrono::milliseconds{ 25 }, + .debounce = true, + .trailing = true, + }, [this](const bool focused) { // Theoretically `debounced_func_trailing` should call `WaitForThreadpoolTimerCallbacks()` // with cancel=true on destruction, which should ensure that our use of `this` here is safe. @@ -204,9 +212,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation }); // Scrollbar updates are also expensive (XAML), so we'll throttle them as well. - shared->updateScrollBar = std::make_shared>( + shared->updateScrollBar = std::make_shared>( _dispatcher, - std::chrono::milliseconds{ 8 }, + til::throttled_func_options{ + .delay = std::chrono::milliseconds{ 8 }, + .trailing = true, + }, [weakThis = get_weak()](const auto& update) { if (auto core{ weakThis.get() }; core && !core->_IsClosing()) { @@ -2757,15 +2768,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation } } - safe_void_coroutine ControlCore::_terminalCompletionsChanged(std::wstring_view menuJson, - unsigned int replaceLength) + void ControlCore::_terminalCompletionsChanged(std::wstring_view menuJson, unsigned int replaceLength) { - auto args = winrt::make_self(winrt::hstring{ menuJson }, - replaceLength); - - co_await winrt::resume_background(); - - CompletionsChanged.raise(*this, *args); + CompletionsChanged.raise(*this, winrt::make(winrt::hstring{ menuJson }, replaceLength)); } // Select the region of text between [s.start, s.end), in buffer space diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 2453670d3f..492916040c 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -307,9 +307,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation private: struct SharedState { - std::unique_ptr> outputIdle; - std::unique_ptr> focusChanged; - std::shared_ptr> updateScrollBar; + std::unique_ptr> outputIdle; + std::unique_ptr> focusChanged; + std::shared_ptr> updateScrollBar; }; void _setupDispatcherAndCallbacks(); @@ -339,7 +339,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void _terminalSearchMissingCommand(std::wstring_view missingCommand, const til::CoordType& bufferRow); void _terminalWindowSizeChanged(int32_t width, int32_t height); - safe_void_coroutine _terminalCompletionsChanged(std::wstring_view menuJson, unsigned int replaceLength); + void _terminalCompletionsChanged(std::wstring_view menuJson, unsigned int replaceLength); #pragma endregion #pragma region RendererCallbacks diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index ea8446f04e..0e6f6e8e48 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -358,9 +358,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation // 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. - _playWarningBell = std::make_shared( + _playWarningBell = std::make_shared>( dispatcher, - TerminalWarningBellInterval, + til::throttled_func_options{ + .delay = TerminalWarningBellInterval, + .leading = true, + }, [weakThis = get_weak()]() { if (auto control{ weakThis.get() }; control && !control->_IsClosing()) { @@ -368,9 +371,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation } }); - _updateScrollBar = std::make_shared>( + _updateScrollBar = std::make_shared>( dispatcher, - ScrollBarUpdateInterval, + til::throttled_func_options{ + .delay = ScrollBarUpdateInterval, + .trailing = true, + }, [weakThis = get_weak()](const auto& update) { if (auto control{ weakThis.get() }; control && !control->_IsClosing()) { diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 2f9dad9784..fea4ec2f4a 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -284,7 +284,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool _quickFixesAvailable{ false }; til::CoordType _quickFixBufferPos{}; - std::shared_ptr _playWarningBell; + std::shared_ptr> _playWarningBell; struct ScrollBarUpdate { @@ -294,7 +294,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation double newViewportSize; }; - std::shared_ptr> _updateScrollBar; + std::shared_ptr> _updateScrollBar; bool _isInternalScrollBarUpdate; diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp index cdf1e851ae..bbdd0a5aae 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp @@ -66,9 +66,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { if (!_updatePreviewControl) { - _updatePreviewControl = std::make_shared>( + _updatePreviewControl = std::make_shared>( winrt::Windows::System::DispatcherQueue::GetForCurrentThread(), - std::chrono::milliseconds{ 100 }, + til::throttled_func_options{ + .delay = std::chrono::milliseconds{ 100 }, + .debounce = true, + .trailing = true, + }, [this]() { const auto settings = _Profile.TermSettings(); _previewConnection->DisplayPowerlineGlyphs(_Profile.DefaultAppearance().HasPowerlineCharacters()); diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h index 27c8e455a5..4e52248f43 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h @@ -33,7 +33,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation winrt::com_ptr _previewConnection{ nullptr }; Microsoft::Terminal::Control::TermControl _previewControl{ nullptr }; - std::shared_ptr> _updatePreviewControl; + std::shared_ptr> _updatePreviewControl; Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _ViewModelChangedRevoker; Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _AppearanceViewModelChangedRevoker; Editor::IHostedInWindow _windowRoot; diff --git a/src/cascadia/TerminalSettingsModel/ApplicationState.cpp b/src/cascadia/TerminalSettingsModel/ApplicationState.cpp index 05169f06f5..ed2f69d883 100644 --- a/src/cascadia/TerminalSettingsModel/ApplicationState.cpp +++ b/src/cascadia/TerminalSettingsModel/ApplicationState.cpp @@ -96,7 +96,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation ApplicationState::ApplicationState(const std::filesystem::path& stateRoot) noexcept : _sharedPath{ stateRoot / stateFileName }, _elevatedPath{ stateRoot / elevatedStateFileName }, - _throttler{ std::chrono::seconds(1), [this]() { _write(); } } + _throttler{ + til::throttled_func_options{ + .delay = std::chrono::seconds{ 1 }, + .debounce = true, + .trailing = true, + }, + [this]() { _write(); } + } { _read(); } diff --git a/src/cascadia/TerminalSettingsModel/ApplicationState.h b/src/cascadia/TerminalSettingsModel/ApplicationState.h index 881383023e..999ab0c0fc 100644 --- a/src/cascadia/TerminalSettingsModel/ApplicationState.h +++ b/src/cascadia/TerminalSettingsModel/ApplicationState.h @@ -91,7 +91,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation til::shared_mutex _state; std::filesystem::path _sharedPath; std::filesystem::path _elevatedPath; - til::throttled_func_trailing<> _throttler; + til::throttled_func<> _throttler; void _write() const noexcept; void _read() const noexcept; diff --git a/src/cascadia/WinRTUtils/inc/ThrottledFunc.h b/src/cascadia/WinRTUtils/inc/ThrottledFunc.h index 94c42b3362..3bcf1d87f9 100644 --- a/src/cascadia/WinRTUtils/inc/ThrottledFunc.h +++ b/src/cascadia/WinRTUtils/inc/ThrottledFunc.h @@ -3,40 +3,44 @@ #pragma once -#include "til/throttled_func.h" +#include // ThrottledFunc is a copy of til::throttled_func, // specialized for the use with a WinRT Dispatcher. -template -class ThrottledFunc : public std::enable_shared_from_this> +template +class ThrottledFunc : public std::enable_shared_from_this> { public: - using filetime_duration = std::chrono::duration>; + using filetime_duration = til::throttled_func_options::filetime_duration; using function = std::function; - // Throttles invocations to the given `func` to not occur more often than `delay`. + // Throttles invocations to the given `func` to not occur more often than specified in options. // - // If this is a: - // * ThrottledFuncLeading: `func` will be invoked immediately and - // further invocations prevented until `delay` time has passed. - // * ThrottledFuncTrailing: On the first invocation a timer of `delay` time will - // be started. After the timer has expired `func` will be invoked just once. + // Options: + // * delay: The minimum time between invocations + // * leading: If true, `func` will be invoked immediately on first call + // * trailing: If true, `func` will be invoked after the delay + // * debounce: If true, resets the timer on each call // - // After `func` was invoked the state is reset and this cycle is repeated again. - ThrottledFunc( - winrt::Windows::System::DispatcherQueue dispatcher, - filetime_duration delay, - function func) : + // At least one of leading or trailing must be true. + ThrottledFunc(winrt::Windows::System::DispatcherQueue dispatcher, til::throttled_func_options opts, function func) : _dispatcher{ std::move(dispatcher) }, _func{ std::move(func) }, - _timer{ _create_timer() } + _timer{ _create_timer() }, + _debounce{ opts.debounce }, + _leading{ opts.leading }, + _trailing{ opts.trailing } { - const auto d = -delay.count(); + if (!_leading && !_trailing) + { + throw std::invalid_argument("neither leading nor trailing"); + } + + const auto d = -opts.delay.count(); if (d >= 0) { throw std::invalid_argument("non-positive delay specified"); } - memcpy(&_delay, &d, sizeof(d)); } @@ -54,88 +58,119 @@ public: template void Run(MakeArgs&&... args) { - if (!_storage.emplace(std::forward(args)...)) - { - _leading_edge(); - } + _lead(std::make_tuple(std::forward(args)...)); } - // Modifies the pending arguments for the next function - // invocation, if there is one pending currently. - // - // `func` will be invoked as func(Args...). Make sure to bind any - // arguments in `func` by reference if you'd like to modify them. template void ModifyPending(F func) { - _storage.modify_pending(func); + const auto guard = _lock.lock_exclusive(); + if (_pendingRunArgs) + { + std::apply(func, *_pendingRunArgs); + } } private: static void __stdcall _timer_callback(PTP_CALLBACK_INSTANCE /*instance*/, PVOID context, PTP_TIMER /*timer*/) noexcept + try { - static_cast(context)->_trailing_edge(); + static_cast(context)->_trail(); } + CATCH_LOG() - void _leading_edge() - { - if constexpr (leading) - { - _dispatcher.TryEnqueue(winrt::Windows::System::DispatcherQueuePriority::Normal, [weakSelf = this->weak_from_this()]() { - if (auto self{ weakSelf.lock() }) - { - try - { - self->_func(); - } - CATCH_LOG(); - - SetThreadpoolTimerEx(self->_timer.get(), &self->_delay, 0, 0); - } - }); - } - else - { - SetThreadpoolTimerEx(_timer.get(), &_delay, 0, 0); - } - } - - void _trailing_edge() - { - if constexpr (leading) - { - _storage.reset(); - } - else - { - _dispatcher.TryEnqueue(winrt::Windows::System::DispatcherQueuePriority::Normal, [weakSelf = this->weak_from_this()]() { - if (auto self{ weakSelf.lock() }) - { - try - { - self->_storage.apply(self->_func); - } - CATCH_LOG(); - } - }); - } - } - - inline wil::unique_threadpool_timer _create_timer() + wil::unique_threadpool_timer _create_timer() { wil::unique_threadpool_timer timer{ CreateThreadpoolTimer(&_timer_callback, this, nullptr) }; THROW_LAST_ERROR_IF(!timer); return timer; } - FILETIME _delay; + void _lead(std::tuple args) + { + bool timerRunning = false; + + { + const auto guard = _lock.lock_exclusive(); + + timerRunning = _timerRunning; + _timerRunning = true; + + if (!timerRunning && _leading) + { + // Call the function immediately on the leading edge. + // See below (out of lock). + } + else if (_trailing) + { + _pendingRunArgs.emplace(std::move(args)); + } + } + + const auto execute = !timerRunning && _leading; + const auto schedule = !timerRunning || _debounce; + + if (execute) + { + _execute(std::move(args), schedule); + } + else if (schedule) + { + SetThreadpoolTimerEx(_timer.get(), &_delay, 0, 0); + } + } + + void _trail() + { + decltype(_pendingRunArgs) args; + + { + const auto guard = _lock.lock_exclusive(); + + _timerRunning = false; + args = std::exchange(_pendingRunArgs, std::nullopt); + } + + if (args) + { + _execute(std::move(*args), false); + } + } + + void _execute(std::tuple args, bool schedule) + { + _dispatcher.TryEnqueue( + winrt::Windows::System::DispatcherQueuePriority::Normal, + [weakSelf = this->weak_from_this(), args = std::move(args), schedule]() mutable { + if (auto self{ weakSelf.lock() }) + { + try + { + std::apply(self->_func, std::move(args)); + } + CATCH_LOG(); + + if (schedule) + { + SetThreadpoolTimerEx(self->_timer.get(), &self->_delay, 0, 0); + } + } + }); + } + winrt::Windows::System::DispatcherQueue _dispatcher; + + // Everything below this point is just like til::throttled_func. + function _func; - wil::unique_threadpool_timer _timer; - til::details::throttled_func_storage _storage; -}; + FILETIME _delay; -template -using ThrottledFuncTrailing = ThrottledFunc; -using ThrottledFuncLeading = ThrottledFunc; + wil::srwlock _lock; + std::optional> _pendingRunArgs; + bool _timerRunning = false; + + bool _debounce; + bool _leading; + bool _trailing; +}; diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 33d4b31cfb..ec964059d6 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -288,9 +288,12 @@ void AppHost::Initialize() // the PTY requesting a change to the window state and the Terminal // realizing it, but should mitigate issues where the Terminal and PTY get // de-sync'd. - _showHideWindowThrottler = std::make_shared>( + _showHideWindowThrottler = std::make_shared>( winrt::Windows::System::DispatcherQueue::GetForCurrentThread(), - std::chrono::milliseconds(200), + til::throttled_func_options{ + .delay = std::chrono::milliseconds{ 200 }, + .trailing = true, + }, [this](const bool show) { _window->ShowWindowChanged(show); }); diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h index be987ef0c7..2b38c31d1b 100644 --- a/src/cascadia/WindowsTerminal/AppHost.h +++ b/src/cascadia/WindowsTerminal/AppHost.h @@ -39,7 +39,7 @@ private: std::unique_ptr _window; winrt::TerminalApp::AppLogic _appLogic{ nullptr }; winrt::TerminalApp::TerminalWindow _windowLogic{ nullptr }; - std::shared_ptr> _showHideWindowThrottler; + std::shared_ptr> _showHideWindowThrottler; SafeDispatcherTimer _frameTimer; LARGE_INTEGER _lastActivatedTime{}; winrt::guid _virtualDesktopId{}; diff --git a/src/inc/til/throttled_func.h b/src/inc/til/throttled_func.h index 7a2d544add..d1704ebd17 100644 --- a/src/inc/til/throttled_func.h +++ b/src/inc/til/throttled_func.h @@ -5,120 +5,49 @@ namespace til { - namespace details + struct throttled_func_options { - template - class throttled_func_storage - { - public: - template - bool emplace(MakeArgs&&... args) - { - std::unique_lock guard{ _lock }; - const bool hadValue = _pendingRunArgs.has_value(); - _pendingRunArgs.emplace(std::forward(args)...); - return hadValue; - } + using filetime_duration = std::chrono::duration>; - template - void modify_pending(F f) - { - std::unique_lock guard{ _lock }; - if (_pendingRunArgs) - { - std::apply(f, *_pendingRunArgs); - } - } + filetime_duration delay{}; + bool debounce = false; + bool leading = false; + bool trailing = false; + }; - void apply(const auto& func) - { - decltype(_pendingRunArgs) args; - { - std::unique_lock guard{ _lock }; - args = std::exchange(_pendingRunArgs, std::nullopt); - } - // Theoretically it should always have a value, because the throttled_func - // should not call the callback without there being a reason. - // But in practice a failure here was observed at least once. - // It's unknown to me what caused it, so the best we can do is avoid a crash. - assert(args.has_value()); - if (args) - { - std::apply(func, *args); - } - } - - explicit operator bool() const - { - std::shared_lock guard{ _lock }; - return _pendingRunArgs.has_value(); - } - - private: - // std::mutex uses imperfect Critical Sections on Windows. - // --> std::shared_mutex uses SRW locks that are small and fast. - mutable std::shared_mutex _lock; - std::optional> _pendingRunArgs; - }; - - template<> - class throttled_func_storage<> - { - public: - bool emplace() - { - return _isPending.exchange(true, std::memory_order_relaxed); - } - - void apply(const auto& func) - { - if (_isPending.exchange(false, std::memory_order_relaxed)) - { - func(); - } - } - - void reset() - { - _isPending.store(false, std::memory_order_relaxed); - } - - explicit operator bool() const - { - return _isPending.load(std::memory_order_relaxed); - } - - private: - std::atomic _isPending; - }; - } // namespace details - - template + template class throttled_func { public: - using filetime_duration = std::chrono::duration>; + using filetime_duration = throttled_func_options::filetime_duration; using function = std::function; - // Throttles invocations to the given `func` to not occur more often than `delay`. + // Throttles invocations to the given `func` to not occur more often than specified in options. // - // If this is a: - // * throttled_func_leading: `func` will be invoked immediately and - // further invocations prevented until `delay` time has passed. - // * throttled_func_trailing: On the first invocation a timer of `delay` time will - // be started. After the timer has expired `func` will be invoked just once. + // Options: + // * delay: The minimum time between invocations + // * debounce: If true, resets the timer on each call + // * leading: If true, `func` will be invoked immediately on first call + // * trailing: If true, `func` will be invoked after the delay // - // After `func` was invoked the state is reset and this cycle is repeated again. - throttled_func(filetime_duration delay, function func) : + // At least one of leading or trailing must be true. + throttled_func(throttled_func_options opts, function func) : _func{ std::move(func) }, - _timer{ _createTimer() } + _timer{ _create_timer() }, + _debounce{ opts.debounce }, + _leading{ opts.leading }, + _trailing{ opts.trailing } { - const auto d = -delay.count(); + if (!_leading && !_trailing) + { + throw std::invalid_argument("neither leading nor trailing"); + } + + const auto d = -opts.delay.count(); if (d >= 0) { throw std::invalid_argument("non-positive delay specified"); } - memcpy(&_delay, &d, sizeof(d)); } @@ -139,27 +68,7 @@ namespace til template void operator()(MakeArgs&&... args) { - const auto hadValue = _storage.emplace(std::forward(args)...); - - if constexpr (Debounce) - { - SetThreadpoolTimerEx(_timer.get(), &_delay, 0, 0); - } - else - { - if (!hadValue) - { - SetThreadpoolTimerEx(_timer.get(), &_delay, 0, 0); - } - } - - if constexpr (Leading) - { - if (!hadValue) - { - _func(); - } - } + _lead(std::make_tuple(std::forward(args)...)); } // Modifies the pending arguments for the next function @@ -170,7 +79,11 @@ namespace til template void modify_pending(F func) { - _storage.modify_pending(func); + const auto guard = _lock.lock_exclusive(); + if (_pendingRunArgs) + { + std::apply(func, *_pendingRunArgs); + } } // Makes sure that the currently pending timer is executed @@ -190,36 +103,76 @@ namespace til static void __stdcall _timer_callback(PTP_CALLBACK_INSTANCE /*instance*/, PVOID context, PTP_TIMER /*timer*/) noexcept try { - const auto self = static_cast(context); - if constexpr (Leading) - { - self->_storage.reset(); - } - else - { - self->_storage.apply(self->_func); - } + static_cast(context)->_trail(); } CATCH_LOG() - wil::unique_threadpool_timer _createTimer() + wil::unique_threadpool_timer _create_timer() { wil::unique_threadpool_timer timer{ CreateThreadpoolTimer(&_timer_callback, this, nullptr) }; THROW_LAST_ERROR_IF(!timer); return timer; } - FILETIME _delay; + void _lead(std::tuple args) + { + bool timerRunning = false; + + { + const auto guard = _lock.lock_exclusive(); + + timerRunning = _timerRunning; + _timerRunning = true; + + if (!timerRunning && _leading) + { + // Call the function immediately on the leading edge. + // See below (out of lock). + } + else if (_trailing) + { + _pendingRunArgs.emplace(std::move(args)); + } + } + + if (!timerRunning && _leading) + { + std::apply(_func, std::move(args)); + } + + if (!timerRunning || _debounce) + { + SetThreadpoolTimerEx(_timer.get(), &_delay, 0, 0); + } + } + + void _trail() + { + decltype(_pendingRunArgs) args; + + { + const auto guard = _lock.lock_exclusive(); + + _timerRunning = false; + args = std::exchange(_pendingRunArgs, std::nullopt); + } + + if (args) + { + std::apply(_func, *std::move(args)); + } + } + function _func; wil::unique_threadpool_timer _timer; - details::throttled_func_storage _storage; + FILETIME _delay; + + wil::srwlock _lock; + std::optional> _pendingRunArgs; + bool _timerRunning = false; + + bool _debounce; + bool _leading; + bool _trailing; }; - - template - using throttled_func_trailing = throttled_func; - using throttled_func_leading = throttled_func; - - template - using debounced_func_trailing = throttled_func; - using debounced_func_leading = throttled_func; } // namespace til diff --git a/src/til/ut_til/throttled_func.cpp b/src/til/ut_til/throttled_func.cpp index 3cb85025f5..dce22d5080 100644 --- a/src/til/ut_til/throttled_func.cpp +++ b/src/til/ut_til/throttled_func.cpp @@ -19,21 +19,25 @@ class ThrottledFuncTests TEST_METHOD(Basic) { using namespace std::chrono_literals; - using throttled_func = til::throttled_func_trailing; + using throttled_func = til::throttled_func; til::latch latch{ 2 }; std::unique_ptr tf; - tf = std::make_unique(10ms, [&](bool reschedule) { - latch.count_down(); + tf = std::make_unique( + til::throttled_func_options{ + .delay = 10ms, + .trailing = true }, + [&](bool reschedule) { + latch.count_down(); - // This will ensure that the callback is called even if we - // invoke the throttled_func from inside the callback itself. - if (reschedule) - { - tf->operator()(false); - } - }); + // This will ensure that the callback is called even if we + // invoke the throttled_func from inside the callback itself. + if (reschedule) + { + tf->operator()(false); + } + }); // This will ensure that the throttled_func invokes the callback in general. tf->operator()(true);