-
Notifications
You must be signed in to change notification settings - Fork 28.7k
Add benchmark reproducing large static scrolling content #53686
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
Conversation
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
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.
import 'recorder.dart'; | ||
import 'test_data.dart'; | ||
|
||
/// The number of rows should be such that |
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.
such that ... ?
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.
Oops. Leftover. I moved this check into the constructor. Replaced with dartdocs.
const double maxScrollExtent = kMaxSampleCount * kScrollDelta; | ||
const double pictureHeight = kRows * kRowHeight; |
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.
I'm curious why these constants are here instead of at the top with the others?
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.
They are here for validation only, and they don't supply any new configuration for this benchmark. The ones at the top can be manually tweaked, unlike these that will always remain derivatives of the top-level constants.
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.
Other than comment /// The number of rows should be such that , lgtm
Description
Add benchmark that reproduces #42987, i.e. a large piece of static content (a
Picture
that never changes) is scrolled. As the clip slides along the picture it causes repaints. Since our DOM operations are not culled, it causes jank proportional to the size of the picture.I will send a separate fix in flutter/engine, but want to get the benchmark first so we can validate the fix later.
Related Issues
#42987