From 74537b7f13ec92006f72743a1d85e57625c92d9e Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 4 Oct 2022 15:30:36 -0500 Subject: [PATCH 1/2] refactor!: Store node ID in `TreeUpdate`, not `accesskit::Node` --- common/src/lib.rs | 151 +---------------- consumer/src/lib.rs | 65 ++++--- consumer/src/node.rs | 83 ++++++--- consumer/src/tree.rs | 197 ++++++++++++++++------ platforms/windows/examples/hello_world.rs | 20 ++- platforms/windows/src/tests/simple.rs | 18 +- platforms/windows/src/tests/subclassed.rs | 18 +- platforms/winit/examples/simple.rs | 20 ++- 8 files changed, 297 insertions(+), 275 deletions(-) diff --git a/common/src/lib.rs b/common/src/lib.rs index 34a2ffb9f..15000e4d7 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -34,12 +34,13 @@ use std::{ /// is ordered roughly by expected usage frequency (with the notable exception /// of [`Role::Unknown`]). This is more efficient in serialization formats /// where integers use a variable-length encoding. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "schemars", derive(JsonSchema))] #[cfg_attr(feature = "serde", serde(crate = "serde"))] #[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))] pub enum Role { + #[default] Unknown, InlineTextBox, Cell, @@ -654,14 +655,13 @@ pub struct TextSelection { } /// A single accessible object. A complete UI is represented as a tree of these. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, Default, PartialEq)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "schemars", derive(JsonSchema))] #[cfg_attr(feature = "serde", serde(crate = "serde"))] #[cfg_attr(feature = "serde", serde(deny_unknown_fields))] #[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))] pub struct Node { - pub id: NodeId, pub role: Role, /// An affine transform to apply to any coordinates within this node /// and its descendants, including the [`bounds`] field of this node. @@ -1165,149 +1165,6 @@ pub struct Node { pub text_indent: Option, } -impl Node { - pub fn new(id: NodeId, role: Role) -> Node { - Node { - id, - role, - transform: None, - bounds: None, - children: Default::default(), - actions: EnumSet::new(), - name: None, - name_from: None, - description: None, - description_from: None, - value: None, - autofill_available: false, - expanded: None, - default: false, - editable: false, - focusable: false, - orientation: None, - hovered: false, - ignored: false, - invisible: false, - linked: false, - multiline: false, - multiselectable: false, - protected: false, - required: false, - visited: false, - busy: false, - nonatomic_text_field_root: false, - live_atomic: false, - modal: false, - canvas_has_fallback: false, - scrollable: false, - clickable: false, - clips_children: false, - not_user_selectable_style: false, - selected: None, - selected_from_focus: false, - grabbed: None, - drop_effects: EnumSet::new(), - is_line_breaking_object: false, - is_page_breaking_object: false, - has_aria_attribute: false, - touch_pass_through: false, - indirect_children: Default::default(), - active_descendant: None, - error_message: None, - in_page_link_target: None, - member_of: None, - next_on_line: None, - previous_on_line: None, - popup_for: None, - controls: Default::default(), - details: Default::default(), - described_by: Default::default(), - flow_to: Default::default(), - labelled_by: Default::default(), - radio_group: Default::default(), - markers: Default::default(), - text_direction: None, - character_offsets: Default::default(), - words: Default::default(), - custom_actions: Default::default(), - access_key: None, - invalid_state: None, - auto_complete: None, - checked_state: None, - checked_state_description: None, - class_name: None, - css_display: None, - font_family: None, - html_tag: None, - inner_html: None, - input_type: None, - key_shortcuts: None, - language: None, - live_relevant: None, - live: None, - placeholder: None, - aria_role: None, - role_description: None, - tooltip: None, - url: None, - default_action_verb: None, - scroll_x: None, - scroll_x_min: None, - scroll_x_max: None, - scroll_y: None, - scroll_y_min: None, - scroll_y_max: None, - text_selection: None, - aria_column_count: None, - aria_cell_column_index: None, - aria_cell_column_span: None, - aria_row_count: None, - aria_cell_row_index: None, - aria_cell_row_span: None, - table_row_count: None, - table_column_count: None, - table_header: None, - table_row_index: None, - table_row_header: None, - table_column_index: None, - table_column_header: None, - table_cell_column_index: None, - table_cell_column_span: None, - table_cell_row_index: None, - table_cell_row_span: None, - sort_direction: None, - hierarchical_level: None, - read_only: false, - disabled: false, - set_size: None, - pos_in_set: None, - color_value: None, - aria_current: None, - background_color: None, - foreground_color: None, - has_popup: None, - list_style: None, - text_align: None, - vertical_offset: None, - bold: false, - italic: false, - overline: None, - strikethrough: None, - underline: None, - previous_focus: None, - next_focus: None, - numeric_value: None, - min_numeric_value: None, - max_numeric_value: None, - numeric_value_step: None, - numeric_value_jump: None, - font_size: None, - font_weight: None, - text_indent: None, - } - } -} - /// The data associated with an accessibility tree that's global to the /// tree and not associated with any particular node. #[derive(Clone, Debug, PartialEq, Eq)] @@ -1364,7 +1221,7 @@ pub struct TreeUpdate { /// placeholder must be updated within the same `TreeUpdate`, otherwise /// it's a fatal error. This guarantees the tree is always complete /// before or after a `TreeUpdate`. - pub nodes: Vec, + pub nodes: Vec<(NodeId, Node)>, /// Rarely updated information about the tree as a whole. This may be omitted /// if it has not changed since the previous update, but providing the same diff --git a/consumer/src/lib.rs b/consumer/src/lib.rs index 254598057..cf27893df 100644 --- a/consumer/src/lib.rs +++ b/consumer/src/lib.rs @@ -48,24 +48,28 @@ mod tests { pub fn test_tree() -> Arc { let root = Node { + role: Role::RootWebArea, children: vec![ PARAGRAPH_0_ID, PARAGRAPH_1_IGNORED_ID, PARAGRAPH_2_ID, PARAGRAPH_3_IGNORED_ID, ], - ..Node::new(ROOT_ID, Role::RootWebArea) + ..Default::default() }; let paragraph_0 = Node { + role: Role::Paragraph, children: vec![STATIC_TEXT_0_0_IGNORED_ID], - ..Node::new(PARAGRAPH_0_ID, Role::Paragraph) + ..Default::default() }; let static_text_0_0_ignored = Node { + role: Role::StaticText, ignored: true, name: Some("static_text_0_0_ignored".into()), - ..Node::new(STATIC_TEXT_0_0_IGNORED_ID, Role::StaticText) + ..Default::default() }; let paragraph_1_ignored = Node { + role: Role::Paragraph, transform: Some(Box::new(Affine::translate(Vec2::new(10.0, 40.0)))), bounds: Some(Rect { x0: 0.0, @@ -75,9 +79,10 @@ mod tests { }), children: vec![STATIC_TEXT_1_0_ID], ignored: true, - ..Node::new(PARAGRAPH_1_IGNORED_ID, Role::Paragraph) + ..Default::default() }; let static_text_1_0 = Node { + role: Role::StaticText, bounds: Some(Rect { x0: 10.0, y0: 10.0, @@ -85,17 +90,20 @@ mod tests { y1: 30.0, }), name: Some("static_text_1_0".into()), - ..Node::new(STATIC_TEXT_1_0_ID, Role::StaticText) + ..Default::default() }; let paragraph_2 = Node { + role: Role::Paragraph, children: vec![STATIC_TEXT_2_0_ID], - ..Node::new(PARAGRAPH_2_ID, Role::Paragraph) + ..Default::default() }; let static_text_2_0 = Node { + role: Role::StaticText, name: Some("static_text_2_0".into()), - ..Node::new(STATIC_TEXT_2_0_ID, Role::StaticText) + ..Default::default() }; let paragraph_3_ignored = Node { + role: Role::Paragraph, children: vec![ EMPTY_CONTAINER_3_0_IGNORED_ID, LINK_3_1_IGNORED_ID, @@ -103,45 +111,50 @@ mod tests { EMPTY_CONTAINER_3_3_IGNORED_ID, ], ignored: true, - ..Node::new(PARAGRAPH_3_IGNORED_ID, Role::Paragraph) + ..Default::default() }; let empty_container_3_0_ignored = Node { + role: Role::GenericContainer, ignored: true, - ..Node::new(EMPTY_CONTAINER_3_0_IGNORED_ID, Role::GenericContainer) + ..Default::default() }; let link_3_1_ignored = Node { + role: Role::Link, children: vec![STATIC_TEXT_3_1_0_ID], ignored: true, linked: true, - ..Node::new(LINK_3_1_IGNORED_ID, Role::Link) + ..Default::default() }; let static_text_3_1_0 = Node { + role: Role::StaticText, name: Some("static_text_3_1_0".into()), - ..Node::new(STATIC_TEXT_3_1_0_ID, Role::StaticText) + ..Default::default() }; let button_3_2 = Node { + role: Role::Button, name: Some("button_3_2".into()), - ..Node::new(BUTTON_3_2_ID, Role::Button) + ..Default::default() }; let empty_container_3_3_ignored = Node { + role: Role::GenericContainer, ignored: true, - ..Node::new(EMPTY_CONTAINER_3_3_IGNORED_ID, Role::GenericContainer) + ..Default::default() }; let initial_update = TreeUpdate { nodes: vec![ - root, - paragraph_0, - static_text_0_0_ignored, - paragraph_1_ignored, - static_text_1_0, - paragraph_2, - static_text_2_0, - paragraph_3_ignored, - empty_container_3_0_ignored, - link_3_1_ignored, - static_text_3_1_0, - button_3_2, - empty_container_3_3_ignored, + (ROOT_ID, root), + (PARAGRAPH_0_ID, paragraph_0), + (STATIC_TEXT_0_0_IGNORED_ID, static_text_0_0_ignored), + (PARAGRAPH_1_IGNORED_ID, paragraph_1_ignored), + (STATIC_TEXT_1_0_ID, static_text_1_0), + (PARAGRAPH_2_ID, paragraph_2), + (STATIC_TEXT_2_0_ID, static_text_2_0), + (PARAGRAPH_3_IGNORED_ID, paragraph_3_ignored), + (EMPTY_CONTAINER_3_0_IGNORED_ID, empty_container_3_0_ignored), + (LINK_3_1_IGNORED_ID, link_3_1_ignored), + (STATIC_TEXT_3_1_0_ID, static_text_3_1_0), + (BUTTON_3_2_ID, button_3_2), + (EMPTY_CONTAINER_3_3_IGNORED_ID, empty_container_3_3_ignored), ], tree: Some(Tree::new(ROOT_ID)), focus: None, diff --git a/consumer/src/node.rs b/consumer/src/node.rs index 4b437cc08..ee3890264 100644 --- a/consumer/src/node.rs +++ b/consumer/src/node.rs @@ -26,6 +26,7 @@ use crate::NodeData; #[derive(Copy, Clone)] pub struct Node<'a> { pub tree_reader: &'a TreeReader<'a>, + pub(crate) id: NodeId, pub(crate) state: &'a NodeState, } @@ -278,7 +279,7 @@ impl<'a> Node<'a> { // Convenience getters pub fn id(&self) -> NodeId { - self.data().id + self.id } pub fn role(&self) -> Role { @@ -794,11 +795,21 @@ mod tests { fn no_name_or_labelled_by() { let update = TreeUpdate { nodes: vec![ - Node { - children: vec![NODE_ID_2], - ..Node::new(NODE_ID_1, Role::Window) - }, - Node::new(NODE_ID_2, Role::Button), + ( + NODE_ID_1, + Node { + role: Role::Window, + children: vec![NODE_ID_2], + ..Default::default() + }, + ), + ( + NODE_ID_2, + Node { + role: Role::Button, + ..Default::default() + }, + ), ], tree: Some(Tree::new(NODE_ID_1)), focus: None, @@ -816,26 +827,46 @@ mod tests { let update = TreeUpdate { nodes: vec![ - Node { - children: vec![NODE_ID_2, NODE_ID_3, NODE_ID_4, NODE_ID_5], - ..Node::new(NODE_ID_1, Role::Window) - }, - Node { - labelled_by: vec![NODE_ID_3, NODE_ID_5], - ..Node::new(NODE_ID_2, Role::CheckBox) - }, - Node { - name: Some(LABEL_1.into()), - ..Node::new(NODE_ID_3, Role::StaticText) - }, - Node { - labelled_by: vec![NODE_ID_5], - ..Node::new(NODE_ID_4, Role::CheckBox) - }, - Node { - name: Some(LABEL_2.into()), - ..Node::new(NODE_ID_5, Role::StaticText) - }, + ( + NODE_ID_1, + Node { + role: Role::Window, + children: vec![NODE_ID_2, NODE_ID_3, NODE_ID_4, NODE_ID_5], + ..Default::default() + }, + ), + ( + NODE_ID_2, + Node { + role: Role::CheckBox, + labelled_by: vec![NODE_ID_3, NODE_ID_5], + ..Default::default() + }, + ), + ( + NODE_ID_3, + Node { + role: Role::StaticText, + name: Some(LABEL_1.into()), + ..Default::default() + }, + ), + ( + NODE_ID_4, + Node { + role: Role::CheckBox, + labelled_by: vec![NODE_ID_5], + ..Default::default() + }, + ), + ( + NODE_ID_5, + Node { + role: Role::StaticText, + name: Some(LABEL_2.into()), + ..Default::default() + }, + ), ], tree: Some(Tree::new(NODE_ID_1)), focus: None, diff --git a/consumer/src/tree.rs b/consumer/src/tree.rs index 98cf713a2..5fea0f76e 100644 --- a/consumer/src/tree.rs +++ b/consumer/src/tree.rs @@ -63,9 +63,9 @@ impl State { nodes: &mut im::HashMap, changes: &mut Option<&mut InternalChanges>, parent_and_index: Option, + id: NodeId, data: NodeData, ) { - let id = data.id; let state = NodeState { parent_and_index, data: Box::new(data), @@ -76,8 +76,7 @@ impl State { } } - for node_data in update.nodes { - let node_id = node_data.id; + for (node_id, node_data) in update.nodes { orphans.remove(&node_id); let mut seen_child_ids = HashSet::new(); @@ -94,6 +93,7 @@ impl State { &mut self.nodes, &mut changes, Some(parent_and_index), + *child_id, child_data, ); } else { @@ -122,10 +122,11 @@ impl State { &mut self.nodes, &mut changes, Some(parent_and_index), + node_id, node_data, ); } else if node_id == root { - add_node(&mut self.nodes, &mut changes, None, node_data); + add_node(&mut self.nodes, &mut changes, None, node_id, node_data); } else { pending_nodes.insert(node_id, node_data); } @@ -181,9 +182,9 @@ impl State { fn serialize(&self) -> TreeUpdate { let mut nodes = Vec::new(); - fn traverse(state: &State, nodes: &mut Vec, id: NodeId) { + fn traverse(state: &State, nodes: &mut Vec<(NodeId, NodeData)>, id: NodeId) { let node = state.nodes.get(&id).unwrap(); - nodes.push((*node.data).clone()); + nodes.push((id, (*node.data).clone())); for child_id in node.data.children.iter() { traverse(state, nodes, *child_id); @@ -210,6 +211,7 @@ impl Reader<'_> { pub fn node_by_id(&self, id: NodeId) -> Option> { self.state.nodes.get(&id).map(|node_state| Node { tree_reader: self, + id, state: node_state, }) } @@ -357,7 +359,13 @@ mod tests { #[test] fn init_tree_with_root_node() { let update = TreeUpdate { - nodes: vec![Node::new(NODE_ID_1, Role::Window)], + nodes: vec![( + NODE_ID_1, + Node { + role: Role::Window, + ..Node::default() + }, + )], tree: Some(Tree::new(NODE_ID_1)), focus: None, }; @@ -371,12 +379,28 @@ mod tests { fn root_node_has_children() { let update = TreeUpdate { nodes: vec![ - Node { - children: vec![NODE_ID_2, NODE_ID_3], - ..Node::new(NODE_ID_1, Role::Window) - }, - Node::new(NODE_ID_2, Role::Button), - Node::new(NODE_ID_3, Role::Button), + ( + NODE_ID_1, + Node { + role: Role::Window, + children: vec![NODE_ID_2, NODE_ID_3], + ..Default::default() + }, + ), + ( + NODE_ID_2, + Node { + role: Role::Button, + ..Default::default() + }, + ), + ( + NODE_ID_3, + Node { + role: Role::Button, + ..Default::default() + }, + ), ], tree: Some(Tree::new(NODE_ID_1)), focus: None, @@ -396,9 +420,12 @@ mod tests { #[test] fn add_child_to_root_node() { - let root_node = Node::new(NODE_ID_1, Role::Window); + let root_node = Node { + role: Role::Window, + ..Default::default() + }; let first_update = TreeUpdate { - nodes: vec![root_node.clone()], + nodes: vec![(NODE_ID_1, root_node.clone())], tree: Some(Tree::new(NODE_ID_1)), focus: None, }; @@ -406,11 +433,20 @@ mod tests { assert_eq!(0, tree.read().root().children().count()); let second_update = TreeUpdate { nodes: vec![ - Node { - children: vec![NODE_ID_2], - ..root_node - }, - Node::new(NODE_ID_2, Role::RootWebArea), + ( + NODE_ID_1, + Node { + children: vec![NODE_ID_2], + ..root_node + }, + ), + ( + NODE_ID_2, + Node { + role: Role::RootWebArea, + ..Default::default() + }, + ), ], tree: None, focus: None, @@ -448,14 +484,26 @@ mod tests { #[test] fn remove_child_from_root_node() { - let root_node = Node::new(NODE_ID_1, Role::Window); + let root_node = Node { + role: Role::Window, + ..Default::default() + }; let first_update = TreeUpdate { nodes: vec![ - Node { - children: vec![NODE_ID_2], - ..root_node.clone() - }, - Node::new(NODE_ID_2, Role::RootWebArea), + ( + NODE_ID_1, + Node { + children: vec![NODE_ID_2], + ..root_node.clone() + }, + ), + ( + NODE_ID_2, + Node { + role: Role::RootWebArea, + ..Default::default() + }, + ), ], tree: Some(Tree::new(NODE_ID_1)), focus: None, @@ -463,7 +511,7 @@ mod tests { let tree = super::Tree::new(first_update, Box::new(NullActionHandler {})); assert_eq!(1, tree.read().root().children().count()); let second_update = TreeUpdate { - nodes: vec![root_node], + nodes: vec![(NODE_ID_1, root_node)], tree: None, focus: None, }; @@ -497,12 +545,28 @@ mod tests { fn move_focus_between_siblings() { let first_update = TreeUpdate { nodes: vec![ - Node { - children: vec![NODE_ID_2, NODE_ID_3], - ..Node::new(NODE_ID_1, Role::Window) - }, - Node::new(NODE_ID_2, Role::Button), - Node::new(NODE_ID_3, Role::Button), + ( + NODE_ID_1, + Node { + role: Role::Window, + children: vec![NODE_ID_2, NODE_ID_3], + ..Default::default() + }, + ), + ( + NODE_ID_2, + Node { + role: Role::Button, + ..Default::default() + }, + ), + ( + NODE_ID_3, + Node { + role: Role::Button, + ..Default::default() + }, + ), ], tree: Some(Tree::new(NODE_ID_1)), focus: Some(NODE_ID_2), @@ -557,17 +621,27 @@ mod tests { #[test] fn update_node() { - let child_node = Node::new(NODE_ID_2, Role::Button); + let child_node = Node { + role: Role::Button, + ..Default::default() + }; let first_update = TreeUpdate { nodes: vec![ - Node { - children: vec![NODE_ID_2], - ..Node::new(NODE_ID_1, Role::Window) - }, - Node { - name: Some("foo".into()), - ..child_node.clone() - }, + ( + NODE_ID_1, + Node { + role: Role::Window, + children: vec![NODE_ID_2], + ..Default::default() + }, + ), + ( + NODE_ID_2, + Node { + name: Some("foo".into()), + ..child_node.clone() + }, + ), ], tree: Some(Tree::new(NODE_ID_1)), focus: None, @@ -578,10 +652,13 @@ mod tests { tree.read().node_by_id(NODE_ID_2).unwrap().name() ); let second_update = TreeUpdate { - nodes: vec![Node { - name: Some("bar".into()), - ..child_node - }], + nodes: vec![( + NODE_ID_2, + Node { + name: Some("bar".into()), + ..child_node + }, + )], tree: None, focus: None, }; @@ -613,12 +690,28 @@ mod tests { fn no_change_update() { let update = TreeUpdate { nodes: vec![ - Node { - children: vec![NODE_ID_2, NODE_ID_3], - ..Node::new(NODE_ID_1, Role::Window) - }, - Node::new(NODE_ID_2, Role::Button), - Node::new(NODE_ID_3, Role::Button), + ( + NODE_ID_1, + Node { + role: Role::Window, + children: vec![NODE_ID_2, NODE_ID_3], + ..Default::default() + }, + ), + ( + NODE_ID_2, + Node { + role: Role::Button, + ..Default::default() + }, + ), + ( + NODE_ID_3, + Node { + role: Role::Button, + ..Default::default() + }, + ), ], tree: Some(Tree::new(NODE_ID_1)), focus: Some(NODE_ID_2), diff --git a/platforms/windows/examples/hello_world.rs b/platforms/windows/examples/hello_world.rs index 68c04fa20..e82aefc43 100644 --- a/platforms/windows/examples/hello_world.rs +++ b/platforms/windows/examples/hello_world.rs @@ -81,24 +81,30 @@ fn make_button(id: NodeId, name: &str) -> Node { }; Node { + role: Role::Button, bounds: Some(rect), name: Some(name.into()), focusable: true, default_action_verb: Some(DefaultActionVerb::Click), - ..Node::new(id, Role::Button) + ..Default::default() } } fn get_initial_state() -> TreeUpdate { let root = Node { + role: Role::Window, children: vec![BUTTON_1_ID, BUTTON_2_ID], name: Some(WINDOW_TITLE.into()), - ..Node::new(WINDOW_ID, Role::Window) + ..Default::default() }; let button_1 = make_button(BUTTON_1_ID, "Button 1"); let button_2 = make_button(BUTTON_2_ID, "Button 2"); TreeUpdate { - nodes: vec![root, button_1, button_2], + nodes: vec![ + (WINDOW_ID, root), + (BUTTON_1_ID, button_1), + (BUTTON_2_ID, button_2), + ], tree: Some(Tree::new(WINDOW_ID)), focus: None, } @@ -136,17 +142,19 @@ impl WindowState { "You pressed button 2" }; let node = Node { + role: Role::StaticText, name: Some(name.into()), live: Some(Live::Polite), - ..Node::new(PRESSED_TEXT_ID, Role::StaticText) + ..Default::default() }; let root = Node { + role: Role::Window, children: vec![BUTTON_1_ID, BUTTON_2_ID, PRESSED_TEXT_ID], name: Some(WINDOW_TITLE.into()), - ..Node::new(WINDOW_ID, Role::Window) + ..Node::default() }; let update = TreeUpdate { - nodes: vec![node, root], + nodes: vec![(PRESSED_TEXT_ID, node), (WINDOW_ID, root)], tree: None, focus: is_window_focused.then(|| focus), }; diff --git a/platforms/windows/src/tests/simple.rs b/platforms/windows/src/tests/simple.rs index ef4744975..b8ba50157 100644 --- a/platforms/windows/src/tests/simple.rs +++ b/platforms/windows/src/tests/simple.rs @@ -16,24 +16,30 @@ const WINDOW_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(1) }); const BUTTON_1_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(2) }); const BUTTON_2_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(3) }); -fn make_button(id: NodeId, name: &str) -> Node { +fn make_button(name: &str) -> Node { Node { + role: Role::Button, name: Some(name.into()), focusable: true, - ..Node::new(id, Role::Button) + ..Default::default() } } fn get_initial_state() -> TreeUpdate { let root = Node { + role: Role::Window, children: vec![BUTTON_1_ID, BUTTON_2_ID], name: Some(WINDOW_TITLE.into()), - ..Node::new(WINDOW_ID, Role::Window) + ..Default::default() }; - let button_1 = make_button(BUTTON_1_ID, "Button 1"); - let button_2 = make_button(BUTTON_2_ID, "Button 2"); + let button_1 = make_button("Button 1"); + let button_2 = make_button("Button 2"); TreeUpdate { - nodes: vec![root, button_1, button_2], + nodes: vec![ + (WINDOW_ID, root), + (BUTTON_1_ID, button_1), + (BUTTON_2_ID, button_2), + ], tree: Some(Tree::new(WINDOW_ID)), focus: None, } diff --git a/platforms/windows/src/tests/subclassed.rs b/platforms/windows/src/tests/subclassed.rs index 8826857e7..67dc741c6 100644 --- a/platforms/windows/src/tests/subclassed.rs +++ b/platforms/windows/src/tests/subclassed.rs @@ -21,24 +21,30 @@ const WINDOW_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(1) }); const BUTTON_1_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(2) }); const BUTTON_2_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(3) }); -fn make_button(id: NodeId, name: &str) -> Node { +fn make_button(name: &str) -> Node { Node { + role: Role::Button, name: Some(name.into()), focusable: true, - ..Node::new(id, Role::Button) + ..Default::default() } } fn get_initial_state() -> TreeUpdate { let root = Node { + role: Role::Window, children: vec![BUTTON_1_ID, BUTTON_2_ID], name: Some(WINDOW_TITLE.into()), - ..Node::new(WINDOW_ID, Role::Window) + ..Default::default() }; - let button_1 = make_button(BUTTON_1_ID, "Button 1"); - let button_2 = make_button(BUTTON_2_ID, "Button 2"); + let button_1 = make_button("Button 1"); + let button_2 = make_button("Button 2"); TreeUpdate { - nodes: vec![root, button_1, button_2], + nodes: vec![ + (WINDOW_ID, root), + (BUTTON_1_ID, button_1), + (BUTTON_2_ID, button_2), + ], tree: Some(Tree::new(WINDOW_ID)), focus: None, } diff --git a/platforms/winit/examples/simple.rs b/platforms/winit/examples/simple.rs index ba93a012a..a8b122893 100644 --- a/platforms/winit/examples/simple.rs +++ b/platforms/winit/examples/simple.rs @@ -43,11 +43,12 @@ fn make_button(id: NodeId, name: &str) -> Node { }; Node { + role: Role::Button, bounds: Some(rect), name: Some(name.into()), focusable: true, default_action_verb: Some(DefaultActionVerb::Click), - ..Node::new(id, Role::Button) + ..Default::default() } } @@ -90,17 +91,19 @@ impl State { "You pressed button 2" }; let node = Node { + role: Role::StaticText, name: Some(name.into()), live: Some(Live::Polite), - ..Node::new(PRESSED_TEXT_ID, Role::StaticText) + ..Default::default() }; let root = Node { + role: Role::Window, children: vec![BUTTON_1_ID, BUTTON_2_ID, PRESSED_TEXT_ID], name: Some(WINDOW_TITLE.into()), - ..Node::new(WINDOW_ID, Role::Window) + ..Default::default() }; let update = TreeUpdate { - nodes: vec![node, root], + nodes: vec![(PRESSED_TEXT_ID, node), (WINDOW_ID, root)], tree: None, focus: self.is_window_focused.then_some(self.focus), }; @@ -110,14 +113,19 @@ impl State { fn initial_tree_update(state: &State) -> TreeUpdate { let root = Node { + role: Role::Window, children: vec![BUTTON_1_ID, BUTTON_2_ID], name: Some(WINDOW_TITLE.into()), - ..Node::new(WINDOW_ID, Role::Window) + ..Default::default() }; let button_1 = make_button(BUTTON_1_ID, "Button 1"); let button_2 = make_button(BUTTON_2_ID, "Button 2"); TreeUpdate { - nodes: vec![root, button_1, button_2], + nodes: vec![ + (WINDOW_ID, root), + (BUTTON_1_ID, button_1), + (BUTTON_2_ID, button_2), + ], tree: Some(Tree::new(WINDOW_ID)), focus: state.is_window_focused.then_some(state.focus), } From 3ea10837bef3c6bbfadced50fb33cd79a870f767 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 5 Oct 2022 01:55:02 -0500 Subject: [PATCH 2/2] Compromise a little memory usage for code size and speed; store node ID in NodeState rather than copying it into consumer Node struct --- consumer/src/node.rs | 3 +-- consumer/src/tree.rs | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/consumer/src/node.rs b/consumer/src/node.rs index ee3890264..26cb66759 100644 --- a/consumer/src/node.rs +++ b/consumer/src/node.rs @@ -26,7 +26,6 @@ use crate::NodeData; #[derive(Copy, Clone)] pub struct Node<'a> { pub tree_reader: &'a TreeReader<'a>, - pub(crate) id: NodeId, pub(crate) state: &'a NodeState, } @@ -279,7 +278,7 @@ impl<'a> Node<'a> { // Convenience getters pub fn id(&self) -> NodeId { - self.id + self.state.id } pub fn role(&self) -> Role { diff --git a/consumer/src/tree.rs b/consumer/src/tree.rs index 5fea0f76e..e6b0f9a99 100644 --- a/consumer/src/tree.rs +++ b/consumer/src/tree.rs @@ -15,6 +15,7 @@ pub(crate) struct ParentAndIndex(pub(crate) NodeId, pub(crate) usize); #[derive(Clone)] pub(crate) struct NodeState { + pub(crate) id: NodeId, pub(crate) parent_and_index: Option, pub(crate) data: Box, } @@ -67,6 +68,7 @@ impl State { data: NodeData, ) { let state = NodeState { + id, parent_and_index, data: Box::new(data), }; @@ -211,7 +213,6 @@ impl Reader<'_> { pub fn node_by_id(&self, id: NodeId) -> Option> { self.state.nodes.get(&id).map(|node_state| Node { tree_reader: self, - id, state: node_state, }) }