Extend lead/trail edge support for throttled_func #19210
Conversation
| toolTipInterval, | ||
| til::throttled_func_options{ | ||
| .delay = std::chrono::milliseconds{ hoverTimeoutMillis }, | ||
| .debounce = true, |
There was a problem hiding this comment.
Try jiggling your cursor over the minimize/maximize buttons and the popup will flicker. By setting debounce=true here this is also fixed.
| co_await winrt::resume_background(); | ||
|
|
||
| CompletionsChanged.raise(*this, *args); | ||
| CompletionsChanged.raise(*this, winrt::make<CompletionsChangedEventArgs>(winrt::hstring{ menuJson }, replaceLength)); |
There was a problem hiding this comment.
Oh right, this is a drive by change. I did it initially when I considered going a different route with this and I can back it out.
The reason I kept this is because this is the only function which yields to a background thread before invoking the callback. We shouldn't do that to keep it consistent (no surprises).
|
|
||
| if (execute) | ||
| { | ||
| _execute(std::move(args), schedule); |
There was a problem hiding this comment.
doesn't this move from args twice, once on R106 and once on R115, if the function is both leading and trailing?
There was a problem hiding this comment.
No, because the condition for executing here is !timerRunning && _leading which is the same condition as above that precludes it from being moved.
| const auto toolTipInterval = std::chrono::milliseconds(hoverTimeoutMillis); | ||
| if (FAILED(SystemParametersInfoW(SPI_GETMOUSEHOVERTIME, 0, &hoverTimeoutMillis, 0))) | ||
| { | ||
| hoverTimeoutMillis = 400; |
There was a problem hiding this comment.
does SystemParametersInfoW destroy the value on failure? bah!
There was a problem hiding this comment.
I don't know that, but I don't believe that we should rely on the assumption that it doesn't. I doubt that this was tested in some way too (after all, can this call even fail?).
There was a problem hiding this comment.
After reviewing the kernel code, I found that the call can never fail.
| co_await winrt::resume_background(); | ||
|
|
||
| CompletionsChanged.raise(*this, *args); | ||
| CompletionsChanged.raise(*this, winrt::make<CompletionsChangedEventArgs>(winrt::hstring{ menuJson }, replaceLength)); |
You can now create throttled functions which trigger both on the leading and trailing edge. This was then also ported to `ThrottledFunc` for `DispatcherQueue`s and used for title/taskbar updates. Closes #19188 * In CMD run: ```batch FOR /L %N IN () DO @echo %time% ``` * Doesn't hang the UI ✅ SERVICE NOTES This replays part of #19192 to make it compatible with throttled_func. (cherry picked from commit dbf740c) Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgdI2TQ PVTI_lADOAF3p4s4AxadtzgdSMv0 Service-Version: 1.23
You can now create throttled functions which trigger both on the leading
and trailing edge. This was then also ported to
ThrottledFuncforDispatcherQueues and used for title/taskbar updates.Closes #19188
Validation Steps Performed