[CanvasKit] Only render one frame at a time. #50055
Conversation
Only have one additional queued frame. If more than one frame is requested while the current frame is rendering, only the last frame is queued.
harryterkelsen
changed the title
| ui.Scene scene, | ||
| ui.FlutterView view, | ||
| Completer<void> completer | ||
| })? _nextRender; |
There was a problem hiding this comment.
Can the record definition be moved into a typedef? Not only would that dedupe the definitions, but it would also allow the record structure to be documented. Example.
| if (_currentRender != null) { | ||
| // If a scene is already queued up, drop it and queue this one up instead | ||
| // so that the scene view always displays the most recently requested scene. | ||
| _nextRender?.completer.complete(); |
There was a problem hiding this comment.
Can this break multi-view? The framework may render multiple Scene objects into different views, all in the same frame (e.g. every view contains an animation). That would queue up multiple renders in the same frame.
If this is indeed an issue, we could store the currentRender and nextRender on the FlutterView object instead (or on the ViewRasterizer). This way, every view has its own queue that doesn't interfere with other views.
There was a problem hiding this comment.
Good catch. I'll make the queues per-view and add a test.
|
Thanks! PTAL @yjbanov |
| CkParagraphBuilder(style); | ||
|
|
||
| final Map<ui.FlutterView, _RenderQueue> _renderQueues = | ||
| <ui.FlutterView, _RenderQueue>{}; |
There was a problem hiding this comment.
Is there an advantage in putting the _RenderQueue in a map, compared to making it a field on ViewRasterizer (or FlutterView)? It seems the map requires that we handle more states than we need. For example, when nothing is rendering we can express it as _RenderQueue.current == null, but Map can also be in _renderQueues.contains(view) == false state, which also means the same thing. We also need to remember to remove the view after it's been disposed of (in fact, I don't see any code that does that).
This comment is non-blocking. Just trying to see if we can simplify it a bit.
There was a problem hiding this comment.
Good idea. This simplifies things quite a bit and lets us get away with not storing the view in the render request anymore, since it is already known by the view rasterizer.
…142662) flutter/engine@c83617e...c4247c5 2024-01-31 [email protected] Make screen reader announcement append a non-breaking space every other message. (flutter/engine#50151) 2024-01-31 [email protected] Roll Skia from 0ad5b2a9cebd to 19e5e8f089b2 (3 revisions) (flutter/engine#50214) 2024-01-31 [email protected] [CanvasKit] Only render one frame at a time. (flutter/engine#50055) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
4 participants
Only have one additional queued frame. If more than one frame is requested while the current frame is rendering, only the last frame is queued.
Fixes flutter/flutter#140981
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.