Optimize static content scrolling #17621
Conversation
yjbanov
force-pushed
the
optimize-static-content-scrolling
branch
2 times, most recently
from
56e5b76
to
b45e37f
Compare
| /// of the clip. | ||
| static double _predictTrend(double delta, double extent) { | ||
| if (delta <= 0.0) { | ||
| // Shrinking. Give it 10% of the extent in case the trend is reversed. |
There was a problem hiding this comment.
Why not keep extent neutral?
There was a problem hiding this comment.
The assumption is that the larger the scrollable area the faster the absolute scroll speed. For example, consider scrolling in portrait mode vs landscape mode, or scrolling a full screen vs a 100px-high drop-down list. My method for choosing this strategy was not very scientific though, so I'm open to other ideas.
| /// [PictureRecorder] when the framework calls [PictureRecorder.endRecording]. | ||
| /// However, if you are writing a unit-test and using [RecordingCanvas] | ||
| /// directly it is up to you to call this method explicitly. | ||
| void endRecording() { |
There was a problem hiding this comment.
The code path seems to be from endRecordingIfNeeded, is it guaranteed to be called ?
There was a problem hiding this comment.
PictureRecorder.endRecording always calls it, and it is the only way to get an instance of Picture. Therefore it is guaranteed to be called.
There was a problem hiding this comment.
Ah , i meant stopRecordingIfNeeded entry point that eventually calls picturerecorder.endrecording
There was a problem hiding this comment.
Also checked stopRecordingIfNeeded. It is only used to determine whether a Canvas was created (Flutter creates canvases lazily, like canvas_pool.dart). However, if it starts recording a picture, it is guaranteed to call endRecording to get the Picture object.
yjbanov
force-pushed
the
optimize-static-content-scrolling
branch
from
14b1fd3
to
6d8fc1f
Compare
yjbanov
force-pushed
the
optimize-static-content-scrolling
branch
from
6d8fc1f
to
22e526f
Compare
liyuqian
added
perf: speed
* store paint command bounds * do not apply commands outside the clip region * better cull rect prediction * enforce RecordingCanvas.endRecording
|
So am I lead to believe that this is the progress check for any scrolling where it feels janky? As chrome mobile seems to have a lot of issues, even with the most basic of listviews such as the official one mentioned on flutter's own cookbook: https://flutter.dev/docs/cookbook/lists/long-lists |
Optimize static content scrolling by pruning paint operations that are clipped out either by one of
Canvas.clip*methods, or more commonly, by a clipping ancestor layer. This also adjusts the_optimalCullRectcalculation. Previously we allowed the cull rect to grow indefinitely. With this change we also allow it to shrink.Fixes flutter/flutter#42987
Fixes flutter/flutter#48516
Fixes partially flutter/flutter#32338 (the provided sample can use a
SingleChildScrollViewwith aColumnof thousands ofTextwidgets with reasonable scrolling performance, but this PR does not fix the sample app as is)Fixes partially flutter/flutter#54584 (but we should also look at the canvas sandwich problem)
How does this affect static content scrolling?
When scrolling we typically shift a picture inside a static clip. However, when viewed from the picture's coordinate system, the clip is moving around a static picture. This will occasionally cause repaints because
PersistedPicturewill conclude that the clip has moved to a location that was not previously painted and ask the canvas to paint at the new clip region. This logic existed there for the sake of painting on<canvas>where we did not want to allocate a canvas that's too big. However, when painting using DOM nodes we never took clipping into account and always created all the DOM nodes no matter whether they are visible or not.This change eliminates a lot of nodes that are outside the visible range. We still need the optimization that predicts the next several clips, otherwise we'd be repainting on every frame. However, we can no longer allow the predicted clip to grow indefinitely because there are situations when the picture contains enormous amounts of paint operations, so if there's no upper bound for the clip region, we'll eventually jank again.
How does this affect lazy-rendered scrolling?
Lazy-rendered scrolling already prunes paint operations at the framework level (e.g. via slivers), and so it doesn't need pruning at the engine level. So when we see that the picture fully fits in the clip (as is commonly the case for lazy-rendered content) we do not filter the paint ops. However, we have to track the paint bounds for every paint operation. We already compute these bounds but we "forget" them as soon as we compute them. With this change we store these bounds on every paint operation. Unfortunately this causes a 2-5% regression for lazy-rendered content. In subsequent PRs I will optimize these away, but I decided to not include them as part of this PR to minimize the risk of correctness regressions (it's harder to roll back a mega PR than small PRs).
Benchmarks
The benchmark results can be found here.