From 8af59d6969a34519ff0231ddbd086880f1fe49a0 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 25 Nov 2022 22:22:56 -0600 Subject: [PATCH] move height into the tree instead of the realdom --- packages/native-core/src/node.rs | 3 -- packages/native-core/src/passes.rs | 53 ++++++++++++++++++++++++-- packages/native-core/src/real_dom.rs | 21 +++-------- packages/native-core/src/tree.rs | 56 +++++++++++++++++++++++++++- 4 files changed, 110 insertions(+), 23 deletions(-) diff --git a/packages/native-core/src/node.rs b/packages/native-core/src/node.rs index fc06862f9..4530fd350 100644 --- a/packages/native-core/src/node.rs +++ b/packages/native-core/src/node.rs @@ -19,8 +19,6 @@ pub struct NodeData { pub element_id: Option, /// Additional inforation specific to the node type pub node_type: NodeType, - /// The number of parents before the root node. The root node has height 1. - pub height: u16, } /// A type of node with data specific to the node type. The types are a subset of the [VNode] types. @@ -45,7 +43,6 @@ impl Node { node_data: NodeData { element_id: None, node_type, - height: 0, node_id: NodeId(0), }, } diff --git a/packages/native-core/src/passes.rs b/packages/native-core/src/passes.rs index f7b416e5c..da01dfb95 100644 --- a/packages/native-core/src/passes.rs +++ b/packages/native-core/src/passes.rs @@ -96,6 +96,22 @@ impl DirtyNodeStates { } } + fn get(&self, pass_id: PassId, node_id: NodeId) -> bool { + let pass_id = pass_id.0; + let index = pass_id / 64; + let bit = pass_id % 64; + let encoded = 1 << bit; + if let Some(dirty) = self.dirty.get(&node_id) { + if let Some(atomic) = dirty.get(index as usize) { + atomic.load(Ordering::SeqCst) & encoded != 0 + } else { + false + } + } else { + false + } + } + fn all_dirty(&self, pass_id: PassId) -> impl Iterator + '_ { let pass_id = pass_id.0; let index = pass_id / 64; @@ -160,7 +176,23 @@ fn get_pass>( } } } - _ => { + AnyPass::Downward(pass) => { + let mut sorted: Vec<_> = dirty_nodes.all_dirty(pass_id).collect(); + sorted.sort_unstable_by_key(|id| shared_view.height(*id)); + println!( + "Task: {:?} {:?}", + pass_id, + sorted + .iter() + .map(|id| (id, shared_view.height(*id))) + .collect::>() + ); + for node in sorted.into_iter() { + global.push(node); + } + current_pass.replace(AnyPass::Downward(pass)); + } + AnyPass::Node(pass) => { for node in dirty_nodes.all_dirty(pass_id) { global.push(node); } @@ -169,7 +201,7 @@ fn get_pass>( pass_id, dirty_nodes.all_dirty(pass_id).collect::>() ); - current_pass.replace(pass); + current_pass.replace(AnyPass::Node(pass)); } } return; @@ -224,6 +256,7 @@ pub fn resolve_passes( loop { let read = current_pass.read(); if let Some(current_pass) = &*read { + let current_pass_id = current_pass.pass_id(); match current_pass { AnyPass::Upward(_) => { todo!("Upward passes are single threaded") @@ -248,7 +281,9 @@ pub fn resolve_passes( for dependant in pass.dependants() { dirty_nodes.insert(*dependant, *id); } - w.push(*id); + if !dirty_nodes.get(current_pass_id, *id) { + w.push(*id); + } } } } @@ -333,6 +368,7 @@ fn node_pass() { tree.add_child(parent, child2); let grandchild2 = tree.create_node(4); tree.add_child(child2, grandchild2); + println!("{:#?}", tree); struct AddPass; @@ -484,14 +520,19 @@ fn down_pass() { #[test] fn dependant_down_pass() { use crate::tree::{Tree, TreeLike}; + // 0 let mut tree = Tree::new(1); let parent = tree.root(); + // 1 let child1 = tree.create_node(1); tree.add_child(parent, child1); + // 2 let grandchild1 = tree.create_node(1); tree.add_child(child1, grandchild1); + // 3 let child2 = tree.create_node(1); tree.add_child(parent, child2); + // 4 let grandchild2 = tree.create_node(1); tree.add_child(child2, grandchild2); @@ -512,7 +553,10 @@ fn dependant_down_pass() { fn pass(&self, node: &mut i32, parent: Option<&mut i32>) -> bool { if let Some(parent) = parent { + println!("AddPass: {} -> {}", node, *node + *parent); *node += *parent; + } else { + println!("AddPass: {}", node); } true } @@ -535,7 +579,10 @@ fn dependant_down_pass() { fn pass(&self, node: &mut i32, parent: Option<&mut i32>) -> bool { if let Some(parent) = parent { + println!("SubtractPass: {} -> {}", node, *node - *parent); *node -= *parent; + } else { + println!("SubtractPass: {}", node); } true } diff --git a/packages/native-core/src/real_dom.rs b/packages/native-core/src/real_dom.rs index e80510aa4..683d3bb65 100644 --- a/packages/native-core/src/real_dom.rs +++ b/packages/native-core/src/real_dom.rs @@ -93,21 +93,6 @@ impl RealDom { fn add_child(&mut self, node_id: RealNodeId, child_id: RealNodeId) { self.tree.add_child(node_id, child_id); - self.resolve_height(child_id); - } - - fn resolve_height(&mut self, node_id: RealNodeId) { - if let Some((node, Some(parent))) = self.tree.node_parent_mut(node_id) { - let height = parent.node_data.height; - node.node_data.height = height + 1; - unsafe { - let self_mut = self as *mut Self; - // Safety: No node will have itself as a child - for child in self.tree.children_ids(node_id).unwrap() { - (*self_mut).resolve_height(*child); - } - } - } } /// Updates the dom with some mutations and return a set of nodes that were updated. Pass the dirty nodes to update_state. @@ -382,7 +367,11 @@ impl RealDom { pub fn get_listening_sorted(&self, event: &'static str) -> Vec<&Node> { if let Some(nodes) = self.nodes_listening.get(event) { let mut listening: Vec<_> = nodes.iter().map(|id| &self[*id]).collect(); - listening.sort_by(|n1, n2| (n1.node_data.height).cmp(&n2.node_data.height).reverse()); + listening.sort_by(|n1, n2| { + (self.tree.height(n1.node_data.node_id)) + .cmp(&self.tree.height(n2.node_data.node_id)) + .reverse() + }); listening } else { Vec::new() diff --git a/packages/native-core/src/tree.rs b/packages/native-core/src/tree.rs index 4bec046be..53f81ba54 100644 --- a/packages/native-core/src/tree.rs +++ b/packages/native-core/src/tree.rs @@ -15,6 +15,7 @@ pub struct Node { value: T, parent: Option, children: Vec, + height: u16, } #[derive(Debug)] @@ -46,6 +47,18 @@ impl Tree { self.remove_recursive(child); } } + + fn set_height(&mut self, node: NodeId, height: u16) { + let self_mut = self as *mut Self; + let node = self.nodes.get_mut(node.0).unwrap(); + node.height = height; + unsafe { + // Safety: No node has itself as a child + for child in &node.children { + (*self_mut).set_height(*child, height + 1); + } + } + } } pub trait TreeView: Sized { @@ -106,6 +119,8 @@ pub trait TreeView: Sized { fn parent_id(&self, id: NodeId) -> Option; + fn height(&self, id: NodeId) -> Option; + fn map &T2, FMut: Fn(&mut T) -> &mut T2>( &mut self, map: F, @@ -306,6 +321,10 @@ impl TreeView for Tree { self.nodes.get(id.0).and_then(|node| node.parent) } + fn height(&self, id: NodeId) -> Option { + self.nodes.get(id.0).map(|n| n.height) + } + fn get_unchecked(&self, id: NodeId) -> &T { unsafe { &self.nodes.get_unchecked(id.0).value } } @@ -326,6 +345,7 @@ impl TreeLike for Tree { value: root, parent: None, children: Vec::new(), + height: 0, })); Self { nodes, root } } @@ -335,12 +355,16 @@ impl TreeLike for Tree { value, parent: None, children: Vec::new(), + height: 0, })) } fn add_child(&mut self, parent: NodeId, new: NodeId) { - self.nodes.get_mut(parent.0).unwrap().children.push(new); self.nodes.get_mut(new.0).unwrap().parent = Some(parent); + let parent = self.nodes.get_mut(parent.0).unwrap(); + parent.children.push(new); + let height = parent.height + 1; + self.set_height(new, height); } fn remove(&mut self, id: NodeId) -> Option { @@ -372,6 +396,8 @@ impl TreeLike for Tree { *id = new_id; } } + let height = parent.height + 1; + self.set_height(new_id, height); } } @@ -386,6 +412,8 @@ impl TreeLike for Tree { .position(|child| child == &id) .unwrap(); parent.children.insert(index, new); + let height = parent.height + 1; + self.set_height(new, height); } fn insert_after(&mut self, id: NodeId, new: NodeId) { @@ -399,6 +427,8 @@ impl TreeLike for Tree { .position(|child| child == &id) .unwrap(); parent.children.insert(index + 1, new); + let height = parent.height + 1; + self.set_height(new, height); } } @@ -498,6 +528,10 @@ where self.tree.parent_id(id) } + fn height(&self, id: NodeId) -> Option { + self.tree.height(id) + } + fn get_unchecked(&self, id: NodeId) -> &T2 { (self.map)(self.tree.get_unchecked(id)) } @@ -630,6 +664,10 @@ impl<'a, T, Tr: TreeView> TreeView for SharedView<'a, T, Tr> { self.with_node(id, |t| t.parent_id(id)) } + fn height(&self, id: NodeId) -> Option { + unsafe { (*self.tree.get()).height(id) } + } + fn size(&self) -> usize { unsafe { (*self.tree.get()).size() } } @@ -644,6 +682,8 @@ fn creation() { println!("Tree: {:#?}", tree); assert_eq!(tree.size(), 2); + assert_eq!(tree.height(parent), Some(0)); + assert_eq!(tree.height(child), Some(1)); assert_eq!(*tree.get(parent).unwrap(), 1); assert_eq!(*tree.get(child).unwrap(), 0); assert_eq!(tree.parent_id(parent), None); @@ -664,6 +704,10 @@ fn insertion() { println!("Tree: {:#?}", tree); assert_eq!(tree.size(), 4); + assert_eq!(tree.height(parent), Some(0)); + assert_eq!(tree.height(child), Some(1)); + assert_eq!(tree.height(before), Some(1)); + assert_eq!(tree.height(after), Some(1)); assert_eq!(*tree.get(parent).unwrap(), 0); assert_eq!(*tree.get(before).unwrap(), 1); assert_eq!(*tree.get(child).unwrap(), 2); @@ -687,6 +731,10 @@ fn deletion() { println!("Tree: {:#?}", tree); assert_eq!(tree.size(), 4); + assert_eq!(tree.height(parent), Some(0)); + assert_eq!(tree.height(child), Some(1)); + assert_eq!(tree.height(before), Some(1)); + assert_eq!(tree.height(after), Some(1)); assert_eq!(*tree.get(parent).unwrap(), 0); assert_eq!(*tree.get(before).unwrap(), 1); assert_eq!(*tree.get(child).unwrap(), 2); @@ -700,6 +748,9 @@ fn deletion() { println!("Tree: {:#?}", tree); assert_eq!(tree.size(), 3); + assert_eq!(tree.height(parent), Some(0)); + assert_eq!(tree.height(before), Some(1)); + assert_eq!(tree.height(after), Some(1)); assert_eq!(*tree.get(parent).unwrap(), 0); assert_eq!(*tree.get(before).unwrap(), 1); assert_eq!(tree.get(child), None); @@ -712,6 +763,8 @@ fn deletion() { println!("Tree: {:#?}", tree); assert_eq!(tree.size(), 2); + assert_eq!(tree.height(parent), Some(0)); + assert_eq!(tree.height(after), Some(1)); assert_eq!(*tree.get(parent).unwrap(), 0); assert_eq!(tree.get(before), None); assert_eq!(*tree.get(after).unwrap(), 3); @@ -722,6 +775,7 @@ fn deletion() { println!("Tree: {:#?}", tree); assert_eq!(tree.size(), 1); + assert_eq!(tree.height(parent), Some(0)); assert_eq!(*tree.get(parent).unwrap(), 0); assert_eq!(tree.get(after), None); assert_eq!(tree.children_ids(parent).unwrap(), &[]);