-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Decrease CPU usage of OverflowTextBlock + optimization #403
Merged
danbelcher-MSFT
merged 7 commits into
microsoft:master
from
rudyhuyn:OptimizeOverflowTextblock
Apr 18, 2019
Merged
Decrease CPU usage of OverflowTextBlock + optimization #403
danbelcher-MSFT
merged 7 commits into
microsoft:master
from
rudyhuyn:OptimizeOverflowTextblock
Apr 18, 2019
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
- optimize ExpressionTokens in StandardCalculatorViewModel to only update new tokens instead of the full list - refactor how we manage the visibility of left/right buttons - remove useless ExpressionItemContainerStyle - only modify accessbility view if necessary
rudyhuyn
changed the title
Optimize performance and CPU usage of OverflowTextBlock
Decrease CPU usage of OverflowTextBlock + optimizations
Mar 28, 2019
rudyhuyn
changed the title
Decrease CPU usage of OverflowTextBlock + optimizations
Decrease CPU usage of OverflowTextBlock + optimization
Mar 28, 2019
danbelcher-MSFT
suggested changes
Apr 16, 2019
…ate to make this control customizable by other developers
rudyhuyn
commented
Apr 17, 2019
@@ -30,25 +30,38 @@ DEPENDENCY_PROPERTY_INITIALIZATION(OverflowTextBlock, TokensUpdated); | |||
|
|||
void OverflowTextBlock::OnApplyTemplate() | |||
{ | |||
assert(((m_scrollLeft == nullptr) && (m_scrollRight == nullptr)) || ((m_scrollLeft != nullptr) && (m_scrollRight != nullptr))); | |||
UnregisterEventHandlers(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slight refactoring to make this control more customizable (in case developers want to use it in their own apps):
- can apply a template more than 1 time
- make controls optional
danbelcher-MSFT
approved these changes
Apr 18, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good. Thanks, Rudy!
EriWong
pushed a commit
to EriWong/calculator
that referenced
this pull request
Jun 5, 2019
Fixes microsoft#402 and microsoft#414 Divide by 4 the CPU usage of OverflowTextBlock when buttons are pressed very quickly. Description of the changes: Xaml-side: OverflowTextBlock has some performance issues: double scrollviewer: the listview was in a scrollviewer, while the control already containing one -> it breaks the virtualization of the listview and impacts on UI performance. The listview used a StackPanel, this panel doesn't support virtualization of ListViewItems contrary to ItemsStackPanel No ListView-specific features were used, an ItemsControl is more efficient and lighter. refactor how we manage the visibility of the left/right buttons in OverflowTextBlock, the new version is more reactive and will not display the right arrow when not necessary (see GIF below). remove the ItemContainerSelector ExpressionItemContainerStyle, not really used by OverflowTextBlock remove UI glitches generated by ChangeView when users type fast (control partially hidden and scrolling issues, see the GIF below). only modify the accessibility view when it's necessary ViewModel-side: stop fully refreshing ExpressionTokens in StandardCalculatorViewModel when a new command were sent, instead, use a IObservableVector to only send new tokens to the UI (in average only 1 or 2 UI items are refreshed while the full expression was refreshed before) How changes were validated: Manually
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.
Fixes #402 and #414
Divide by 4 the CPU usage of
OverflowTextBlock
when buttons are pressed very quickly.Description of the changes:
Xaml-side:
OverflowTextBlock
has some performance issues:StackPanel
, this panel doesn't support virtualization of ListViewItems contrary toItemsStackPanel
ItemsControl
is more efficient and lighter.ExpressionItemContainerStyle
, not really used byOverflowTextBlock
ViewModel-side:
ExpressionTokens
inStandardCalculatorViewModel
when a new command were sent, instead, use a IObservableVector to only send new tokens to the UI (in average only 1 or 2 UI items are refreshed while the full expression was refreshed before)Demo:
How changes were validated:
Manually