From 8eab70b759f372feeed4069bab8f6ed8b753391f Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Sun, 9 Oct 2022 12:13:11 -0500 Subject: [PATCH 1/2] fix(consumer): Don't compare node contents during tree update --- consumer/src/tree.rs | 48 +++----------------------------------------- 1 file changed, 3 insertions(+), 45 deletions(-) diff --git a/consumer/src/tree.rs b/consumer/src/tree.rs index e5cc97991..e91786fa5 100644 --- a/consumer/src/tree.rs +++ b/consumer/src/tree.rs @@ -113,11 +113,9 @@ impl State { orphans.insert(*child_id); } } - if *node_state.data != *node_data { - node_state.data = node_data; - if let Some(changes) = &mut changes { - changes.updated_node_ids.insert(node_id); - } + node_state.data = node_data; + if let Some(changes) = &mut changes { + changes.updated_node_ids.insert(node_id); } } else if let Some(parent_and_index) = pending_children.remove(&node_id) { add_node( @@ -676,44 +674,4 @@ mod tests { tree.read().node_by_id(NODE_ID_2).unwrap().name() ); } - - // Verify that if an update consists entirely of node data and tree data - // that's the same as before, no changes are reported. This would be useful - // for a provider that constructs a fresh tree every time, such as - // an immediate-mode GUI. - #[test] - fn no_change_update() { - let update = TreeUpdate { - nodes: vec![ - ( - NODE_ID_1, - Arc::new(Node { - role: Role::Window, - children: vec![NODE_ID_2, NODE_ID_3], - ..Default::default() - }), - ), - ( - NODE_ID_2, - Arc::new(Node { - role: Role::Button, - ..Default::default() - }), - ), - ( - NODE_ID_3, - Arc::new(Node { - role: Role::Button, - ..Default::default() - }), - ), - ], - tree: Some(Tree::new(NODE_ID_1)), - focus: Some(NODE_ID_2), - }; - let tree = super::Tree::new(update.clone(), Box::new(NullActionHandler {})); - tree.update_and_process_changes(update, |_| { - panic!("expected no changes"); - }); - } } From c28c2b256d6b5f849b04f9ba2b1858cd1381dab3 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Sun, 9 Oct 2022 13:06:24 -0500 Subject: [PATCH 2/2] Add a node on performance to the TreeUpdate documentation --- common/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/src/lib.rs b/common/src/lib.rs index 6455579bf..746f32140 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -1198,6 +1198,12 @@ impl Tree { /// The sender and receiver must be in sync; the update is only meant /// to bring the tree from a specific previous state into its next state. /// Trying to apply it to the wrong tree should immediately panic. +/// +/// Note that for performance, an update should only include nodes that are +/// new or changed. AccessKit platform adapters will avoid raising extraneous +/// events for nodes that have not changed since the previous update, +/// but there is still a cost in processing these nodes and replacing +/// the previous instances. #[derive(Clone, Debug, Default, PartialEq)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "schemars", derive(JsonSchema))]