Repository navigation
Skip painting hidden children of a sliver list - #18465
Conversation
|
CC @tvolkert and @chinmaygarde on the effect of this PR on FL-53. |
|
This will need tests. cc @goderbauer and @jonahwilliams for review |
|
Unit test added. |
|
Rebaseline those tree dump strings in the unit test is so painful... What's the best way to do that as I can foresee that I'm going to change it frequently? @Hixie @tvolkert @chinmaygarde |
|
In addition to SliverLists, this will also affect the SliverGrid. Does this work correctly there as well? |
Sorry, I have no guidance to offer there. We should document the current process on the wiki though. |
| expect(list, hasAGoodToStringDeep); | ||
| final String dump = list.toStringDeep(minLevel: DiagnosticLevel.info); | ||
| final int numChildrenPainted = 'layer: OffsetLayer'.allMatches(dump).length; | ||
| expect(numChildrenPainted, 2); // only the middle two should be painted |
There was a problem hiding this comment.
this test might be a good candidate for the mock_canvas API (search for tests with expectations that mention paints)
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@goderbauer The SliverGrid can only scroll along the main axis, and this PR would prune those that are outside of the main axis. But this PR won't prune widgets that are outside of the cross axis. I guess that there won't be many hidden widgets along the cross axis because you can never scroll to reveal them. Hence we probably don't need to prune them. |
There was a problem hiding this comment.
I believe _evaluatePredicates should just return false on a miss-match and you add the failure reason to the StringBuffer description.
|
RSLGTM |
* master: Remove race conditions involving finding available ports (flutter#18488) Skip painting hidden children of a sliver list (flutter#18465) Fixed some typos (flutter#18516) update Tristate checkbox semantics to consider indeterminate as "unchecked" (flutter#18297) Add flutter tool support for creating app-specific VM snapshots. (flutter#18417) mention Text.rich on RichText (flutter#18434) invert children for real (flutter#18489) Fix clipping for SliverLists (flutter#18410) Keep TextFields visible when keyboard comes up (flutter#18291) Update all packages (flutter#18506) Prepare for upgrading video player in Gallery to newest version (flutter#18501) Update Android dependencies (flutter#18499) Enable running with prebuilt test-only apk (flutter#18453) Revert "Update all packages (flutter#18471)" (flutter#18492) Add some more dart doc to RepaintBoundary (flutter#17183) Update all packages (flutter#18471) Clean up output of "flutter run --release" (flutter#18380) Ensure errors launching emulators are exposed to the daemon (flutter#18446) Check video widget is mounted on call to setState (flutter#18467) Fixed concurrent list modification in vmservice.dart
This reverts commit 3806537.
6 participants
We have many hidden children because of iOS accessibility (see also #18410).
For example, in the tile screen of complex_layout app, there could be up to 20 children being painted without this PR. With this PR, only 12 get painted.
Unfortunately, this PR does not move the needle of complex_layout_scroll_perf test. So flutter-team-archive/engine#5497 is still needed to prune out more hidden widgets that maybe only the C++ engine side could determine.
On the other hand, without flutter-team-archive/engine#5497, this PR alone could improve the performance of FL-53 by 40% (because this PR alone could save a lot of render target switches).