Avoid animations during startup #15204
Merged
Conversation
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
lhecker
added
Area-Performance
|
Before After Side effect: Scrollbars are more responsive now w7rsxTfcrsAhEKgU.mp4 |
DHowett
force-pushed
the
dev/lhecker/lazy-command-palette
branch
from
d11290c to
83bb7e0
Compare
DHowett
force-pushed
the
dev/lhecker/startup-animations
branch
from
649f89b to
082aef0
Compare
DHowett
force-pushed
the
dev/lhecker/startup-animations
branch
from
082aef0 to
bf8d0b5
Compare
DHowett
approved these changes
Apr 19, 2023
| @@ -961,11 +961,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |||
|
|
|||
| auto bufferHeight = _core.BufferHeight(); | |||
|
|
|||
| ScrollBar().Maximum(bufferHeight - bufferHeight); | |||
| ScrollBar().Maximum(0); | |||
| ScrollBar().Minimum(0); | ||
| ScrollBar().Value(0); | ||
| ScrollBar().ViewportSize(bufferHeight); | ||
| ScrollBar().LargeChange(std::max(bufferHeight - 1, 0)); // scroll one "screenful" at a time when the scroll bar is clicked | ||
| ScrollBar().LargeChange(bufferHeight); // scroll one "screenful" at a time when the scroll bar is clicked |
There was a problem hiding this comment.
there must have been a reason it was bufferHeight-1, right?
There was a problem hiding this comment.
I think it could be because the original author compared the behavior against less, where PgUp/Down moves by 29 lines in a 30 line viewport. But that's because the 30th line is the ":" command prompt. Setting this to bufferHeight does do what you'd think it does (= page by 30 rows / the viewport height) and is the more correct behavior IMO.
zadjii-msft
approved these changes
Apr 20, 2023
DHowett
pushed a commit
that referenced
this pull request
Apr 25, 2023
This fixes 3 sources for animations: * `TabView`'s `EntranceThemeTransition` causes tabs to slowly slide in from the bottom. Removing the transition requires you to override the entire list of transitions obviously, which is a global change. Nice. Am I glad I don't need to deal with the complexity of CSS. /s * `TabBase`, `SettingsTab` and `TerminalTab` were using a lot of coroutines with `resume_foreground` even though almost none of the functions are called from background tabs in the first place. This caused us to miss the initial XAML drawing pass, which resulted in animations when the tab icons would asynchronously pop into existence. It also appears as if `resume_foreground`, etc. have a very high CPU cost attached, which surprises me absolutely not at all given WinRT. The improvement is difficult to quantify because the run to run variation is very high. But it seems like this shaves about 10% off of the ~500ms startup delay on my PC depending on how you measure it. Part of #5907 * It starts when it should ✅ * It doesn't "exit" when it shouldn't ✅ (Scrolling, Settings reload, Bell `\a`, Progress `\e]9;4;2;80\e\\`) (cherry picked from commit 35b9e75) Service-Card-Id: 89001994 Service-Version: 1.17
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes 3 sources for animations:
TabView'sEntranceThemeTransitioncauses tabs to slowly slide infrom the bottom. Removing the transition requires you to override the
entire list of transitions obviously, which is a global change. Nice.
Am I glad I don't need to deal with the complexity of CSS. /s
TabBase,SettingsTabandTerminalTabwhere using a lot ofcoroutines with
resume_foregroundeven though almost none of thefunctions are called from background tabs in the first place. This
caused us to miss the initial XAML drawing pass, which resulted in
animations when the tab icons would asynchronously pop into existence.
It also appears as if
resume_foreground, etc. have a very high CPUcost attached, which surprises me absolutely not at all given WinRT.
The improvement is difficult to quantify because the run to run
variation is very high. But it seems like this shaves about 10% off
of the ~500ms startup delay on my PC depending on how you measure it.
Part of #5907
PR Checklist
(Scrolling, Settings reload, Bell
\a, Progress\e]9;4;2;80\e\\)