wip(space colonization): Refactor to use a compile-time safe data structure that is also using contiguous memory

src/space_colonization/math.rs

@@ -1,9 +1,7 @@
-use std::rc::Rc;
-
 use super::{Attractor, Node, Point};
 
 pub fn calculate_new_node_position(
-    growth_cell: &(Rc<Node>, Vec<&Attractor>),
+    growth_cell: &(Node, Vec<&Attractor>),
     segment_length: u16,
 ) -> Point {
     let node = &growth_cell.0;
@@ -26,8 +24,6 @@ pub fn calculate_new_node_position(
 
 #[cfg(test)]
 mod tests {
-    use std::cell::Cell;
-
     use super::*;
     const SEGMENT_LENGTH: u16 = 5;
 
@@ -42,29 +38,29 @@ mod tests {
     fn new_node_ignores_dead_attractor() {}
 
     struct GrowthCell {
-        node: Rc<Node>,
+        node: Node,
         attractors: Vec<Attractor>,
     }
 
     impl GrowthCell {
         pub fn from_positions(positions: Vec<(i32, i32)>) -> Self {
             assert!(positions.len() >= 2);
-            let node = Rc::new(Node {
+            let node = Node {
                 position: Point::new(positions[0]),
                 children: Vec::new().into(),
-            });
+            };
             let mut attractors = Vec::new();
             for p in positions.iter().skip(1) {
                 attractors.push(Attractor {
                     position: Point::new(*p),
-                    dead: Cell::new(false),
+                    dead: false,
                 });
             }
             Self { node, attractors }
         }
 
-        fn as_refs(&self) -> (Rc<Node>, Vec<&Attractor>) {
+        fn as_refs(&self) -> (Node, Vec<&Attractor>) {
-            (self.node.clone(), self.attractors.iter().collect())
+            (self.node, self.attractors.iter().collect())
         }
     }
 }

src/space_colonization/mod.rs

@@ -1,8 +1,3 @@
-use std::{
-    cell::{Cell, RefCell},
-    rc::Rc,
-};
-
 use wasm_bindgen::prelude::*;
 
 mod point;
@@ -20,14 +15,14 @@ extern "C" {
 #[derive(Debug)]
 pub struct Attractor {
     pub position: Point,
-    pub dead: Cell<bool>,
+    pub dead: bool,
 }
 
 impl Attractor {
     pub fn new(position: (i32, i32)) -> Attractor {
         Attractor {
             position: Point::new(position),
-            dead: Cell::new(false),
+            dead: false,
         }
     }
 }
@@ -35,7 +30,17 @@ impl Attractor {
 #[derive(Debug, PartialEq, Eq)]
 pub struct Node {
     pub position: Point,
-    pub children: RefCell<Vec<Rc<Node>>>,
+    pub children: Vec<Node>,
+}
+
+#[derive(Debug)]
+pub struct NodeRef {
+    path: Vec<u16>
+}
+
+#[derive(Debug)]
+pub struct AttractorRef {
+    path: Vec<u16>
 }
 
 impl std::hash::Hash for Node {
@@ -49,7 +54,7 @@ impl Node {
     where
         F: Copy + Fn(&Node, &Node),
     {
-        let children = self.children.borrow();
+        let children = self.children;
         for child in children.iter() {
             render_fn(self, &child);
         }

src/space_colonization/space_colonization.rs

@@ -1,11 +1,9 @@
 use super::math::calculate_new_node_position;
-use super::{Attractor, Node, Point};
+use super::{Attractor, Node, NodeRef, Point};
 use log::info;
 use rand::thread_rng;
 use rand::Rng;
-use std::cell::{Cell, RefCell};
 use std::collections::HashMap;
-use std::rc::Rc;
 
 pub struct SpaceColonization {
     max_point: Point,
@@ -42,28 +40,21 @@ pub struct SpaceColonization {
     ///
     /// If density is 10, then there will be an average distance of 10 between attractors
     density: i32,
-    // TODO learning opportunity : avoid Rc and RefCell to have memory contiguous, increase
+    new_nodes: Vec<(NodeRef, Node)>,
-    // performance and simplify code. Using a mutable struct instead of pointers to nodeswill
-    // improve performance. If required, use ids to point to nodes, those ids can be simply their
-    // indices in the array. If the node pointed to is deep, the index can be a vec or a tuple.
-    new_nodes: RefCell<Vec<(Rc<Node>, Rc<Node>)>>,
-    /// Tree like representation of all nodes
-    /// [node: [child1: [grand-child], child2: [grand-child2]]]
-    pub nodes_tree: RefCell<Vec<Rc<Node>>>,
     /// Flat list of all nodes in the tree
     /// [node, child1, grand-child, child2, grand-child2]
-    nodes: RefCell<Vec<Rc<Node>>>,
+    nodes: Vec<Node>,
-    pub attractors: Rc<RefCell<Vec<Attractor>>>,
+    pub attractors: Vec<Attractor>,
 }
 
 impl SpaceColonization {
     pub fn new(width: i32, height: i32) -> SpaceColonization {
         let mut nodes_vec = Vec::new();
-        nodes_vec.push(Rc::new(Node {
+        nodes_vec.push(Node {
             position: Point { x: 100, y: 100 },
-            children: RefCell::new(Vec::new()),
+            children: Vec::new(),
-        }));
+        });
-        let attractors = Rc::new(RefCell::new(Vec::new()));
+        let attractors = Vec::new();
 
         let mut sc = SpaceColonization {
             max_point: Point {
@@ -74,10 +65,9 @@ impl SpaceColonization {
             attraction_distance: 100,
             segment_length: 5,
             density: 30,
-            nodes_tree: RefCell::new(nodes_vec.clone()),
+            nodes: nodes_vec,
-            nodes: RefCell::new(nodes_vec),
             attractors,
-            new_nodes: RefCell::new(Vec::new()),
+            new_nodes: Vec::new(),
         };
 
         sc.place_attractors();
@@ -89,7 +79,7 @@ impl SpaceColonization {
     pub fn new_for_tests(
         width: i32,
         height: i32,
-        nodes: Vec<Rc<Node>>,
+        nodes: Vec<Node>,
         attractors: Vec<Attractor>,
     ) -> SpaceColonization {
         SpaceColonization {
@@ -101,10 +91,9 @@ impl SpaceColonization {
             attraction_distance: 12,
             segment_length: 3,
             density: 3,
-            nodes_tree: RefCell::new(nodes.clone()),
+            nodes,
-            nodes: RefCell::new(nodes),
+            attractors,
-            attractors: Rc::new(RefCell::new(attractors)),
+            new_nodes: Vec::new(),
-            new_nodes: RefCell::new(Vec::new()),
         }
     }
 
@@ -112,8 +101,8 @@ impl SpaceColonization {
     where
         F: Copy + Fn(&Node, &Node),
     {
-        info!("Rendering {} nodes", self.nodes.borrow().len());
+        info!("Rendering {} nodes", self.nodes.len());
-        for n in self.nodes_tree.borrow().iter() {
+        for n in self.nodes_tree.iter() {
             n.render(render_id, render_fn);
         }
     }
@@ -123,9 +112,9 @@ impl SpaceColonization {
         let mut y_pos = 0;
         while x_pos < self.max_point.x {
             while y_pos < self.max_point.y {
-                self.attractors.borrow_mut().push(Attractor {
+                self.attractors.push(Attractor {
                     position: self.get_random_point(x_pos.into(), y_pos.into()),
-                    dead: Cell::new(false),
+                    dead: false,
                 });
                 y_pos += self.density;
             }
@@ -159,14 +148,23 @@ impl SpaceColonization {
     }
 
     pub fn grow(&self) {
+        // TODO
+        // Find a clean API that will be stable across refactoring
+        // Write the test against this api including performance
+        // Find a way to make a compile-time safe datastructure that will ensure that
+        //  - I can store my attractors and their state (remove them or update them when dead)
+        //  - I can store my nodes and their state
+        //  - I can efficiently render my nodes on a canvas
+        //  - I use as little memory as possible
+        //  - I can update my nodes
         self.grow_nodes();
-        println!("new nodes for iteration {:?}", self.new_nodes.borrow());
+        println!("new nodes for iteration {:?}", self.new_nodes);
-        let mut nodes_mut = self.nodes.borrow_mut();
+        let mut nodes_mut = self.nodes;
-        for new_pair in self.new_nodes.borrow().iter() {
+        for new_pair in self.new_nodes.iter() {
-            new_pair.0.children.borrow_mut().push(new_pair.1.clone());
+            new_pair.0.children.push(new_pair.1);
-            nodes_mut.push(new_pair.1.clone());
+            nodes_mut.push(new_pair.1);
         }
-        self.new_nodes.borrow_mut().clear();
+        self.new_nodes.clear();
     }
 
     pub fn grow_nodes(&self) {
@@ -176,21 +174,21 @@ impl SpaceColonization {
         // attractors within the attraction range that this node is the closest to
         //
         // calculate new node position
-        let attractors = self.attractors.borrow();
+        let attractors = self.attractors;
-        let mut growing_paths: HashMap<Rc<Node>, Vec<&Attractor>> = HashMap::new();
+        let mut growing_paths: HashMap<<NodeRef>, Vec<AttractorRef>> = HashMap::new();
         for a in attractors.iter() {
-            if a.dead.get() {
+            if a.dead {
                 continue;
             }
-            let mut closest_node: Option<Rc<Node>> = None;
+            let mut closest_node: Option<Node> = None;
             let mut closest_node_distance = f64::MAX;
 
-            for n in self.nodes.borrow().iter() {
+            for n in self.nodes.iter() {
                 let distance = n.position.distance(&a.position);
                 if distance <= self.attraction_distance as f64 {
                     // TODO make sure it is closest node amongs all nodes
                     if distance < closest_node_distance {
-                        closest_node = Some(n.clone());
+                        closest_node = Some(n);
                         closest_node_distance = distance;
                         if distance < self.kill_distance as f64 {
                             a.dead.replace(true);
@@ -216,8 +214,7 @@ impl SpaceColonization {
                 }
             }
             self.new_nodes
-                .borrow_mut()
+                .push((growth_cell.0, Node::new(position)));
-                .push((growth_cell.0.clone(), Rc::new(Node::new(position))));
         }
     }
 }
@@ -229,71 +226,68 @@ mod test {
     #[test]
     fn grow_should_reach_single_attractor_and_die() {
         let mut nodes = Vec::new();
-        nodes.push(Rc::new(Node::new(Point::new((0, 0)))));
+        nodes.push(Node::new(Point::new((0, 0))));
         let mut attractors = Vec::new();
         attractors.push(Attractor::new((10, 0)));
 
         let sc = SpaceColonization::new_for_tests(100, 100, nodes, attractors);
 
-        assert_eq!(sc.attractors.borrow().len(), 1);
+        assert_eq!(sc.attractors.len(), 1);
         assert!(sc
             .attractors
-            .borrow()
             .iter()
-            .find(|a| a.dead.get() == true)
+            .find(|a| a.dead == true)
             .is_none());
-        assert_eq!(sc.nodes_tree.borrow()[0].children.borrow().len(), 0);
+        assert_eq!(sc.nodes_tree[0].children.len(), 0);
 
         sc.grow();
 
-        assert_eq!(sc.new_nodes.borrow().len(), 0);
+        assert_eq!(sc.new_nodes.len(), 0);
-        assert_eq!(sc.nodes_tree.borrow()[0].children.borrow().len(), 1);
+        assert_eq!(sc.nodes_tree.[0].children.len(), 1);
         assert!(sc
             .attractors
-            .borrow()
             .iter()
-            .find(|a| a.dead.get() == true)
+            .find(|a| a.dead == true)
             .is_none());
         assert_eq!(
-            sc.nodes_tree.borrow()[0].children.borrow()[0].position,
+            sc.nodes_tree.[0].children.[0].position,
             Point::new((3, 0))
         );
         assert_eq!(
-            sc.nodes_tree.borrow()[0].children.borrow().len(),
+            sc.nodes_tree[0].children.len(),
             1,
         );
         assert_eq!(
-            sc.nodes_tree.borrow().len(),
+            sc.nodes_tree.len(),
             1,
         );
-        println!("root node direct children iteration 1 {:?}", sc.nodes_tree.borrow()[0].children.borrow());
+        println!("root node direct children iteration 1 {:?}", sc.nodes_tree.[0].children);
 
         sc.grow();
 
         assert_eq!(
-            sc.nodes_tree.borrow().len(),
+            sc.nodes_tree.len(),
             1,
         );
         assert_eq!(sc
             .attractors
-            .borrow()
             .iter()
-            .filter(|a| a.dead.get() == true)
+            .filter(|a| a.dead == true)
             .collect::<Vec<&Attractor>>().len(), 1);
         
-        println!("root node direct children iteration 2 {:?}", sc.nodes_tree.borrow()[0].children.borrow());
+        println!("root node direct children iteration 2 {:?}", sc.nodes_tree.[0].children);
         assert_eq!(
-            sc.nodes_tree.borrow()[0].children.borrow().len(),
+            sc.nodes_tree[0].children.len(),
             1,
         );
         assert_eq!(
-            sc.nodes_tree.borrow()[0].children.borrow()[0].position,
+            sc.nodes_tree[0].children[0].position,
             Point::new((3, 0))
         );
         assert_eq!(
-            sc.nodes_tree.borrow()[0].children.borrow()[0].children.borrow()[0].position,
+            sc.nodes_tree[0].children[0].children[0].position,
             Point::new((6, 0))
         );
-        assert_eq!(sc.nodes.borrow().len(), 3);
+        assert_eq!(sc.nodes.len(), 3);
     }
 }