Flutter-Bounty-Hunters / follow_the_leader

MIT License
13 stars 5 forks source link

[Bug] - Toolbars are still pointing to the wrong place when the focal point moves #38

Closed matthew-carroll closed 1 year ago

matthew-carroll commented 1 year ago

We made a change in #32 to solve a problem where toolbars were pointing to the wrong place on frame one, and also solved some later focal point issues.

However, there's now an issue seen in SuperEditor and SuperReader where dragging a handle, expanding the selection bounds, doesn't result in a change to the focal offset. The arrow keeps pointing to the same global offset as before the selection was expanded. If I force a hot reload, the focal point updates. So something is going stale here.

After trying to replicate in overlord and super_editor, the best lead I have is that hiding and showing the toolbar results in a situation where either the toolbar no longer has a focal point, or has an old focal point:

https://github.com/Flutter-Bounty-Hunters/follow_the_leader/assets/7259036/6ec1dd18-e686-46ed-900f-97a80e95112a

I traced some of the offsets. It looks like in the Cupertino popover, the paint() method is obtaining the global offset of the focal point, and then converting it globalToLocal() to see where the focal point sits in relation to the toolbar. When we un-hide the toolbar, that first frame has the correct global offset, but it calculates the wrong local offset in globalToLocal().

Conclusion of Root Cause: This use-case involves hiding the Follower and then showing it again later. In this case, "hiding" the toolbar means removing it from the widget tree, which also removes it from the render tree and the layer tree. When we re-show the Follower we see the following order of operations:

  1. Build Leader
  2. Layout Leader
  3. Paint Leader
  4. Composite Leader (layer is already attached to Scene)
    1. Set leader properties on the LeaderLink
  5. Build Follower
  6. Layout Follower
  7. Paint Follower (where we position the toolbar arrow)
  8. Composite Follower (layer isn't attached to the Scene yet)
    1. Find path from Follower to root of scene and calculate the transform (can't do this until we're attached)

We face two problems: I don't think Flutter attaches new Layers until the end of the frame. I think it accumulates all the desired Layers and then does a single synchronization with the engine. Therefore, we can create a FollowerLayer in paint() but that Layer can't establish its transform until the next frame. Second, because RenderFollower can't force its FollowerLayer to attach to the Scene during paint(), the content in the Follower, e.g., the toolbar arrow, can't orient itself during paint().

We seem to be stuck in a situation where we MUST schedule another frame, after the FollowerLayer is attached to the Scene, at which point we can establish the transform from the FollowerLayer to the root, and then we can use that info in paint() to position the toolbar's arrow.

One move that might help this issue, in the average case, is to avoid detaching the Follower widget from the tree. I assume that if the widget isn't removed from the tree, then the FollowerLayer will retain state, and an extra frame won't be necessary upon showing the toolbar again.

matthew-carroll commented 1 year ago

I tried a couple approaches to keep the FollowerLayer attached to the root render view.

First, I statically retained all LayerHandle<FollowerLayer> and recycled them, instead of always creating new handles every time a RenderFollower is added to the tree. Retaining the LayerHandle did not result in keeping the layer attached to the root render view between usages.

Second, I wrapped the entire widget tree with a custom render object. In that custom render object I implemented the same kind of LayerHandle cache as above, and then I painted every cached layer with an opacity of zero, in the hopes that keeping the layers in the tree would keep them attached, and just reparent the layers between the layer cache render object, and the RenderFollowers that's actually use the layers. However, even in this case, the layer reports as unattached when it's obtained from the cache.

In the absence of any way to keep FollowerLayers attached to the render view, it seems that the best we can do is check in paint() if the layer is attached. If the layer isn't attached, schedule another frame to paint again when it is attached.