From 6a0f480c8ae24c7280938c6572d091c22d0b4c7c Mon Sep 17 00:00:00 2001 From: Jean-Gabriel Gill-Couture Date: Sun, 30 Jul 2023 22:27:02 -0400 Subject: [PATCH] wip(space colonization): Refactor to use a compile-time safe data structure that is also using contiguous memory --- src/space_colonization/math.rs | 18 ++- src/space_colonization/mod.rs | 23 ++-- src/space_colonization/space_colonization.rs | 122 +++++++++---------- 3 files changed, 79 insertions(+), 84 deletions(-) diff --git a/src/space_colonization/math.rs b/src/space_colonization/math.rs index 25b946f..756dfd3 100644 --- a/src/space_colonization/math.rs +++ b/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, 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, attractors: Vec, } 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, Vec<&Attractor>) { - (self.node.clone(), self.attractors.iter().collect()) + fn as_refs(&self) -> (Node, Vec<&Attractor>) { + (self.node, self.attractors.iter().collect()) } } } diff --git a/src/space_colonization/mod.rs b/src/space_colonization/mod.rs index f447d09..80c84a0 100644 --- a/src/space_colonization/mod.rs +++ b/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, + 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>>, + pub children: Vec, +} + +#[derive(Debug)] +pub struct NodeRef { + path: Vec +} + +#[derive(Debug)] +pub struct AttractorRef { + path: Vec } 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); } diff --git a/src/space_colonization/space_colonization.rs b/src/space_colonization/space_colonization.rs index ba50803..3b7e442 100644 --- a/src/space_colonization/space_colonization.rs +++ b/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 - // 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, Rc)>>, - /// Tree like representation of all nodes - /// [node: [child1: [grand-child], child2: [grand-child2]]] - pub nodes_tree: RefCell>>, + new_nodes: Vec<(NodeRef, Node)>, /// Flat list of all nodes in the tree /// [node, child1, grand-child, child2, grand-child2] - nodes: RefCell>>, - pub attractors: Rc>>, + nodes: Vec, + pub attractors: Vec, } 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()), - })); - let attractors = Rc::new(RefCell::new(Vec::new())); + children: 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: RefCell::new(nodes_vec), + nodes: 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>, + nodes: Vec, attractors: Vec, ) -> SpaceColonization { SpaceColonization { @@ -101,10 +91,9 @@ impl SpaceColonization { attraction_distance: 12, segment_length: 3, density: 3, - nodes_tree: RefCell::new(nodes.clone()), - nodes: RefCell::new(nodes), - attractors: Rc::new(RefCell::new(attractors)), - new_nodes: RefCell::new(Vec::new()), + nodes, + attractors, + new_nodes: Vec::new(), } } @@ -112,8 +101,8 @@ impl SpaceColonization { where F: Copy + Fn(&Node, &Node), { - info!("Rendering {} nodes", self.nodes.borrow().len()); - for n in self.nodes_tree.borrow().iter() { + info!("Rendering {} nodes", self.nodes.len()); + 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()); - let mut nodes_mut = self.nodes.borrow_mut(); - for new_pair in self.new_nodes.borrow().iter() { - new_pair.0.children.borrow_mut().push(new_pair.1.clone()); - nodes_mut.push(new_pair.1.clone()); + println!("new nodes for iteration {:?}", self.new_nodes); + let mut nodes_mut = self.nodes; + for new_pair in self.new_nodes.iter() { + new_pair.0.children.push(new_pair.1); + 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 mut growing_paths: HashMap, Vec<&Attractor>> = HashMap::new(); + let attractors = self.attractors; + let mut growing_paths: HashMap<, Vec> = HashMap::new(); for a in attractors.iter() { - if a.dead.get() { + if a.dead { continue; } - let mut closest_node: Option> = None; + let mut closest_node: Option = 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.clone(), Rc::new(Node::new(position)))); + .push((growth_cell.0, 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.nodes_tree.borrow()[0].children.borrow().len(), 1); + assert_eq!(sc.new_nodes.len(), 0); + 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::>().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); } }