-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Next steps for tree structure tracking #54
Comments
For reference, there's code to track tree structure in the lasagna branch of Druid. The actual data structure can likely be adapted from that. |
I think this should happen during tree marking. The Pod could also set
I guess we could send the
I think this should work currently. The |
Ah yes. I went over this more carefully, and I see that it's quite fine. In fact, the code is in pretty good shape. The only things that really needs improvement is that all ancestors of a container with So at this point, I prefer doing finer grained tracking as proposed here, but I don't think it's absolutely required, and not doing it (or planning to do it later) would be less of a diff against the Druid code. |
@raphlinus Can you please clarify what you mean with the following (as I'm currently investigating the whole topic a little bit):
|
For sure. The specific goal I'm addressing is to distinguish the cases where the vector of a node's immediate children have changed, as opposed to the case where the tree structure has changed of any of the descendants. The latter is needed so we can reach all changes in a recursive traversal, but (I think) there is valuable work to be saved by not recalculating anything based on the immediate children vector in that case. The general pattern to achieve this is to have two separate flags, and only one of them in the |
This is an outline for how I think we should track tree structure in the widget tree.
A number of operations on the view side can change the widget tree structure. The widget tree doesn't change structure by itself, but only in response to mutations from the view side. Those operations are, roughly: an
AnyView
changing view type, andOption<V>
switching betweenNone
andSome
, and a variable length implementor ofViewSequence
(such asVec
) changing length. The goal is for those view operations to mark the tree precisely, then for a traversal of the tree to update accessibility and internal invariants.Tree marking
The desired result of tree marking is as follows: every container that has had its list of children changed is marked with
PodFlags::TREE_CHANGED
, and all of its ancestors are marked with the corresponding upward flag; we could create a new one, or useUPDATE
for this purpose.Any
ViewSequence
implementation that changes its count should signalChangeFlags::TREE
, as shouldAnyView
on type change. The tree marking is currently done by therebuild
method in the blanket impl ofViewSequence
forV: View
, though this plumbing is in a state of flux. We should not propagateTREE_CHANGED
orTREE
upward (it is currently inUPWARD_FLAGS
) but should instead propagate the corresponding upward flag.Accessibility
A container generally needs to update its accessibility node in several cases: its geometry changed or its children changed. Probably the best way to do this is to have the framework set
REQUEST_ACCESSIBILITY
(and the corresponding upward flag) in both these cases, so widgets don't have to have this logic themselves.Open question: should that be done during tree marking, or perhaps in a separate traversal? The
update
traversal would be a natural place, but it's not obvious we're going to keep that.WidgetAdded
Druid sends a
WidgetAdded
lifecycle event to widgets newly added to the tree. This is propagated from several different traversals. I think it's likely we'll want to send such a lifecycle event also (though I'm not 100% sure it's still needed). We need to figure out which traversal it is. Druid uses the presence of the data field; we'll either need a flag inPodFlags
indicating whether the widget has been initialized, or make use of structure tracking.Druid also adds children to the Bloom filter at the same time. The way Bloom filters work, widgets can't be removed (I believe this is a bug in the existing xilem code, but haven't validated it carefully).
Structure tracking
One thing Druid does not do, that I think we should, is track the precise parent/child tree structure in the framework. I propose we have two hash tables, one from child widget ids to parent, and another from parent widget id to a
Vec
of child widget ids. This would be accessible from most contexts, and mutable from the context that propagates tree changes (in my mind, this is most naturally theupdate
context, but my mind can be changed).Generally it is the responsibility of container widgets to call a
set_children()
method on the context when the children have changed (using essentially the same logic as for accessibility), but it's possible we'll also want to have delta methods such asadd_child
orremove_child
.This could also be the locus from which
WidgetAdded
is generated.Structure tracking would obviate the need for Bloom filters, and those should go away. For example, when propagating an accessibility event, the widget id provided in that event can be expanded to an id path using that structure info (basically following the path through the parent chain up to the root), and used to dispatch precisely.
I haven't analyzed the focus chain logic closely, but I think a lot of that could be downstream of precisely tracked structure, rather than relying on the existing
children_changed
/register_child
/register_for_focus
mechanism. I think we're already paying much of the cost in both runtime and code complexity for doing this tracking, just not getting the full benefit.The text was updated successfully, but these errors were encountered: