Reuse property buffers when cloning nodes

accesskit/src/lib.rs

@@ -857,14 +857,14 @@ enum PropertyValue {
     None,
     NodeIdVec(Vec<NodeId>),
     NodeId(NodeId),
-    String(Box<str>),
+    String(String),
     F64(f64),
     F32(f32),
     Usize(usize),
     Color(Color),
     TextDecoration(TextDecoration),
-    LengthSlice(Box<[u8]>),
-    CoordSlice(Box<[f32]>),
+    LengthVec(Vec<u8>),
+    CoordVec(Vec<f32>),
     Bool(bool),
     Invalid(Invalid),
     Toggled(Toggled),
@@ -970,11 +970,11 @@ enum PropertyId {
     Strikethrough,
     Underline,
 
-    // LengthSlice
+    // LengthVec
     CharacterLengths,
     WordStarts,
 
-    // CoordSlice
+    // CoordVec
     CharacterPositions,
     CharacterWidths,
 
@@ -1007,6 +1007,21 @@ enum PropertyId {
     Unset,
 }
 
+impl PropertyValue {
+    fn clone_from_reusing_buffer(&mut self, source: &Self) {
+        match (self, source) {
+            (Self::NodeIdVec(dest), Self::NodeIdVec(source)) => dest.clone_from(source),
+            (Self::String(dest), Self::String(source)) => dest.clone_from(source),
+            (Self::LengthVec(dest), Self::LengthVec(source)) => dest.clone_from(source),
+            (Self::CoordVec(dest), Self::CoordVec(source)) => dest.clone_from(source),
+            (Self::CustomActionVec(dest), Self::CustomActionVec(source)) => dest.clone_from(source),
+            (Self::Affine(dest), Self::Affine(source)) => dest.clone_from(source),
+            (Self::TextSelection(dest), Self::TextSelection(source)) => dest.clone_from(source),
+            (dest, source) => *dest = source.clone(),
+        }
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 #[repr(transparent)]
 struct PropertyIndices([u8; PropertyId::Unset as usize]);
@@ -1017,19 +1032,38 @@ impl Default for PropertyIndices {
     }
 }
 
-#[derive(Clone, Debug, Default, PartialEq)]
+#[derive(Debug, Default, PartialEq)]
 struct Properties {
     indices: PropertyIndices,
     values: Vec<PropertyValue>,
 }
 
+impl Clone for Properties {
+    fn clone(&self) -> Self {
+        Self {
+            indices: self.indices,
+            values: self.values.clone(),
+        }
+    }
+
+    fn clone_from(&mut self, source: &Self) {
+        self.indices = source.indices;
+        let reusable = self.values.len().min(source.values.len());
+        for (dest, value) in self.values.iter_mut().zip(&source.values) {
+            dest.clone_from_reusing_buffer(value);
+        }
+        self.values.truncate(source.values.len());
+        self.values.extend_from_slice(&source.values[reusable..]);
+    }
+}
+
 /// A single accessible object. A complete UI is represented as a tree of these.
 ///
 /// For brevity, and to make more of the documentation usable in bindings
 /// to other languages, documentation of getter methods is written as if
 /// documenting fields in a struct, and such methods are referred to
 /// as properties.
-#[derive(Clone, Default, PartialEq)]
+#[derive(Default, PartialEq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "schemars", derive(JsonSchema))]
 #[cfg_attr(feature = "serde", serde(deny_unknown_fields))]
@@ -1042,6 +1076,26 @@ pub struct Node {
     properties: Properties,
 }
 
+impl Clone for Node {
+    fn clone(&self) -> Self {
+        Self {
+            role: self.role,
+            actions: self.actions,
+            child_actions: self.child_actions,
+            flags: self.flags,
+            properties: self.properties.clone(),
+        }
+    }
+
+    fn clone_from(&mut self, source: &Self) {
+        self.role = source.role;
+        self.actions = source.actions;
+        self.child_actions = source.child_actions;
+        self.flags = source.flags;
+        self.properties.clone_from(&source.properties);
+    }
+}
+
 impl PropertyIndices {
     fn get<'a>(&self, values: &'a [PropertyValue], id: PropertyId) -> &'a PropertyValue {
         let index = self.0[id as usize];
@@ -1141,7 +1195,7 @@ macro_rules! option_ref_type_getters {
         impl PropertyIndices {
             $(fn $method<'a>(&self, values: &'a [PropertyValue], id: PropertyId) -> Option<&'a $type> {
                 match self.get(values, id) {
-                    PropertyValue::$variant(value) => Some(value),
+                    PropertyValue::$variant(value) => Some(&**value),
                     _ => None,
                 }
             })*
@@ -1175,10 +1229,10 @@ macro_rules! copy_type_getters {
     }
 }
 
-macro_rules! box_type_setters {
+macro_rules! owned_type_setters {
     ($(($method:ident, $type:ty, $variant:ident)),+) => {
         impl Node {
-            $(fn $method(&mut self, id: PropertyId, value: impl Into<Box<$type>>) {
+            $(fn $method(&mut self, id: PropertyId, value: impl Into<$type>) {
                 self.properties.set(id, PropertyValue::$variant(value.into()));
             })*
         }
@@ -1357,7 +1411,7 @@ macro_rules! string_property_methods {
     ($($(#[$doc:meta])* ($id:ident, $getter:ident, $setter:ident, $clearer:ident)),+) => {
         $(property_methods! {
             $(#[$doc])*
-            ($id, $getter, get_string_property, Option<&str>, $setter, set_string_property, impl Into<Box<str>>, $clearer)
+            ($id, $getter, get_string_property, Option<&str>, $setter, set_string_property, impl Into<String>, $clearer)
         })*
         impl Node {
             option_properties_debug_method! { debug_string_properties, [$($getter,)*] }
@@ -1577,7 +1631,7 @@ macro_rules! length_slice_property_methods {
     ($($(#[$doc:meta])* ($id:ident, $getter:ident, $setter:ident, $clearer:ident)),+) => {
         $(property_methods! {
             $(#[$doc])*
-            ($id, $getter, get_length_slice_property, &[u8], $setter, set_length_slice_property, impl Into<Box<[u8]>>, $clearer)
+            ($id, $getter, get_length_vec_property, &[u8], $setter, set_length_vec_property, impl Into<Vec<u8>>, $clearer)
         })*
         impl Node {
             slice_properties_debug_method! { debug_length_slice_properties, [$($getter,)*] }
@@ -1614,7 +1668,7 @@ macro_rules! coord_slice_property_methods {
     ($($(#[$doc:meta])* ($id:ident, $getter:ident, $setter:ident, $clearer:ident)),+) => {
         $(property_methods! {
             $(#[$doc])*
-            ($id, $getter, get_coord_slice_property, Option<&[f32]>, $setter, set_coord_slice_property, impl Into<Box<[f32]>>, $clearer)
+            ($id, $getter, get_coord_vec_property, Option<&[f32]>, $setter, set_coord_vec_property, impl Into<Vec<f32>>, $clearer)
         })*
         impl Node {
             option_properties_debug_method! { debug_coord_slice_properties, [$($getter,)*] }
@@ -1832,12 +1886,12 @@ flag_methods! {
 option_ref_type_getters! {
     (get_affine_property, Affine, Affine),
     (get_string_property, str, String),
-    (get_coord_slice_property, [f32], CoordSlice),
+    (get_coord_vec_property, [f32], CoordVec),
     (get_text_selection_property, TextSelection, TextSelection)
 }
 
 slice_type_getters! {
-    (get_length_slice_property, u8, LengthSlice)
+    (get_length_vec_property, u8, LengthVec)
 }
 
 copy_type_getters! {
@@ -1852,12 +1906,12 @@ copy_type_getters! {
     (get_tree_id_property, TreeId, TreeId)
 }
 
-box_type_setters! {
-    (set_affine_property, Affine, Affine),
-    (set_string_property, str, String),
-    (set_length_slice_property, [u8], LengthSlice),
-    (set_coord_slice_property, [f32], CoordSlice),
-    (set_text_selection_property, TextSelection, TextSelection)
+owned_type_setters! {
+    (set_affine_property, Box<Affine>, Affine),
+    (set_string_property, String, String),
+    (set_length_vec_property, Vec<u8>, LengthVec),
+    (set_coord_vec_property, Vec<f32>, CoordVec),
+    (set_text_selection_property, Box<TextSelection>, TextSelection)
 }
 
 copy_type_setters! {
@@ -2596,8 +2650,8 @@ impl Serialize for Properties {
                 Usize,
                 Color,
                 TextDecoration,
-                LengthSlice,
-                CoordSlice,
+                LengthVec,
+                CoordVec,
                 Bool,
                 Invalid,
                 Toggled,
@@ -2720,11 +2774,11 @@ impl<'de> Visitor<'de> for PropertiesVisitor {
                     Strikethrough,
                     Underline
                 },
-                LengthSlice {
+                LengthVec {
                     CharacterLengths,
                     WordStarts
                 },
-                CoordSlice {
+                CoordVec {
                     CharacterPositions,
                     CharacterWidths
                 },
@@ -2812,7 +2866,7 @@ impl JsonSchema for Properties {
                 PreviousOnLine,
                 PopupFor
             },
-            Box<str> {
+            String {
                 Label,
                 Description,
                 Value,
@@ -2872,11 +2926,11 @@ impl JsonSchema for Properties {
                 Strikethrough,
                 Underline
             },
-            Box<[u8]> {
+            Vec<u8> {
                 CharacterLengths,
                 WordStarts
             },
-            Box<[f32]> {
+            Vec<f32> {
                 CharacterPositions,
                 CharacterWidths
             },
@@ -3138,6 +3192,53 @@ mod tests {
     use super::*;
     use alloc::format;
 
+    #[test]
+    fn clone_from_should_be_equivalent_to_clone() {
+        let mut source = Node::new(Role::Button);
+        source.add_action(Action::Click);
+        source.add_child_action(Action::Focus);
+        source.set_hidden();
+        source.set_label("source");
+        source.set_children([NodeId(1)]);
+
+        let mut dest = Node::new(Role::CheckBox);
+        dest.add_action(Action::Focus);
+        dest.set_multiselectable();
+        dest.set_description("dest");
+        dest.set_children([NodeId(7), NodeId(8)]);
+        dest.clone_from(&source);
+
+        assert_eq!(dest, source.clone());
+    }
+
+    #[test]
+    fn clone_from_should_reuse_the_destination_string_buffer() {
+        let mut source = Node::new(Role::Button);
+        source.set_label("new label");
+        let mut dest = Node::new(Role::Button);
+        dest.set_label("old label with room to spare");
+        let buffer = dest.label().unwrap().as_ptr();
+
+        dest.clone_from(&source);
+
+        assert_eq!(dest.label(), Some("new label"));
+        assert_eq!(dest.label().unwrap().as_ptr(), buffer);
+    }
+
+    #[test]
+    fn clone_from_should_reuse_the_destination_slice_buffer() {
+        let mut source = Node::new(Role::TextRun);
+        source.set_character_lengths([1u8, 2, 3]);
+        let mut dest = Node::new(Role::TextRun);
+        dest.set_character_lengths([9u8; 16]);
+        let buffer = dest.character_lengths().as_ptr();
+
+        dest.clone_from(&source);
+
+        assert_eq!(dest.character_lengths(), &[1, 2, 3]);
+        assert_eq!(dest.character_lengths().as_ptr(), buffer);
+    }
+
     #[test]
     fn u64_should_be_convertible_to_node_id() {
         assert_eq!(NodeId::from(0u64), NodeId(0));

accesskit_consumer/src/node.rs

@@ -54,12 +54,26 @@ impl From<FullNodeId> for u128 {
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub(crate) struct ParentAndIndex(pub(crate) FullNodeId, pub(crate) usize);
 
-#[derive(Clone, Debug)]
+#[derive(Debug)]
 pub(crate) struct NodeState {
     pub(crate) parent_and_index: Option<ParentAndIndex>,
     pub(crate) data: Node,
 }
 
+impl Clone for NodeState {
+    fn clone(&self) -> Self {
+        Self {
+            parent_and_index: self.parent_and_index,
+            data: self.data.clone(),
+        }
+    }
+
+    fn clone_from(&mut self, source: &Self) {
+        self.parent_and_index = source.parent_and_index;
+        self.data.clone_from(&source.data);
+    }
+}
+
 #[derive(Copy, Clone, Debug)]
 pub struct NodeRef<'a> {
     pub tree_state: &'a TreeState,

accesskit_consumer/src/tree.rs

@@ -14,13 +14,29 @@ use crate::node::{FullNodeId, NodeRef, NodeState, ParentAndIndex};
 #[repr(transparent)]
 pub(crate) struct TreeIndex(pub(crate) u32);
 
-#[derive(Clone, Debug, Default)]
+#[derive(Debug, Default)]
 struct TreeIndexMap {
     id_to_index: HashMap<TreeId, TreeIndex>,
     index_to_id: HashMap<TreeIndex, TreeId>,
     next: u32,
 }
 
+impl Clone for TreeIndexMap {
+    fn clone(&self) -> Self {
+        Self {
+            id_to_index: self.id_to_index.clone(),
+            index_to_id: self.index_to_id.clone(),
+            next: self.next,
+        }
+    }
+
+    fn clone_from(&mut self, source: &Self) {
+        self.id_to_index.clone_from(&source.id_to_index);
+        self.index_to_id.clone_from(&source.index_to_id);
+        self.next = source.next;
+    }
+}
+
 impl TreeIndexMap {
     fn get_or_create_index(&mut self, id: TreeId) -> TreeIndex {
         *self.id_to_index.entry(id).or_insert_with(|| {
@@ -275,7 +291,7 @@ impl TreeState {
                             record_graft(&mut pending_grafts, new_subtree_id, node_id);
                         }
                     }
-                    node_state.data.clone_from(&node_data);
+                    node_state.data = node_data;
                     if let Some(changes) = &mut changes {
                         changes.updated_node_ids.insert(node_id);
                     }