You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is for tracking all related pull requests for enabling retained rendering.
In my local experiments, the retained rendering can speedup fading transition by 2.15x and put our flutter_gallery__transition_perf average_frame_rasterizer_time_millis well below 16ms on Moto G4. Hence it will fix #13736
Let's try to close this issue before the end of October so we can squeeze another 30% speedup?
We'll also have to check the memory usage (thanks Chinmay for the remind). My guess is that the memory cost should be much less compared to our current raster cache: in my experiments, I only add the raster cache when there's an animated opacity. Those things should be much rarer than RepaintBoundaries and PictureLayers. Chinmay and I also think that we shall provide the raster cache option to the developers just like what we did with clipping.
For example, the constructor of AnimatedOpacity could take one more parameter cacheBehavior which has 3 possible values Cache.always, Cache.never, Cache.auto.
Yes, retained rendering in my current experimental implementation means
On the framework side, maintain which Layer is dirty, and which Layer subtree has dirty Layers.
On the engine side, make std::shared_ptr<flow::Layer> instead of std::unique_ptr<flow:Layer> and send such shared layer pointer to the Dart side during the SceneBuilder::pushXXX call.
During Layer::addToScene phase, check if the subtree is dirty or not. If so, proceed as before. If not, reuse the shared layer pointer (retained layer subtree) and immediately stop the subtree walk from there.
During the Paint/Preroll of some special engine layer (e.g., OpacityLayer), take a raster snapshot of the whole subtree, and reuse it if the whole subtree is retained and the matrix is unchanged other than translation.
For flow::PhysicalShapeLayer in Fuchsia, we can skip the Paint entirely if the subtree is retained and the matrix is unchanged other than translation.
Will changing AnimatedOpacity alone be effective or will we also require that the child of AnimatedOpacity be wrapped in a RepaintBoundary?
Also, we should probably be consistent with the methods we provide to developers for controlling the raster cache. For example, we already have a concept of willChange. Perhaps we should use the same flag in AnimatedOpacity. Alternatively, we could change the existing flag to use the Cache enum before we freeze the API. I'm leaning towards the enum, because it is extensible in the future. A bool flag will be hard to extend.
Child of AnimatedOpacity definitely won't have to be wrapped in a RepaintBoundary because we already automatically consider the Opacity as a RepaintBoundary.
I also prefer willChange to take an enum, and reuse it in retained rendering.
we already automatically consider the Opacity as a RepaintBoundary.
Do you mean the engine treats it as a repaint boundary or the framework? I'm not aware of anything in the framework that treats it as a repaint boundary, and so the framework will hand you a new Picture on every frame. Perhaps I'm not looking in the right place?
AFAICT, alwaysNeedsCompositing does not affect what the framework repaints. It only causes an opacity layer created for the Opacity widget. When the opacity changes the opacity setter is called, which in turns requests a repaint. This will repaint the entire subtree under the Opacity widget and generate a brand new Picture, unless there's a RepaintBoundary to stop it.
BTW, RepaintBoundary may not be sufficient. You need something that will prevent re-layouts from being propagated from the child to the Opacity widget. IOW, something that causes RenderObject._relayoutBoundary to be true, such as a SizedBox or something.
Oh, you're right. In my experiments, the RepaintBoundary is not provided by Opacity or AnimatedOpacity, but by the ModalRoute. It works for me because my AnimatedOpacity is always applied to a page transition between two ModalRoute.
Considering the cost of Opacity and AnimatedOpacity, I wonder if we should set isRepaintBoundary => true there just like other heavy RenderObject such as TextureBox and RenderView.
In any case, I did plan to add contraints to engine OpacityLayer so it should only have one child instead of multiple children. I thought that this was always satisfied because of the RepaintBoundary and its single child OffsetLayer. If this is not the case, then I'll have to make it so by introducing RepaintBoundary.
So to answer your original question, yes we'll have Opacity and AnimatedOpacity to be RepaintBoundary so their children is wrapped into a single OffsetLayer.
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 -v and a minimal reproduction of the issue.
Activity
liyuqian commentedon Sep 12, 2018
CC @Hixie @cbracken @chinmaygarde @jason-simmons @yjbanov @amirh
Let's try to close this issue before the end of October so we can squeeze another 30% speedup?
We'll also have to check the memory usage (thanks Chinmay for the remind). My guess is that the memory cost should be much less compared to our current raster cache: in my experiments, I only add the raster cache when there's an animated opacity. Those things should be much rarer than RepaintBoundaries and PictureLayers. Chinmay and I also think that we shall provide the raster cache option to the developers just like what we did with clipping.
yjbanov commentedon Sep 13, 2018
What would the API look like?
liyuqian commentedon Sep 13, 2018
For example, the constructor of
AnimatedOpacitycould take one more parametercacheBehaviorwhich has 3 possible valuesCache.always, Cache.never, Cache.auto.liyuqian commentedon Sep 14, 2018
Just found a bug in my experimentation which caused my program to never reuse the cache...
So the 30% speedup is just by replacing
saveLayerwith taking snapshot and then paint. So this part is memory-free.After fixing the bug, the speedup is actually ~2.125x
Hixie commentedon Sep 17, 2018
Can you elaborate on what "retained rendering" entails?
liyuqian commentedon Sep 19, 2018
Yes, retained rendering in my current experimental implementation means
std::shared_ptr<flow::Layer>instead ofstd::unique_ptr<flow:Layer>and send such shared layer pointer to the Dart side during the SceneBuilder::pushXXX call.yjbanov commentedon Sep 19, 2018
Will changing
AnimatedOpacityalone be effective or will we also require that the child ofAnimatedOpacitybe wrapped in aRepaintBoundary?Also, we should probably be consistent with the methods we provide to developers for controlling the raster cache. For example, we already have a concept of willChange. Perhaps we should use the same flag in
AnimatedOpacity. Alternatively, we could change the existing flag to use theCacheenum before we freeze the API. I'm leaning towards the enum, because it is extensible in the future. Aboolflag will be hard to extend.liyuqian commentedon Sep 24, 2018
Child of
AnimatedOpacitydefinitely won't have to be wrapped in aRepaintBoundarybecause we already automatically consider theOpacityas aRepaintBoundary.I also prefer
willChangeto take an enum, and reuse it in retained rendering.yjbanov commentedon Sep 24, 2018
Do you mean the engine treats it as a repaint boundary or the framework? I'm not aware of anything in the framework that treats it as a repaint boundary, and so the framework will hand you a new
Pictureon every frame. Perhaps I'm not looking in the right place?liyuqian commentedon Sep 24, 2018
The framework has
alwaysNeedsCompositing = trueifOpacityhas child and it's not fully opaque or transparent: https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/rendering/proxy_box.dart#L732yjbanov commentedon Sep 24, 2018
AFAICT,
alwaysNeedsCompositingdoes not affect what the framework repaints. It only causes an opacity layer created for theOpacitywidget. When the opacity changes theopacitysetter is called, which in turns requests a repaint. This will repaint the entire subtree under theOpacitywidget and generate a brand newPicture, unless there's aRepaintBoundaryto stop it.BTW,
RepaintBoundarymay not be sufficient. You need something that will prevent re-layouts from being propagated from the child to theOpacitywidget. IOW, something that causesRenderObject._relayoutBoundaryto betrue, such as aSizedBoxor something.liyuqian commentedon Sep 25, 2018
Oh, you're right. In my experiments, the
RepaintBoundaryis not provided byOpacityorAnimatedOpacity, but by theModalRoute. It works for me because myAnimatedOpacityis always applied to a page transition between twoModalRoute.Considering the cost of
OpacityandAnimatedOpacity, I wonder if we should setisRepaintBoundary => truethere just like other heavyRenderObjectsuch asTextureBoxandRenderView.In any case, I did plan to add contraints to engine
OpacityLayerso it should only have one child instead of multiple children. I thought that this was always satisfied because of theRepaintBoundaryand its single childOffsetLayer. If this is not the case, then I'll have to make it so by introducingRepaintBoundary.So to answer your original question, yes we'll have
OpacityandAnimatedOpacityto beRepaintBoundaryso their children is wrapped into a singleOffsetLayer.Hixie commentedon Sep 25, 2018
I'm not sure I understand how the dart:ui API is expected to look after this change. Can you elaborate on this?
liyuqian commentedon Sep 26, 2018
For
CustomPaintwidget, we'll haveinstead of
as
isComplexandwillChangeonly affect raster cache behavior so it's probably more clear to just let developers decide whether to cache it or not.Similarly, for
Opacity/AnimatedOpacity, we could haveliyuqian commentedon Sep 27, 2018
As discussed with @Hixie , we probably won't change the APIs of
CustomPaint, and we'll probably addcacheBehaviortoRepaintBoundary.Don't push offset to leaf layers (#21619)
Share engine layers with the framework (#6406)
Allow raster caching any layer subtree (#6442)
Maintain dirtiness and use retained engine layers (#23434)
liyuqian commentedon Oct 25, 2018
#23434 completes this feature request. We'll track more exploratory work in retained rendering in a new issue: #23535
Maintain dirtiness and use retained engine layers (flutter#23434)
github-actions commentedon Sep 1, 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.