-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Closed
Labels
Area-PerformancePerformance-related issuePerformance-related issueArea-TerminalControlIssues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.)Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.)Issue-BugIt either shouldn't be doing this or needs an investigation.It either shouldn't be doing this or needs an investigation.Needs-Tag-FixDoesn't match tag requirementsDoesn't match tag requirementsProduct-TerminalThe new Windows Terminal.The new Windows Terminal.Resolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Description
PR #3531 added this code to throttle scrollbar updates to reduce CPU usage:
terminal/src/cascadia/TerminalControl/TermControl.cpp
Lines 1419 to 1432 in ee087cf
| // Throttle the update operation. | |
| const auto timeNow = std::chrono::high_resolution_clock::now(); | |
| if (_lastScrollTime.has_value()) | |
| { | |
| const long long deltaTimeInMilliSec = std::chrono::duration_cast<std::chrono::milliseconds>(timeNow - _lastScrollTime.value()).count(); | |
| if (deltaTimeInMilliSec < ScrollRateLimitMilliseconds) | |
| { | |
| _lastScrollTime = std::nullopt; | |
| return; | |
| } | |
| } | |
| _lastScrollTime = timeNow; |
But the code skips the last scroll update. If we have 4 updates very quickly, then:
- 1st will set the
_lastScrollTimeand update the scrollbar - 2nd will return because the time difference is too small but will reset
_lastScrollTimefor the next one (BTW why reset? we could skip the next one too) - 3rd will set the
_lastScrollTimeand update the scrollbar - 4th will return because the time difference is too small but will reset
_lastScrollTimefor the next one
So we missed the last update (the 4th) and now the scrollbar has invalid value.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
Area-PerformancePerformance-related issuePerformance-related issueArea-TerminalControlIssues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.)Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.)Issue-BugIt either shouldn't be doing this or needs an investigation.It either shouldn't be doing this or needs an investigation.Needs-Tag-FixDoesn't match tag requirementsDoesn't match tag requirementsProduct-TerminalThe new Windows Terminal.The new Windows Terminal.Resolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.Fix is checked in, but it might be 3-4 weeks until a release.