-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Throttle scrollbar updates in TermControl to ~one per 8ms (#4608)
In addition to the below (original) description, this commit introduces a ThrottledFunc template that can throttle _any_ function. It applies that type to muffle updates to the scrollbar. --- Redo #3531 but without the bug that it caused (#3622) which is why it was reverted. I'm sorry if I explain this badly. If you don't understand a part, make sure to let me know and I will explain it better. ### Explanation How it worked before: `Terminal` signals that viewport changed -> `TermControl::_TerminalScrollPositionChanged` gets called on the terminal thread -> it dispatches work for later to be ran the UI thread to updates the scrollbar's values Why it's bad: * If we have many viewport changes, it will create a long stack of operations to run. Instead, we should just update the scroll bar with the most recent information that we know. * Imagine if the rate that the work gets pushed on the UI thread is greater than the rate that it can handle: it might freeze? * No need to be real time, we can wait just a little bit (8ms) to accumulate viewport changes before we actually change the scroll bar's value because it appears to be expensive (see perf below). Now: `Terminal` signals that viewport changed -> `TermControl::_TerminalScrollPositionChanged` gets called on the terminal thread -> it tells the `ScrollBarUpdater` about a new update -> the `ScrollBarUpdater` only runs one job (I don't know if that's the right term) on the UI thread at a time. If a job is already running but hasn't updated the scroll bar yet, it changes the setting in the already existing job to update the scroll bar with the new values. A job "waits" some time before doing the update to throttle updates because we don't need real time scroll bar updates. -> eventually, it updates the scroll bar If the user scrolls when a scroll bar update is pending, we keep the scroll bar's Maximum and Minimum but let the user choose its new Value with the `CancelPendingValueChange` method. ### Note Also I changed a little bit the code from the Terminal to notify the TermControl less often when possible. I tried to scroll with the scroll bar, with the mouse wheel. I tried to scroll while content is being outputted. I tried to reproduce the crash from #2248 without success (good). Co-authored-by: Leonard Hecker <leonard@hecker.io> Closes #3622 (cherry picked from commit 25df527) # Conflicts: # .github/actions/spell-check/dictionary/apis.txt
- Loading branch information
Showing
12 changed files
with
462 additions
and
250 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| #pragma once | ||
| #include "pch.h" | ||
|
|
||
| template<typename T> | ||
| class ThreadSafeOptional | ||
| { | ||
| public: | ||
| template<class... Args> | ||
| bool Emplace(Args&&... args) | ||
| { | ||
| std::lock_guard guard{ _lock }; | ||
|
|
||
| bool hadValue = _inner.has_value(); | ||
| _inner.emplace(std::forward<Args>(args)...); | ||
| return !hadValue; | ||
| } | ||
|
|
||
| std::optional<T> Take() | ||
| { | ||
| std::lock_guard guard{ _lock }; | ||
|
|
||
| std::optional<T> value; | ||
| _inner.swap(value); | ||
|
|
||
| return value; | ||
| } | ||
|
|
||
| // Method Description: | ||
| // - If the optional has a value, then call the specified function with a | ||
| // reference to the value. | ||
| // - This method is always thread-safe. It can be called multiple times on | ||
| // different threads. | ||
| // Arguments: | ||
| // - f: the function to call with a reference to the value | ||
| // Return Value: | ||
| // - <none> | ||
| template<typename F> | ||
| void ModifyValue(F f) | ||
| { | ||
| std::lock_guard guard{ _lock }; | ||
|
|
||
| if (_inner.has_value()) | ||
| { | ||
| f(_inner.value()); | ||
| } | ||
| } | ||
|
|
||
| private: | ||
| std::mutex _lock; | ||
| std::optional<T> _inner; | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| /*++ | ||
| Copyright (c) Microsoft Corporation | ||
| Licensed under the MIT license. | ||
| Module Name: | ||
| - ThrottledFunc.h | ||
| Abstract: | ||
| - This module defines a class to throttle function calls. | ||
| - You create an instance of a `ThrottledFunc` with a function and the delay | ||
| between two function calls. | ||
| - The function takes an argument of type `T`, the template argument of | ||
| `ThrottledFunc`. | ||
| - Use the `Run` method to wait and then call the function. | ||
| --*/ | ||
|
|
||
| #pragma once | ||
| #include "pch.h" | ||
|
|
||
| #include "ThreadSafeOptional.h" | ||
|
|
||
| template<typename T> | ||
| class ThrottledFunc : public std::enable_shared_from_this<ThrottledFunc<T>> | ||
| { | ||
| public: | ||
| using Func = std::function<void(T arg)>; | ||
|
|
||
| ThrottledFunc(Func func, winrt::Windows::Foundation::TimeSpan delay) : | ||
| _func{ func }, | ||
| _delay{ delay } | ||
| { | ||
| } | ||
|
|
||
| // Method Description: | ||
| // - Runs the function later with the specified argument, except if `Run` | ||
| // is called again before with a new argument, in which case the new | ||
| // argument will be instead. | ||
| // - For more information, read the "Abstract" section in the header file. | ||
| // Arguments: | ||
| // - arg: the argument to pass to the function | ||
| // Return Value: | ||
| // - <none> | ||
| winrt::fire_and_forget Run(T arg) | ||
| { | ||
| if (!_pendingCallArg.Emplace(arg)) | ||
| { | ||
| // already pending | ||
| return; | ||
| } | ||
|
|
||
| auto weakThis = this->weak_from_this(); | ||
|
|
||
| co_await winrt::resume_after(_delay); | ||
|
|
||
| if (auto self{ weakThis.lock() }) | ||
| { | ||
| auto arg = self->_pendingCallArg.Take(); | ||
| if (arg.has_value()) | ||
| { | ||
| self->_func(arg.value()); | ||
| } | ||
| else | ||
| { | ||
| // should not happen | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Method Description: | ||
| // - Modifies the pending argument for the next function invocation, if | ||
| // there is one pending currently. | ||
| // - Let's say that you just called the `Run` method with argument A. | ||
| // After the delay specified in the constructor, the function R | ||
| // specified in the constructor will be called with argument A. | ||
| // - By using this method, you can modify argument A before the function R | ||
| // is called with argument A. | ||
| // - You pass a function to this method which will take a reference to | ||
| // argument A and will modify it. | ||
| // - When there is no pending invocation of function R, this method will | ||
| // not do anything. | ||
| // - This method is always thread-safe. It can be called multiple times on | ||
| // different threads. | ||
| // Arguments: | ||
| // - f: the function to call with a reference to the argument | ||
| // Return Value: | ||
| // - <none> | ||
| template<typename F> | ||
| void ModifyPending(F f) | ||
| { | ||
| _pendingCallArg.ModifyValue(f); | ||
| } | ||
|
|
||
| private: | ||
| Func _func; | ||
| winrt::Windows::Foundation::TimeSpan _delay; | ||
| ThreadSafeOptional<T> _pendingCallArg; | ||
| }; |
Oops, something went wrong.