-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Description
The zoom page transition is the default page transition for Android, and used heavily by google customers and customer: money. Unfortunately, the raster performance of this transitions leaves a lot to be desired. Currently, the transition uses a FadeTransition and ScaleTransition. The changing scale factor essentially disables raster caching of child pictures, and apparently causes increases in native memory compared to the baseline transition: #100972
To solve this, we originally investigated switching from a ScaleTransition using a transform layer, to a ScaleTransition using an ImageFilterLayer with a ImageFilter.matrix. In theory, this should be faster than a transform layer for this case, since the pages aren't animated at all while the transition occurs and this allows a single raster cache entry to be used for the entire page (which can also absorb the opacity from the FadeTransition). While switching did improve memory consumption, it regressed worst frame raster performance by about 1.5x on our gallery benchmark (https://flutter-flutter-perf.skia.org/e/?begin=1650989645&end=1651012744&keys=X809a5f88fb68c1b259242471f65ee081&num_commits=50&request_type=1&xbaroffset=28565) . This is likely due to either the performance of Skia's ImageFilter, or perhaps specific to shader compilation for the first use of this filter.
Interestingly enough though, we accidentally rolled a version of flutter into google3 that used a ImageFilterLayer 🙃 and it significantly improved the performance of a number of customer money benchmarks. A "worst raster frame" benchmark improved from 104.7 ms to 41.75 ms. A follow-up roll reverted the change and the benchmark regressed again. b/232993351
Its quite difficult to identify performance improvements that are as good as the above change, so I attempted a fix in the engine/raster cache to special case a scale transform matrix filter: flutter/engine#33562 , but that stalled out since it conflicted with another refactor PR.
@xster @zanderso can you help determine how we should prioritize this?