Repository navigation
24fps (noticeable) when opening animation demo on Nexus 5 #9219
Description
There is a noticeable un-smoothness when opening the animation demo.
Make a release build of the gallery, open animation demo:
{{ message }}
There is a noticeable un-smoothness when opening the animation demo.
Make a release build of the gallery, open animation demo:
Activity
sethladd commentedon Apr 5, 2017
For reference, here are the spikes when opening pesto, shrine, and contacts:
eseidelGoogle commentedon Apr 6, 2017
Seth believes this should block an updated release of the gallery. The next build is slated to being release verification next week, so we'd need to fix this by then.
sethladd commentedon Apr 6, 2017
On a Moto G4 (the lower-bound of our Android phone target) we see 22fps when opening any component/widget demo (not just the vignettes)
eseidelGoogle commentedon Apr 11, 2017
@chinmaygarde do you know if we upload images async to the GPU? Just musing at Triage about why these huge spikes might exist.
chinmaygarde commentedon Apr 11, 2017
It is the raster cache snapshotting huge portions of the hierarchy when the animation is about to start and settle at either end. This work is expensive and completely unnecessary.
chinmaygarde commentedon Apr 11, 2017
I investigated this further. Though the extra snapshotting work does take time, it is not the bottleneck. The
SkCanvas::Flushitself takes a long time. This suggests we are simply making too many draw calls to the GPU. We need to simplify the scene.ScottPierce commentedon Apr 11, 2017
@chinmaygarde So that means that the scene has hit the limit of what Flutter can do on the phone? Are there any other potential areas of optimization?
eseidelGoogle commentedon Apr 12, 2017
@chinmaygarde so next step is to look at the skpicture? Or where would we look next for reducing draw calls? What sort of tooling/logging should we add to understand situations like this?
Hixie commentedon Apr 12, 2017
In that screenshot, we're being descheduled for a really long time. The flush is long, but not THAT long.
Can we break down what's in the scene and how expensive each operation is? It doesn't seem that complicated superficially. It has very little text, for instance. A few gradients, a few rectangles, some images.
chinmaygarde commentedon Apr 12, 2017
sethladd commentedon Apr 17, 2017
Thanks for doing the research!
7 remaining items
Hixie commentedon Apr 19, 2017
I would very much prefer if we kept the fade animation... it's what other Android apps do on those devices.
chinmaygarde commentedon Apr 19, 2017
While @sethladd, @xster and I were tooling around on the device, we saw that the behavior was inconsistent. The contacts app had the slide transition while some parts of Google Play Music had the fade.
In any case, if really like the fade transition, we can also introduce a picture layer boundary at that point in the hierarchy and make that picture be eligible for participation into the raster cache. We would pay the cost of populating the raster cache entry once (and may skip a frame or two on less powerful hardware) but subsequent frames would be fine. I'll submit patches for both tomorrow and we can just keep the one we like.
sethladd commentedon Apr 19, 2017
Once we make it fast, we should add it back. It looks really nice.
Hixie commentedon Apr 19, 2017
I thought Opacity already always introduced a repaint boundary.
chinmaygarde commentedon Apr 19, 2017
That is not true.
Update:
RenderImagecurrently hard codes the blend mode toBlendMode.srcIn. @HansMuller is working on making that configurable. He says he will be done with that by today. We can then fix the demo to use the new stuff.flowand an optimization applied where the render target switch is avoided in favor of achieving the same effect via paint options (like the one we are manually applying in this case). The issue tracking the implementation of that optimization is filed here.Hixie commentedon Apr 19, 2017
https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/rendering/proxy_box.dart#L709
...says that Opacity will generate a new layer whenever the opacity is not 100% or 0%. At 100% it just passes everything through unmodified. At 0% it doesn't render anything.
Is this not working?
chinmaygarde commentedon Apr 19, 2017
Ah, I see. I was looking at
isRepaintBoundaryonRenderObject. But, even if a picture layer is introduced, the raster cache may disregard it for caching purposes if the op count of the picture is low enough (which it is in this case because there is only child). To force the raster cache to always cache the picture, you would need to set theisComplexHinton the same. I am hesitant to set that hint for this particular layer because doing it correctly via a paint in aflowoptimization is better than making the raster cache populate an entry for us. Populating the raster cache entry will incur a hit initially and will also take up extra memory.Ideally, the user will use the performance debugging tools to avoid creating unnecessary opacity layers. In case that is not the case, the
flowoptimizations should catch most remaining cases.xster commentedon Apr 19, 2017
In the case of the transitions, there's only one Opacity widget in the app if I understand correctly right? We should let users have one opacity without having to do performance debugging and removing the opacity.
Hixie commentedon Apr 19, 2017
I don't think we can count on our users doing anything involving profiling FadeTransitions, especially those involved in a MaterialPageRoute. They're going to want to assume that Just Works.
chinmaygarde commentedon Apr 19, 2017
Hmm, even with the
flowoptimizations, we will only be able to handle cases where the fade is applied to a pictures that have a child (or maybe multiple children that themselves have no children, and, don't overlap). UsingFadeTransitionson nested hierarchies excessively will cause performance bottlenecks. Some profiling there is unavoidable.Hixie commentedon Apr 19, 2017
I don't really follow. What do you want the end user to do?
To put it another way, what is the fix here for us to re-enable this?
github-actions commentedon Sep 5, 2021
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.