superlistapp / super_editor

A Flutter toolkit for building document editors and readers
https://superlist.com/SuperEditor/
MIT License
1.69k stars 247 forks source link

[SuperEditor] Performance issues on mobile when opening keyboard #1576

Open robertodoering opened 1 year ago

robertodoering commented 1 year ago

I was noticing slowdown in superlist when the virtual keyboard opens on mobile in a document (more so on Android than on iOS).

After profiling, I noticed a lot of cpu time was spend in the FollowerLayer from follow_the_leader, which is used to position the caret and handle.

I assume this is contributing to the slowdown, especially when the cursor moves during the keyboard animation and might be a target for optimization.

Here's a 1.7s profiler export from running the super_editor example on my Android phone, which includes opening the keyboard once. dart_devtools_2023-10-27_17_51_06.034.json

Notably, 700ms out of the 1.7s are spent in the FollowerLayer. More than 10% of that is spent stringifying a Matrix4 for a log message that is then subsequently discarded.

image

matthew-carroll commented 1 year ago

WRT the log statement, it certainly shouldn't take any effort to fix that. The rest might be tricky. Implementing follow_the_leader required a lot of hacking of behavior that's very poorly documented so it's easy to break the whole thing when making minor adjustments to the behavior.

Any chance that anyone on the Superlist side has experience compositing layers during the paint phase, and knows the ins and outs of Flutter's transformation system?

matthew-carroll commented 11 months ago

@robertodoering do you know if anyone has talked to @knopp about investigating this in follow_the_leader? I think @miguelcmedeiros or @brian-superlist may have mentioned a desire to add his analysis to how follow_the_leader tracks transforms.

knopp commented 11 months ago

I have not been looking into that. But I do think that with OverlayPortal there might now be simpler (and probably better *) way to position popovers. I've proof of concept implementation here, with online example. (try resizing the window to see how the popover and popover callout reposition).

This is the widget hierarchy

PopoverAnchor
   OverlayPortal
      AnchorWidget

Where AnchorWidget is user supplied widget to which the popover should be anchored to. The OverlayPortal creates an overlay with a root CustomMultiChildLayoutWidget responsible for positioning of the overlay.

The main reason why CustomMultiChildLayoutWidget inside overlay can calculate layout relative to AnchorWidget is that a) it is a layout boundary and b) because of OverlayPortal it is part of element and render object tree and has depth that is always larger than layout boundary to which AnchorWidget belongs. That means that the layout method of CustomMultiChildLayoutWidget is always ran after the layout of AnchorWidget (and ancestors) is completed.

Although because it is possible that the paint coordinates of AnchorWidget to change even without layout (i.e. during scrolling), every time overlay is visible a persistent frame callback is registered to invalidate overlay layout. So the overlay layout is recomputed during every frame. But because the overlay widget is a layout boundary and the overlay itself is a layout boundary, this should be very cheap.

*) The main benefit of this approach is that it requires much less work than follower layer, and it should never lag one frame behind. Main drawback is that it's not exactly battle tested and there is a good chance that I have some assumption somewhere wrong.

Just some food for though :)