From 347981201c077f2883e549db6f8a3f170e74fc1c Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Fri, 2 May 2025 14:51:51 -0700 Subject: [PATCH 01/10] Use `select` for click selection in outline This gets rid of duplicated code. --- app-proto/src/outline.rs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/app-proto/src/outline.rs b/app-proto/src/outline.rs index 2893b6d..97d6ac2 100644 --- a/app-proto/src/outline.rs +++ b/app-proto/src/outline.rs @@ -208,19 +208,9 @@ fn ElementOutlineItem(key: ElementKey, element: Rc) -> View { div( class="element", on:click={ + let state_clone = state.clone(); move |event: MouseEvent| { - if event.shift_key() { - state.selection.update(|sel| { - if !sel.remove(&key) { - sel.insert(key); - } - }); - } else { - state.selection.update(|sel| { - sel.clear(); - sel.insert(key); - }); - } + state_clone.select(key, event.shift_key()); event.stop_propagation(); event.prevent_default(); } -- 2.43.0 From 17f30d1312893c7743e81c2d9a4e1022211da669 Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Sat, 3 May 2025 00:08:17 -0700 Subject: [PATCH 02/10] Compare elements by serial number, not address Rust pointers don't seem to be designed for use as identifiers, so it's probably more idiomatic and possibly more reliable to manage our own unique identifier system. https://stackoverflow.com/a/72149089 This also lets us get rid of the `ElementRc` newtype. --- app-proto/src/assembly.rs | 14 +++----------- app-proto/src/outline.rs | 6 ++---- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/app-proto/src/assembly.rs b/app-proto/src/assembly.rs index 343cef8..7841922 100644 --- a/app-proto/src/assembly.rs +++ b/app-proto/src/assembly.rs @@ -97,17 +97,9 @@ pub trait Element: ProblemPoser + DisplayItem { fn set_column_index(&self, index: usize); } -// the `Element` trait needs to be dyn-compatible, so its method signatures can -// only use `Self` in the type of the receiver. that means `Element` can't -// implement `PartialEq`. if you need partial equivalence for `Element` trait -// objects, use this wrapper -#[derive(Clone)] -pub struct ElementRc(pub Rc); - -impl PartialEq for ElementRc { - fn eq(&self, ElementRc(other): &Self) -> bool { - let ElementRc(rc) = self; - Rc::ptr_eq(rc, &other) +impl PartialEq for dyn Element { + fn eq(&self, other: &Self) -> bool { + self.serial() == other.serial() } } diff --git a/app-proto/src/outline.rs b/app-proto/src/outline.rs index 97d6ac2..7e7048d 100644 --- a/app-proto/src/outline.rs +++ b/app-proto/src/outline.rs @@ -12,7 +12,6 @@ use crate::{ assembly::{ Element, ElementKey, - ElementRc, HalfCurvatureRegulator, InversiveDistanceRegulator, Regulator, @@ -254,7 +253,6 @@ pub fn Outline() -> View { .clone() .into_iter() .sorted_by_key(|(_, elt)| elt.id().clone()) - .map(|(key, elt)| (key, ElementRc(elt))) .collect() ); @@ -268,10 +266,10 @@ pub fn Outline() -> View { ) { Keyed( list=element_list, - view=|(key, ElementRc(elt))| view! { + view=|(key, elt)| view! { ElementOutlineItem(key=key, element=elt) }, - key=|(_, ElementRc(elt))| elt.serial() + key=|(_, elt)| elt.serial() ) } } -- 2.43.0 From ab01c26415eddf22880723d8e725db6397c47ed0 Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Sun, 4 May 2025 00:09:30 -0700 Subject: [PATCH 03/10] Use pointers, not keys, to refer to elements Use reference-counted pointers, rather than collection keys, to refer to elements in tasks like specifying the subjects of regulators, handling selection, and creating interface components. --- app-proto/src/assembly.rs | 117 +++++++++++++++++++------------------- app-proto/src/display.rs | 21 +++---- app-proto/src/main.rs | 18 +++--- app-proto/src/outline.rs | 51 +++++++++-------- 4 files changed, 108 insertions(+), 99 deletions(-) diff --git a/app-proto/src/assembly.rs b/app-proto/src/assembly.rs index 7841922..4e48c65 100644 --- a/app-proto/src/assembly.rs +++ b/app-proto/src/assembly.rs @@ -5,6 +5,9 @@ use std::{ any::{Any, TypeId}, cell::Cell, collections::BTreeSet, + fmt, + fmt::{Debug, Formatter}, + hash::{Hash, Hasher}, rc::Rc, sync::atomic::{AtomicU64, Ordering} }; @@ -54,11 +57,11 @@ pub trait Element: ProblemPoser + DisplayItem { // the regulators that should be created when an element of this type is // inserted into the given assembly with the given storage key /* KLUDGE */ - // right now, this organization makes sense because regulators identify - // their subjects by storage key, so the element has to be inserted before - // its regulators can be created. if we change the way regulators identify - // their subjects, we should consider refactoring - fn default_regulators(_key: ElementKey, _assembly: &Assembly) -> Vec> where Self: Sized { + // this organization made sense when regulators identified their subjects by + // storage key, so the element has to be inserted before its regulators + // could be created. now that regulators identify their subjects by pointer, + // we should consider refactoring + fn default_regulators(self: Rc, _assembly: &Assembly) -> Vec> where Self: Sized { Vec::new() } @@ -97,12 +100,26 @@ pub trait Element: ProblemPoser + DisplayItem { fn set_column_index(&self, index: usize); } +impl Debug for dyn Element { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), fmt::Error> { + self.id().fmt(f) + } +} + +impl Hash for dyn Element { + fn hash(&self, state: &mut H) { + self.serial().hash(state) + } +} + impl PartialEq for dyn Element { fn eq(&self, other: &Self) -> bool { self.serial() == other.serial() } } +impl Eq for dyn Element {} + pub struct Sphere { pub id: String, pub label: String, @@ -148,8 +165,8 @@ impl Element for Sphere { ) } - fn default_regulators(key: ElementKey, assembly: &Assembly) -> Vec> { - vec![Rc::new(HalfCurvatureRegulator::new(key, assembly))] + fn default_regulators(self: Rc, assembly: &Assembly) -> Vec> { + vec![Rc::new(HalfCurvatureRegulator::new(self, assembly))] } fn id(&self) -> &String { @@ -277,7 +294,7 @@ impl ProblemPoser for Point { } pub trait Regulator: ProblemPoser + OutlineItem { - fn subjects(&self) -> Vec; + fn subjects(&self) -> Vec>; fn measurement(&self) -> ReadSignal; fn set_point(&self) -> Signal; @@ -293,23 +310,21 @@ pub trait Regulator: ProblemPoser + OutlineItem { } pub struct InversiveDistanceRegulator { - pub subjects: [ElementKey; 2], + pub subjects: [Rc; 2], pub measurement: ReadSignal, pub set_point: Signal } impl InversiveDistanceRegulator { - pub fn new(subjects: [ElementKey; 2], assembly: &Assembly) -> InversiveDistanceRegulator { - let measurement = assembly.elements.map( - move |elts| { - let representations = subjects.map(|subj| elts[subj].representation()); - representations[0].with(|rep_0| - representations[1].with(|rep_1| - rep_0.dot(&(&*Q * rep_1)) - ) + pub fn new(subjects: [Rc; 2], assembly: &Assembly) -> InversiveDistanceRegulator { + let representations = subjects.each_ref().map(|subj| subj.representation()); + let measurement = create_memo(move || { + representations[0].with(|rep_0| + representations[1].with(|rep_1| + rep_0.dot(&(&*Q * rep_1)) ) - } - ); + ) + }); let set_point = create_signal(SpecifiedValue::from_empty_spec()); @@ -318,8 +333,8 @@ impl InversiveDistanceRegulator { } impl Regulator for InversiveDistanceRegulator { - fn subjects(&self) -> Vec { - self.subjects.into() + fn subjects(&self) -> Vec> { + self.subjects.clone().into() } fn measurement(&self) -> ReadSignal { @@ -335,8 +350,8 @@ impl ProblemPoser for InversiveDistanceRegulator { fn pose(&self, problem: &mut ConstraintProblem, elts: &Slab>) { self.set_point.with_untracked(|set_pt| { if let Some(val) = set_pt.value { - let [row, col] = self.subjects.map( - |subj| elts[subj].column_index().expect( + let [row, col] = self.subjects.each_ref().map( + |subj| subj.column_index().expect( "Subjects should be indexed before inversive distance regulator writes problem data" ) ); @@ -347,17 +362,15 @@ impl ProblemPoser for InversiveDistanceRegulator { } pub struct HalfCurvatureRegulator { - pub subject: ElementKey, + pub subject: Rc, pub measurement: ReadSignal, pub set_point: Signal } impl HalfCurvatureRegulator { - pub fn new(subject: ElementKey, assembly: &Assembly) -> HalfCurvatureRegulator { - let measurement = assembly.elements.map( - move |elts| elts[subject].representation().with( - |rep| rep[Sphere::CURVATURE_COMPONENT] - ) + pub fn new(subject: Rc, assembly: &Assembly) -> HalfCurvatureRegulator { + let measurement = subject.representation().map( + |rep| rep[Sphere::CURVATURE_COMPONENT] ); let set_point = create_signal(SpecifiedValue::from_empty_spec()); @@ -367,8 +380,8 @@ impl HalfCurvatureRegulator { } impl Regulator for HalfCurvatureRegulator { - fn subjects(&self) -> Vec { - vec![self.subject] + fn subjects(&self) -> Vec> { + vec![self.subject.clone()] } fn measurement(&self) -> ReadSignal { @@ -382,10 +395,7 @@ impl Regulator for HalfCurvatureRegulator { fn try_activate(&self, assembly: &Assembly) -> bool { match self.set_point.with(|set_pt| set_pt.value) { Some(half_curv) => { - let representation = assembly.elements.with_untracked( - |elts| elts[self.subject].representation() - ); - representation.update( + self.subject.representation().update( |rep| change_half_curvature(rep, half_curv) ); true @@ -399,7 +409,7 @@ impl ProblemPoser for HalfCurvatureRegulator { fn pose(&self, problem: &mut ConstraintProblem, elts: &Slab>) { self.set_point.with_untracked(|set_pt| { if let Some(val) = set_pt.value { - let col = elts[self.subject].column_index().expect( + let col = self.subject.column_index().expect( "Subject should be indexed before half-curvature regulator writes problem data" ); problem.frozen.push(Sphere::CURVATURE_COMPONENT, col, val); @@ -410,7 +420,7 @@ impl ProblemPoser for HalfCurvatureRegulator { // the velocity is expressed in uniform coordinates pub struct ElementMotion<'a> { - pub key: ElementKey, + pub element: Rc, pub velocity: DVectorView<'a, f64> } @@ -454,14 +464,15 @@ impl Assembly { // insert an element into the assembly without checking whether we already // have an element with the same identifier. any element that does have the // same identifier will get kicked out of the `elements_by_id` index - fn insert_element_unchecked(&self, elt: T) -> ElementKey { + fn insert_element_unchecked(&self, elt: impl Element + 'static) -> ElementKey { // insert the element let id = elt.id().clone(); - let key = self.elements.update(|elts| elts.insert(Rc::new(elt))); + let elt_rc = Rc::new(elt); + let key = self.elements.update(|elts| elts.insert(elt_rc.clone())); /* KLUDGE */ // reorganize to avoid cloning? self.elements_by_id.update(|elts_by_id| elts_by_id.insert(id, key)); // create and insert the element's default regulators - for reg in T::default_regulators(key, &self) { + for reg in elt_rc.default_regulators(&self) { self.insert_regulator(reg); } @@ -503,11 +514,9 @@ impl Assembly { // add the regulator to each subject's regulator list let subjects = regulator.subjects(); - let subject_regulators: Vec<_> = self.elements.with_untracked( - |elts| subjects.into_iter().map( - |subj| elts[subj].regulators() - ).collect() - ); + let subject_regulators: Vec<_> = subjects.into_iter().map( + |subj| subj.regulators() + ).collect(); for regulators in subject_regulators { regulators.update(|regs| regs.insert(key)); } @@ -646,17 +655,17 @@ impl Assembly { // in the process, we find out how many matrix columns we'll need to // hold the deformation let realized_dim = self.tangent.with(|tan| tan.assembly_dim()); - let motion_dim = self.elements.update_silent(|elts| { + let motion_dim = { let mut next_column_index = realized_dim; for elt_motion in motion.iter() { - let moving_elt = &mut elts[elt_motion.key]; + let moving_elt = &elt_motion.element; if moving_elt.column_index().is_none() { moving_elt.set_column_index(next_column_index); next_column_index += 1; } } next_column_index - }); + }; // project the element motions onto the tangent space of the solution // variety and sum them to get a deformation of the whole assembly. the @@ -667,9 +676,7 @@ impl Assembly { for elt_motion in motion { // we can unwrap the column index because we know that every moving // element has one at this point - let column_index = self.elements.with_untracked( - |elts| elts[elt_motion.key].column_index().unwrap() - ); + let column_index = elt_motion.element.column_index().unwrap(); if column_index < realized_dim { // this element had a column index when we started, so by @@ -682,12 +689,8 @@ impl Assembly { // this element didn't have a column index when we started, so // by invariant (2), it's unconstrained let mut target_column = motion_proj.column_mut(column_index); - let unif_to_std = self.elements.with_untracked( - |elts| { - elts[elt_motion.key].representation().with_untracked( - |rep| local_unif_to_std(rep.as_view()) - ) - } + let unif_to_std = elt_motion.element.representation().with_untracked( + |rep| local_unif_to_std(rep.as_view()) ); target_column += unif_to_std * elt_motion.velocity; } diff --git a/app-proto/src/display.rs b/app-proto/src/display.rs index 51b207d..c6a5441 100644 --- a/app-proto/src/display.rs +++ b/app-proto/src/display.rs @@ -1,5 +1,6 @@ use core::array; use nalgebra::{DMatrix, DVector, Rotation3, Vector3}; +use std::rc::Rc; use sycamore::{prelude::*, motion::create_raf}; use web_sys::{ console, @@ -16,7 +17,7 @@ use web_sys::{ use crate::{ AppState, - assembly::{ElementKey, ElementColor, ElementMotion, Point, Sphere} + assembly::{Element, ElementColor, ElementMotion, Point, Sphere} }; // --- scene data --- @@ -548,7 +549,7 @@ pub fn Display() -> View { // manipulate the assembly if state.selection.with(|sel| sel.len() == 1) { let sel = state.selection.with( - |sel| *sel.into_iter().next().unwrap() + |sel| sel.into_iter().next().unwrap().clone() ); let translate_x = translate_pos_x_val - translate_neg_x_val; let translate_y = translate_pos_y_val - translate_neg_y_val; @@ -574,7 +575,7 @@ pub fn Display() -> View { assembly_for_raf.deform( vec![ ElementMotion { - key: sel, + element: sel, velocity: elt_motion.as_view() } ] @@ -615,8 +616,8 @@ pub fn Display() -> View { // set up the scene state.assembly.elements.with_untracked( - |elts| for (key, elt) in elts { - let selected = state.selection.with(|sel| sel.contains(&key)); + |elts| for (_, elt) in elts { + let selected = state.selection.with(|sel| sel.contains(elt)); elt.show(&mut scene, selected); } ); @@ -849,16 +850,16 @@ pub fn Display() -> View { // find the nearest element along the pointer direction let (dir, pixel_size) = event_dir(&event); console::log_1(&JsValue::from(dir.to_string())); - let mut clicked: Option<(ElementKey, f64)> = None; - for (key, elt) in state.assembly.elements.get_clone_untracked() { + let mut clicked: Option<(Rc, f64)> = None; + for (_, elt) in state.assembly.elements.get_clone_untracked() { match assembly_to_world.with(|asm_to_world| elt.cast(dir, asm_to_world, pixel_size)) { Some(depth) => match clicked { Some((_, best_depth)) => { if depth < best_depth { - clicked = Some((key, depth)) + clicked = Some((elt, depth)) } }, - None => clicked = Some((key, depth)) + None => clicked = Some((elt, depth)) } None => () }; @@ -866,7 +867,7 @@ pub fn Display() -> View { // if we clicked something, select it match clicked { - Some((key, _)) => state.select(key, event.shift_key()), + Some((elt, _)) => state.select(&elt, event.shift_key()), None => state.selection.update(|sel| sel.clear()) }; } diff --git a/app-proto/src/main.rs b/app-proto/src/main.rs index e581997..586d6d0 100644 --- a/app-proto/src/main.rs +++ b/app-proto/src/main.rs @@ -9,17 +9,18 @@ mod specified; mod tests; use rustc_hash::FxHashSet; +use std::rc::Rc; use sycamore::prelude::*; use add_remove::AddRemove; -use assembly::{Assembly, ElementKey}; +use assembly::{Assembly, Element}; use display::Display; use outline::Outline; #[derive(Clone)] struct AppState { assembly: Assembly, - selection: Signal> + selection: Signal>> } impl AppState { @@ -30,20 +31,19 @@ impl AppState { } } - // in single-selection mode, select the element with the given key. in - // multiple-selection mode, toggle whether the element with the given key - // is selected - fn select(&self, key: ElementKey, multi: bool) { + // in single-selection mode, select the given element. in multiple-selection + // mode, toggle whether the given element is selected + fn select(&self, element: &Rc, multi: bool) { if multi { self.selection.update(|sel| { - if !sel.remove(&key) { - sel.insert(key); + if !sel.remove(element) { + sel.insert(element.clone()); } }); } else { self.selection.update(|sel| { sel.clear(); - sel.insert(key); + sel.insert(element.clone()); }); } } diff --git a/app-proto/src/outline.rs b/app-proto/src/outline.rs index 7e7048d..5aeb62b 100644 --- a/app-proto/src/outline.rs +++ b/app-proto/src/outline.rs @@ -91,20 +91,17 @@ fn RegulatorInput(regulator: Rc) -> View { } pub trait OutlineItem { - fn outline_item(self: Rc, element_key: ElementKey) -> View; + fn outline_item(self: Rc, element: Rc) -> View; } impl OutlineItem for InversiveDistanceRegulator { - fn outline_item(self: Rc, element_key: ElementKey) -> View { + fn outline_item(self: Rc, element: Rc) -> View { let state = use_context::(); - let other_subject = if self.subjects[0] == element_key { - self.subjects[1] + let other_subject_label = if self.subjects[0] == element { + self.subjects[1].label() } else { - self.subjects[0] - }; - let other_subject_label = state.assembly.elements.with( - |elts| elts[other_subject].label().clone() - ); + self.subjects[0].label() + }.clone(); view! { li(class="regulator") { div(class="regulator-label") { (other_subject_label) } @@ -117,7 +114,7 @@ impl OutlineItem for InversiveDistanceRegulator { } impl OutlineItem for HalfCurvatureRegulator { - fn outline_item(self: Rc, _element_key: ElementKey) -> View { + fn outline_item(self: Rc, _element: Rc) -> View { view! { li(class="regulator") { div(class="regulator-label") // for spacing @@ -131,21 +128,24 @@ impl OutlineItem for HalfCurvatureRegulator { // a list item that shows a regulator in an outline view of an element #[component(inline_props)] -fn RegulatorOutlineItem(regulator_key: RegulatorKey, element_key: ElementKey) -> View { +fn RegulatorOutlineItem(regulator_key: RegulatorKey, element: Rc) -> View { let state = use_context::(); let regulator = state.assembly.regulators.with( |regs| regs[regulator_key].clone() ); - regulator.outline_item(element_key) + regulator.outline_item(element) } // a list item that shows an element in an outline view of an assembly #[component(inline_props)] fn ElementOutlineItem(key: ElementKey, element: Rc) -> View { let state = use_context::(); - let class = state.selection.map( - move |sel| if sel.contains(&key) { "selected" } else { "" } - ); + let class = { + let element_for_class = element.clone(); + state.selection.map( + move |sel| if sel.contains(&element_for_class) { "selected" } else { "" } + ) + }; let label = element.label().clone(); let representation = element.representation().clone(); let rep_components = move || { @@ -177,10 +177,11 @@ fn ElementOutlineItem(key: ElementKey, element: Rc) -> View { summary( class=class.get(), on:keydown={ + let element_for_handler = element.clone(); move |event: KeyboardEvent| { match event.key().as_str() { "Enter" => { - state.select(key, event.shift_key()); + state.select(&element_for_handler, event.shift_key()); event.prevent_default(); }, "ArrowRight" if regulated.get() => { @@ -207,9 +208,10 @@ fn ElementOutlineItem(key: ElementKey, element: Rc) -> View { div( class="element", on:click={ - let state_clone = state.clone(); + let state_for_handler = state.clone(); + let element_for_handler = element.clone(); move |event: MouseEvent| { - state_clone.select(key, event.shift_key()); + state_for_handler.select(&element_for_handler, event.shift_key()); event.stop_propagation(); event.prevent_default(); } @@ -223,11 +225,14 @@ fn ElementOutlineItem(key: ElementKey, element: Rc) -> View { ul(class="regulators") { Keyed( list=regulator_list, - view=move |reg_key| view! { - RegulatorOutlineItem( - regulator_key=reg_key, - element_key=key - ) + view=move |reg_key| { + let element_for_view = element.clone(); + view! { + RegulatorOutlineItem( + regulator_key=reg_key, + element=element_for_view + ) + } }, key=|reg_key| reg_key.clone() ) -- 2.43.0 From 5191534886bef9d115e2d9ac680e3a696e54680e Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Sun, 4 May 2025 00:24:31 -0700 Subject: [PATCH 04/10] Use pointers for indexing elements by ID --- app-proto/src/assembly.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app-proto/src/assembly.rs b/app-proto/src/assembly.rs index 4e48c65..d1d24f4 100644 --- a/app-proto/src/assembly.rs +++ b/app-proto/src/assembly.rs @@ -446,7 +446,7 @@ pub struct Assembly { pub tangent: Signal, // indexing - pub elements_by_id: Signal> + pub elements_by_id: Signal>> } impl Assembly { @@ -469,7 +469,7 @@ impl Assembly { let id = elt.id().clone(); let elt_rc = Rc::new(elt); let key = self.elements.update(|elts| elts.insert(elt_rc.clone())); /* KLUDGE */ // reorganize to avoid cloning? - self.elements_by_id.update(|elts_by_id| elts_by_id.insert(id, key)); + self.elements_by_id.update(|elts_by_id| elts_by_id.insert(id, elt_rc.clone())); /* KLUDGE */ // reorganize to avoid cloning? // create and insert the element's default regulators for reg in elt_rc.default_regulators(&self) { -- 2.43.0 From fbd6177a07ee9986afb206d979a39592a3abf06f Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Sun, 4 May 2025 01:19:00 -0700 Subject: [PATCH 05/10] Drop unused context Using pointers to refer to elements reduces the need to drag context around. --- app-proto/src/add_remove.rs | 2 +- app-proto/src/assembly.rs | 49 ++++++++++++++++--------------------- app-proto/src/outline.rs | 8 +++--- 3 files changed, 25 insertions(+), 34 deletions(-) diff --git a/app-proto/src/add_remove.rs b/app-proto/src/add_remove.rs index ea86186..f3bbc97 100644 --- a/app-proto/src/add_remove.rs +++ b/app-proto/src/add_remove.rs @@ -241,7 +241,7 @@ pub fn AddRemove() -> View { .unwrap() ); state.assembly.insert_regulator( - Rc::new(InversiveDistanceRegulator::new(subjects, &state.assembly)) + Rc::new(InversiveDistanceRegulator::new(subjects)) ); state.selection.update(|sel| sel.clear()); } diff --git a/app-proto/src/assembly.rs b/app-proto/src/assembly.rs index d1d24f4..200bd34 100644 --- a/app-proto/src/assembly.rs +++ b/app-proto/src/assembly.rs @@ -44,24 +44,18 @@ pub type ElementColor = [f32; 3]; static NEXT_ELEMENT_SERIAL: AtomicU64 = AtomicU64::new(0); pub trait ProblemPoser { - fn pose(&self, problem: &mut ConstraintProblem, elts: &Slab>); + fn pose(&self, problem: &mut ConstraintProblem); } pub trait Element: ProblemPoser + DisplayItem { // the default identifier for an element of this type fn default_id() -> String where Self: Sized; - // create the default example of an element of this type + // the default example of an element of this type fn default(id: String, id_num: u64) -> Self where Self: Sized; - // the regulators that should be created when an element of this type is - // inserted into the given assembly with the given storage key - /* KLUDGE */ - // this organization made sense when regulators identified their subjects by - // storage key, so the element has to be inserted before its regulators - // could be created. now that regulators identify their subjects by pointer, - // we should consider refactoring - fn default_regulators(self: Rc, _assembly: &Assembly) -> Vec> where Self: Sized { + // the default regulators that come with this element + fn default_regulators(self: Rc) -> Vec> { Vec::new() } @@ -165,8 +159,8 @@ impl Element for Sphere { ) } - fn default_regulators(self: Rc, assembly: &Assembly) -> Vec> { - vec![Rc::new(HalfCurvatureRegulator::new(self, assembly))] + fn default_regulators(self: Rc) -> Vec> { + vec![Rc::new(HalfCurvatureRegulator::new(self))] } fn id(&self) -> &String { @@ -199,7 +193,7 @@ impl Element for Sphere { } impl ProblemPoser for Sphere { - fn pose(&self, problem: &mut ConstraintProblem, _elts: &Slab>) { + fn pose(&self, problem: &mut ConstraintProblem) { let index = self.column_index().expect( format!("Sphere \"{}\" should be indexed before writing problem data", self.id).as_str() ); @@ -283,7 +277,7 @@ impl Element for Point { } impl ProblemPoser for Point { - fn pose(&self, problem: &mut ConstraintProblem, _elts: &Slab>) { + fn pose(&self, problem: &mut ConstraintProblem) { let index = self.column_index().expect( format!("Point \"{}\" should be indexed before writing problem data", self.id).as_str() ); @@ -304,7 +298,7 @@ pub trait Regulator: ProblemPoser + OutlineItem { // preconditioning when the set point is present, and use its return value // to report whether the set is present. the default implementation does no // preconditioning - fn try_activate(&self, _assembly: &Assembly) -> bool { + fn try_activate(&self) -> bool { self.set_point().with(|set_pt| set_pt.is_present()) } } @@ -316,7 +310,7 @@ pub struct InversiveDistanceRegulator { } impl InversiveDistanceRegulator { - pub fn new(subjects: [Rc; 2], assembly: &Assembly) -> InversiveDistanceRegulator { + pub fn new(subjects: [Rc; 2]) -> InversiveDistanceRegulator { let representations = subjects.each_ref().map(|subj| subj.representation()); let measurement = create_memo(move || { representations[0].with(|rep_0| @@ -347,7 +341,7 @@ impl Regulator for InversiveDistanceRegulator { } impl ProblemPoser for InversiveDistanceRegulator { - fn pose(&self, problem: &mut ConstraintProblem, elts: &Slab>) { + fn pose(&self, problem: &mut ConstraintProblem) { self.set_point.with_untracked(|set_pt| { if let Some(val) = set_pt.value { let [row, col] = self.subjects.each_ref().map( @@ -368,7 +362,7 @@ pub struct HalfCurvatureRegulator { } impl HalfCurvatureRegulator { - pub fn new(subject: Rc, assembly: &Assembly) -> HalfCurvatureRegulator { + pub fn new(subject: Rc) -> HalfCurvatureRegulator { let measurement = subject.representation().map( |rep| rep[Sphere::CURVATURE_COMPONENT] ); @@ -392,7 +386,7 @@ impl Regulator for HalfCurvatureRegulator { self.set_point } - fn try_activate(&self, assembly: &Assembly) -> bool { + fn try_activate(&self) -> bool { match self.set_point.with(|set_pt| set_pt.value) { Some(half_curv) => { self.subject.representation().update( @@ -406,7 +400,7 @@ impl Regulator for HalfCurvatureRegulator { } impl ProblemPoser for HalfCurvatureRegulator { - fn pose(&self, problem: &mut ConstraintProblem, elts: &Slab>) { + fn pose(&self, problem: &mut ConstraintProblem) { self.set_point.with_untracked(|set_pt| { if let Some(val) = set_pt.value { let col = self.subject.column_index().expect( @@ -468,11 +462,11 @@ impl Assembly { // insert the element let id = elt.id().clone(); let elt_rc = Rc::new(elt); - let key = self.elements.update(|elts| elts.insert(elt_rc.clone())); /* KLUDGE */ // reorganize to avoid cloning? - self.elements_by_id.update(|elts_by_id| elts_by_id.insert(id, elt_rc.clone())); /* KLUDGE */ // reorganize to avoid cloning? + let key = self.elements.update(|elts| elts.insert(elt_rc.clone())); + self.elements_by_id.update(|elts_by_id| elts_by_id.insert(id, elt_rc.clone())); // create and insert the element's default regulators - for reg in elt_rc.default_regulators(&self) { + for reg in elt_rc.default_regulators() { self.insert_regulator(reg); } @@ -513,8 +507,7 @@ impl Assembly { ); // add the regulator to each subject's regulator list - let subjects = regulator.subjects(); - let subject_regulators: Vec<_> = subjects.into_iter().map( + let subject_regulators: Vec<_> = regulator.subjects().into_iter().map( |subj| subj.regulators() ).collect(); for regulators in subject_regulators { @@ -531,7 +524,7 @@ impl Assembly { format!("Updated regulator with subjects {:?}", regulator.subjects()) )); - if regulator.try_activate(&self_for_effect) { + if regulator.try_activate() { self_for_effect.realize(); } }); @@ -573,11 +566,11 @@ impl Assembly { let problem = self.elements.with_untracked(|elts| { let mut problem = ConstraintProblem::new(elts.len()); for (_, elt) in elts { - elt.pose(&mut problem, elts); + elt.pose(&mut problem); } self.regulators.with_untracked(|regs| { for (_, reg) in regs { - reg.pose(&mut problem, elts); + reg.pose(&mut problem); } }); problem diff --git a/app-proto/src/outline.rs b/app-proto/src/outline.rs index 5aeb62b..178a1b7 100644 --- a/app-proto/src/outline.rs +++ b/app-proto/src/outline.rs @@ -11,7 +11,6 @@ use crate::{ AppState, assembly::{ Element, - ElementKey, HalfCurvatureRegulator, InversiveDistanceRegulator, Regulator, @@ -96,7 +95,6 @@ pub trait OutlineItem { impl OutlineItem for InversiveDistanceRegulator { fn outline_item(self: Rc, element: Rc) -> View { - let state = use_context::(); let other_subject_label = if self.subjects[0] == element { self.subjects[1].label() } else { @@ -138,7 +136,7 @@ fn RegulatorOutlineItem(regulator_key: RegulatorKey, element: Rc) - // a list item that shows an element in an outline view of an assembly #[component(inline_props)] -fn ElementOutlineItem(key: ElementKey, element: Rc) -> View { +fn ElementOutlineItem(element: Rc) -> View { let state = use_context::(); let class = { let element_for_class = element.clone(); @@ -271,8 +269,8 @@ pub fn Outline() -> View { ) { Keyed( list=element_list, - view=|(key, elt)| view! { - ElementOutlineItem(key=key, element=elt) + view=|(_, elt)| view! { + ElementOutlineItem(element=elt) }, key=|(_, elt)| elt.serial() ) -- 2.43.0 From 8a86038de0ffc3c8f267bce97e6cbbfe7b9e559b Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Sun, 4 May 2025 10:59:28 -0700 Subject: [PATCH 06/10] Use pointers, not keys, to refer to regulators In the process, move the code that used to handle serial numbering for elements into the `Serial` trait, where it can provide serial numbers for regulators too. --- app-proto/Cargo.toml | 1 + app-proto/src/assembly.rs | 144 +++++++++++++++++++++++++------------- app-proto/src/outline.rs | 41 +++-------- 3 files changed, 106 insertions(+), 80 deletions(-) diff --git a/app-proto/Cargo.toml b/app-proto/Cargo.toml index 5ab7299..77952f9 100644 --- a/app-proto/Cargo.toml +++ b/app-proto/Cargo.toml @@ -3,6 +3,7 @@ name = "dyna3" version = "0.1.0" authors = ["Aaron Fenyes", "Glen Whitney"] edition = "2021" +rust-version = "1.86" [features] default = ["console_error_panic_hook"] diff --git a/app-proto/src/assembly.rs b/app-proto/src/assembly.rs index 200bd34..a20993b 100644 --- a/app-proto/src/assembly.rs +++ b/app-proto/src/assembly.rs @@ -4,7 +4,6 @@ use slab::Slab; use std::{ any::{Any, TypeId}, cell::Cell, - collections::BTreeSet, fmt, fmt::{Debug, Formatter}, hash::{Hash, Hasher}, @@ -30,24 +29,55 @@ use crate::{ specified::SpecifiedValue }; -// the types of the keys we use to access an assembly's elements and regulators +// the types of the keys we use to access an assembly's elements pub type ElementKey = usize; -pub type RegulatorKey = usize; pub type ElementColor = [f32; 3]; /* KLUDGE */ // we should reconsider this design when we build a system for switching between // assemblies. at that point, we might want to switch to hierarchical keys, -// where each each element has a key that identifies it within its assembly and +// where each each item has a key that identifies it within its assembly and // each assembly has a key that identifies it within the sesssion -static NEXT_ELEMENT_SERIAL: AtomicU64 = AtomicU64::new(0); +static NEXT_SERIAL: AtomicU64 = AtomicU64::new(0); + +pub trait Serial { + // a serial number that uniquely identifies this element + fn serial(&self) -> u64; + + // take the next serial number, panicking if that was the last one left + fn next_serial() -> u64 where Self: Sized { + // the technique we use to panic on overflow is taken from _Rust Atomics + // and Locks_, by Mara Bos + // + // https://marabos.nl/atomics/atomics.html#example-handle-overflow + // + NEXT_SERIAL.fetch_update( + Ordering::SeqCst, Ordering::SeqCst, + |serial| serial.checked_add(1) + ).expect("Out of serial numbers for elements") + } +} + +impl Hash for dyn Serial { + fn hash(&self, state: &mut H) { + self.serial().hash(state) + } +} + +impl PartialEq for dyn Serial { + fn eq(&self, other: &Self) -> bool { + self.serial() == other.serial() + } +} + +impl Eq for dyn Serial {} pub trait ProblemPoser { fn pose(&self, problem: &mut ConstraintProblem); } -pub trait Element: ProblemPoser + DisplayItem { +pub trait Element: Serial + ProblemPoser + DisplayItem { // the default identifier for an element of this type fn default_id() -> String where Self: Sized; @@ -65,23 +95,7 @@ pub trait Element: ProblemPoser + DisplayItem { // the regulators the element is subject to. the assembly that owns the // element is responsible for keeping this set up to date - fn regulators(&self) -> Signal>; - - // a serial number that uniquely identifies this element - fn serial(&self) -> u64; - - // take the next serial number, panicking if that was the last one left - fn next_serial() -> u64 where Self: Sized { - // the technique we use to panic on overflow is taken from _Rust Atomics - // and Locks_, by Mara Bos - // - // https://marabos.nl/atomics/atomics.html#example-handle-overflow - // - NEXT_ELEMENT_SERIAL.fetch_update( - Ordering::SeqCst, Ordering::SeqCst, - |serial| serial.checked_add(1) - ).expect("Out of serial numbers for elements") - } + fn regulators(&self) -> Signal>>; // the configuration matrix column index that was assigned to the element // last time the assembly was realized, or `None` if the element has never @@ -102,13 +116,13 @@ impl Debug for dyn Element { impl Hash for dyn Element { fn hash(&self, state: &mut H) { - self.serial().hash(state) + ::hash(self, state) } } impl PartialEq for dyn Element { fn eq(&self, other: &Self) -> bool { - self.serial() == other.serial() + ::eq(self, other) } } @@ -119,8 +133,8 @@ pub struct Sphere { pub label: String, pub color: ElementColor, pub representation: Signal>, - pub regulators: Signal>, - pub serial: u64, + pub regulators: Signal>>, + serial: u64, column_index: Cell> } @@ -138,7 +152,7 @@ impl Sphere { label: label, color: color, representation: create_signal(representation), - regulators: create_signal(BTreeSet::default()), + regulators: create_signal(Vec::new()), serial: Self::next_serial(), column_index: None.into() } @@ -175,14 +189,10 @@ impl Element for Sphere { self.representation } - fn regulators(&self) -> Signal> { + fn regulators(&self) -> Signal>> { self.regulators } - fn serial(&self) -> u64 { - self.serial - } - fn column_index(&self) -> Option { self.column_index.get() } @@ -192,6 +202,12 @@ impl Element for Sphere { } } +impl Serial for Sphere { + fn serial(&self) -> u64 { + self.serial + } +} + impl ProblemPoser for Sphere { fn pose(&self, problem: &mut ConstraintProblem) { let index = self.column_index().expect( @@ -207,8 +223,8 @@ pub struct Point { pub label: String, pub color: ElementColor, pub representation: Signal>, - pub regulators: Signal>, - pub serial: u64, + pub regulators: Signal>>, + serial: u64, column_index: Cell> } @@ -226,7 +242,7 @@ impl Point { label, color, representation: create_signal(representation), - regulators: create_signal(BTreeSet::default()), + regulators: create_signal(Vec::new()), serial: Self::next_serial(), column_index: None.into() } @@ -259,14 +275,10 @@ impl Element for Point { self.representation } - fn regulators(&self) -> Signal> { + fn regulators(&self) -> Signal>> { self.regulators } - fn serial(&self) -> u64 { - self.serial - } - fn column_index(&self) -> Option { self.column_index.get() } @@ -276,6 +288,12 @@ impl Element for Point { } } +impl Serial for Point { + fn serial(&self) -> u64 { + self.serial + } +} + impl ProblemPoser for Point { fn pose(&self, problem: &mut ConstraintProblem) { let index = self.column_index().expect( @@ -287,7 +305,7 @@ impl ProblemPoser for Point { } } -pub trait Regulator: ProblemPoser + OutlineItem { +pub trait Regulator: Serial + ProblemPoser + OutlineItem { fn subjects(&self) -> Vec>; fn measurement(&self) -> ReadSignal; fn set_point(&self) -> Signal; @@ -303,10 +321,25 @@ pub trait Regulator: ProblemPoser + OutlineItem { } } +impl Hash for dyn Regulator { + fn hash(&self, state: &mut H) { + ::hash(self, state) + } +} + +impl PartialEq for dyn Regulator { + fn eq(&self, other: &Self) -> bool { + ::eq(self, other) + } +} + +impl Eq for dyn Regulator {} + pub struct InversiveDistanceRegulator { pub subjects: [Rc; 2], pub measurement: ReadSignal, - pub set_point: Signal + pub set_point: Signal, + serial: u64 } impl InversiveDistanceRegulator { @@ -321,8 +354,9 @@ impl InversiveDistanceRegulator { }); let set_point = create_signal(SpecifiedValue::from_empty_spec()); + let serial = Self::next_serial(); - InversiveDistanceRegulator { subjects, measurement, set_point } + InversiveDistanceRegulator { subjects, measurement, set_point, serial } } } @@ -340,6 +374,12 @@ impl Regulator for InversiveDistanceRegulator { } } +impl Serial for InversiveDistanceRegulator { + fn serial(&self) -> u64 { + self.serial + } +} + impl ProblemPoser for InversiveDistanceRegulator { fn pose(&self, problem: &mut ConstraintProblem) { self.set_point.with_untracked(|set_pt| { @@ -358,7 +398,8 @@ impl ProblemPoser for InversiveDistanceRegulator { pub struct HalfCurvatureRegulator { pub subject: Rc, pub measurement: ReadSignal, - pub set_point: Signal + pub set_point: Signal, + serial: u64 } impl HalfCurvatureRegulator { @@ -368,8 +409,9 @@ impl HalfCurvatureRegulator { ); let set_point = create_signal(SpecifiedValue::from_empty_spec()); + let serial = Self::next_serial(); - HalfCurvatureRegulator { subject, measurement, set_point } + HalfCurvatureRegulator { subject, measurement, set_point, serial } } } @@ -399,6 +441,12 @@ impl Regulator for HalfCurvatureRegulator { } } +impl Serial for HalfCurvatureRegulator { + fn serial(&self) -> u64 { + self.serial + } +} + impl ProblemPoser for HalfCurvatureRegulator { fn pose(&self, problem: &mut ConstraintProblem) { self.set_point.with_untracked(|set_pt| { @@ -502,7 +550,7 @@ impl Assembly { pub fn insert_regulator(&self, regulator: Rc) { // add the regulator to the assembly's regulator list - let key = self.regulators.update( + self.regulators.update( |regs| regs.insert(regulator.clone()) ); @@ -511,7 +559,7 @@ impl Assembly { |subj| subj.regulators() ).collect(); for regulators in subject_regulators { - regulators.update(|regs| regs.insert(key)); + regulators.update(|regs| regs.push(regulator.clone())); } // update the realization when the regulator becomes a constraint, or is diff --git a/app-proto/src/outline.rs b/app-proto/src/outline.rs index 178a1b7..0c852b4 100644 --- a/app-proto/src/outline.rs +++ b/app-proto/src/outline.rs @@ -13,8 +13,7 @@ use crate::{ Element, HalfCurvatureRegulator, InversiveDistanceRegulator, - Regulator, - RegulatorKey + Regulator }, specified::SpecifiedValue }; @@ -90,12 +89,12 @@ fn RegulatorInput(regulator: Rc) -> View { } pub trait OutlineItem { - fn outline_item(self: Rc, element: Rc) -> View; + fn outline_item(self: Rc, element: &Rc) -> View; } impl OutlineItem for InversiveDistanceRegulator { - fn outline_item(self: Rc, element: Rc) -> View { - let other_subject_label = if self.subjects[0] == element { + fn outline_item(self: Rc, element: &Rc) -> View { + let other_subject_label = if self.subjects[0] == element.clone() { self.subjects[1].label() } else { self.subjects[0].label() @@ -112,7 +111,7 @@ impl OutlineItem for InversiveDistanceRegulator { } impl OutlineItem for HalfCurvatureRegulator { - fn outline_item(self: Rc, _element: Rc) -> View { + fn outline_item(self: Rc, _element: &Rc) -> View { view! { li(class="regulator") { div(class="regulator-label") // for spacing @@ -124,16 +123,6 @@ impl OutlineItem for HalfCurvatureRegulator { } } -// a list item that shows a regulator in an outline view of an element -#[component(inline_props)] -fn RegulatorOutlineItem(regulator_key: RegulatorKey, element: Rc) -> View { - let state = use_context::(); - let regulator = state.assembly.regulators.with( - |regs| regs[regulator_key].clone() - ); - regulator.outline_item(element) -} - // a list item that shows an element in an outline view of an assembly #[component(inline_props)] fn ElementOutlineItem(element: Rc) -> View { @@ -158,14 +147,10 @@ fn ElementOutlineItem(element: Rc) -> View { }; let regulated = element.regulators().map(|regs| regs.len() > 0); let regulator_list = element.regulators().map( - move |elt_reg_keys| elt_reg_keys + |regs| regs .clone() .into_iter() - .sorted_by_key( - |®_key| state.assembly.regulators.with( - |regs| regs[reg_key].subjects().len() - ) - ) + .sorted_by_key(|reg| reg.subjects().len()) .collect() ); let details_node = create_node_ref(); @@ -223,16 +208,8 @@ fn ElementOutlineItem(element: Rc) -> View { ul(class="regulators") { Keyed( list=regulator_list, - view=move |reg_key| { - let element_for_view = element.clone(); - view! { - RegulatorOutlineItem( - regulator_key=reg_key, - element=element_for_view - ) - } - }, - key=|reg_key| reg_key.clone() + view=move |reg| reg.outline_item(&element), + key=|reg| reg.serial() ) } } -- 2.43.0 From c6b628d424b04c00d52c82690738047cc68b9f87 Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Sun, 4 May 2025 12:24:17 -0700 Subject: [PATCH 07/10] Store elements and regulators without keys Since we're no longer using storage keys to refer to elements and regulators, we don't need to store these items in keyed collections anymore. To keep element and regulator pointers in `BTreeSet` collections, we implement `Ord` for `Serial` trait objects. As a bonus, this lets us turn the element-wise regulator collections back into `BTreeSet`. --- app-proto/Cargo.lock | 12 +---- app-proto/Cargo.toml | 1 - app-proto/src/assembly.rs | 95 ++++++++++++++++++++++++++------------- app-proto/src/display.rs | 6 +-- app-proto/src/outline.rs | 10 +++-- 5 files changed, 74 insertions(+), 50 deletions(-) diff --git a/app-proto/Cargo.lock b/app-proto/Cargo.lock index 9738589..e6cb21a 100644 --- a/app-proto/Cargo.lock +++ b/app-proto/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "ahash" @@ -90,7 +90,6 @@ dependencies = [ "nalgebra", "readonly", "rustc-hash", - "slab", "sycamore", "wasm-bindgen-test", "web-sys", @@ -414,15 +413,6 @@ dependencies = [ "wide", ] -[[package]] -name = "slab" -version = "0.4.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f92a496fb766b417c996b9c5e57daf2f7ad3b0bebe1ccfca4856390e3d3bb67" -dependencies = [ - "autocfg", -] - [[package]] name = "slotmap" version = "1.0.7" diff --git a/app-proto/Cargo.toml b/app-proto/Cargo.toml index 77952f9..662e123 100644 --- a/app-proto/Cargo.toml +++ b/app-proto/Cargo.toml @@ -16,7 +16,6 @@ lazy_static = "1.5.0" nalgebra = "0.33.0" readonly = "0.2.12" rustc-hash = "2.0.0" -slab = "0.4.9" sycamore = "0.9.0-beta.3" # The `console_error_panic_hook` crate provides better debugging of panics by diff --git a/app-proto/src/assembly.rs b/app-proto/src/assembly.rs index a20993b..ab43a8c 100644 --- a/app-proto/src/assembly.rs +++ b/app-proto/src/assembly.rs @@ -1,14 +1,15 @@ use nalgebra::{DMatrix, DVector, DVectorView}; use rustc_hash::FxHashMap; -use slab::Slab; use std::{ any::{Any, TypeId}, cell::Cell, + collections::BTreeSet, + cmp::Ordering, fmt, fmt::{Debug, Formatter}, hash::{Hash, Hasher}, rc::Rc, - sync::atomic::{AtomicU64, Ordering} + sync::{atomic, atomic::AtomicU64} }; use sycamore::prelude::*; use web_sys::{console, wasm_bindgen::JsValue}; /* DEBUG */ @@ -29,9 +30,6 @@ use crate::{ specified::SpecifiedValue }; -// the types of the keys we use to access an assembly's elements -pub type ElementKey = usize; - pub type ElementColor = [f32; 3]; /* KLUDGE */ @@ -53,7 +51,7 @@ pub trait Serial { // https://marabos.nl/atomics/atomics.html#example-handle-overflow // NEXT_SERIAL.fetch_update( - Ordering::SeqCst, Ordering::SeqCst, + atomic::Ordering::SeqCst, atomic::Ordering::SeqCst, |serial| serial.checked_add(1) ).expect("Out of serial numbers for elements") } @@ -73,6 +71,18 @@ impl PartialEq for dyn Serial { impl Eq for dyn Serial {} +impl PartialOrd for dyn Serial { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for dyn Serial { + fn cmp(&self, other: &Self) -> Ordering { + self.serial().cmp(&other.serial()) + } +} + pub trait ProblemPoser { fn pose(&self, problem: &mut ConstraintProblem); } @@ -95,7 +105,7 @@ pub trait Element: Serial + ProblemPoser + DisplayItem { // the regulators the element is subject to. the assembly that owns the // element is responsible for keeping this set up to date - fn regulators(&self) -> Signal>>; + fn regulators(&self) -> Signal>>; // the configuration matrix column index that was assigned to the element // last time the assembly was realized, or `None` if the element has never @@ -128,12 +138,24 @@ impl PartialEq for dyn Element { impl Eq for dyn Element {} +impl PartialOrd for dyn Element { + fn partial_cmp(&self, other: &Self) -> Option { + ::partial_cmp(self, other) + } +} + +impl Ord for dyn Element { + fn cmp(&self, other: &Self) -> Ordering { + ::cmp(self, other) + } +} + pub struct Sphere { pub id: String, pub label: String, pub color: ElementColor, pub representation: Signal>, - pub regulators: Signal>>, + pub regulators: Signal>>, serial: u64, column_index: Cell> } @@ -152,7 +174,7 @@ impl Sphere { label: label, color: color, representation: create_signal(representation), - regulators: create_signal(Vec::new()), + regulators: create_signal(BTreeSet::new()), serial: Self::next_serial(), column_index: None.into() } @@ -189,7 +211,7 @@ impl Element for Sphere { self.representation } - fn regulators(&self) -> Signal>> { + fn regulators(&self) -> Signal>> { self.regulators } @@ -223,7 +245,7 @@ pub struct Point { pub label: String, pub color: ElementColor, pub representation: Signal>, - pub regulators: Signal>>, + pub regulators: Signal>>, serial: u64, column_index: Cell> } @@ -242,7 +264,7 @@ impl Point { label, color, representation: create_signal(representation), - regulators: create_signal(Vec::new()), + regulators: create_signal(BTreeSet::new()), serial: Self::next_serial(), column_index: None.into() } @@ -275,7 +297,7 @@ impl Element for Point { self.representation } - fn regulators(&self) -> Signal>> { + fn regulators(&self) -> Signal>> { self.regulators } @@ -335,6 +357,18 @@ impl PartialEq for dyn Regulator { impl Eq for dyn Regulator {} +impl PartialOrd for dyn Regulator { + fn partial_cmp(&self, other: &Self) -> Option { + ::partial_cmp(self, other) + } +} + +impl Ord for dyn Regulator { + fn cmp(&self, other: &Self) -> Ordering { + ::cmp(self, other) + } +} + pub struct InversiveDistanceRegulator { pub subjects: [Rc; 2], pub measurement: ReadSignal, @@ -472,8 +506,8 @@ type AssemblyMotion<'a> = Vec>; #[derive(Clone)] pub struct Assembly { // elements and regulators - pub elements: Signal>>, - pub regulators: Signal>>, + pub elements: Signal>>, + pub regulators: Signal>>, // solution variety tangent space. the basis vectors are stored in // configuration matrix format, ordered according to the elements' column @@ -494,8 +528,8 @@ pub struct Assembly { impl Assembly { pub fn new() -> Assembly { Assembly { - elements: create_signal(Slab::new()), - regulators: create_signal(Slab::new()), + elements: create_signal(BTreeSet::new()), + regulators: create_signal(BTreeSet::new()), tangent: create_signal(ConfigSubspace::zero(0)), elements_by_id: create_signal(FxHashMap::default()) } @@ -506,30 +540,27 @@ impl Assembly { // insert an element into the assembly without checking whether we already // have an element with the same identifier. any element that does have the // same identifier will get kicked out of the `elements_by_id` index - fn insert_element_unchecked(&self, elt: impl Element + 'static) -> ElementKey { + fn insert_element_unchecked(&self, elt: impl Element + 'static) { // insert the element let id = elt.id().clone(); let elt_rc = Rc::new(elt); - let key = self.elements.update(|elts| elts.insert(elt_rc.clone())); + self.elements.update(|elts| elts.insert(elt_rc.clone())); self.elements_by_id.update(|elts_by_id| elts_by_id.insert(id, elt_rc.clone())); // create and insert the element's default regulators for reg in elt_rc.default_regulators() { self.insert_regulator(reg); } - - key } - pub fn try_insert_element(&self, elt: impl Element + 'static) -> Option { + pub fn try_insert_element(&self, elt: impl Element + 'static) -> bool { let can_insert = self.elements_by_id.with_untracked( |elts_by_id| !elts_by_id.contains_key(elt.id()) ); if can_insert { - Some(self.insert_element_unchecked(elt)) - } else { - None + self.insert_element_unchecked(elt); } + can_insert } pub fn insert_element_default(&self) { @@ -559,7 +590,7 @@ impl Assembly { |subj| subj.regulators() ).collect(); for regulators in subject_regulators { - regulators.update(|regs| regs.push(regulator.clone())); + regulators.update(|regs| regs.insert(regulator.clone())); } // update the realization when the regulator becomes a constraint, or is @@ -581,7 +612,7 @@ impl Assembly { // print an updated list of regulators console::log_1(&JsValue::from("Regulators:")); self.regulators.with_untracked(|regs| { - for (_, reg) in regs.into_iter() { + for reg in regs.into_iter() { console::log_1(&JsValue::from(format!( " {:?}: {}", reg.subjects(), @@ -605,7 +636,7 @@ impl Assembly { pub fn realize(&self) { // index the elements self.elements.update_silent(|elts| { - for (index, (_, elt)) in elts.into_iter().enumerate() { + for (index, elt) in elts.iter().enumerate() { elt.set_column_index(index); } }); @@ -613,11 +644,11 @@ impl Assembly { // set up the constraint problem let problem = self.elements.with_untracked(|elts| { let mut problem = ConstraintProblem::new(elts.len()); - for (_, elt) in elts { + for elt in elts { elt.pose(&mut problem); } self.regulators.with_untracked(|regs| { - for (_, reg) in regs { + for reg in regs { reg.pose(&mut problem); } }); @@ -660,7 +691,7 @@ impl Assembly { if success { // read out the solution - for (_, elt) in self.elements.get_clone_untracked() { + for elt in self.elements.get_clone_untracked() { elt.representation().update( |rep| rep.set_column(0, &config.column(elt.column_index().unwrap())) ); @@ -741,7 +772,7 @@ impl Assembly { // normalizations, so we restore those afterward /* KLUDGE */ // for now, we only restore the normalizations of spheres - for (_, elt) in self.elements.get_clone_untracked() { + for elt in self.elements.get_clone_untracked() { elt.representation().update_silent(|rep| { match elt.column_index() { Some(column_index) => { diff --git a/app-proto/src/display.rs b/app-proto/src/display.rs index c6a5441..a2fe4b6 100644 --- a/app-proto/src/display.rs +++ b/app-proto/src/display.rs @@ -363,7 +363,7 @@ pub fn Display() -> View { let scene_changed = create_signal(true); create_effect(move || { state.assembly.elements.with(|elts| { - for (_, elt) in elts { + for elt in elts { elt.representation().track(); } }); @@ -616,7 +616,7 @@ pub fn Display() -> View { // set up the scene state.assembly.elements.with_untracked( - |elts| for (_, elt) in elts { + |elts| for elt in elts { let selected = state.selection.with(|sel| sel.contains(elt)); elt.show(&mut scene, selected); } @@ -851,7 +851,7 @@ pub fn Display() -> View { let (dir, pixel_size) = event_dir(&event); console::log_1(&JsValue::from(dir.to_string())); let mut clicked: Option<(Rc, f64)> = None; - for (_, elt) in state.assembly.elements.get_clone_untracked() { + for elt in state.assembly.elements.get_clone_untracked() { match assembly_to_world.with(|asm_to_world| elt.cast(dir, asm_to_world, pixel_size)) { Some(depth) => match clicked { Some((_, best_depth)) => { diff --git a/app-proto/src/outline.rs b/app-proto/src/outline.rs index 0c852b4..caf11e8 100644 --- a/app-proto/src/outline.rs +++ b/app-proto/src/outline.rs @@ -228,11 +228,15 @@ pub fn Outline() -> View { let state = use_context::(); // list the elements alphabetically by ID + /* TO DO */ + // this code is designed to generalize easily to other sort keys. if we only + // ever wanted to sort by ID, we could do that more simply using the + // `elements_by_id` index let element_list = state.assembly.elements.map( |elts| elts .clone() .into_iter() - .sorted_by_key(|(_, elt)| elt.id().clone()) + .sorted_by_key(|elt| elt.id().clone()) .collect() ); @@ -246,10 +250,10 @@ pub fn Outline() -> View { ) { Keyed( list=element_list, - view=|(_, elt)| view! { + view=|elt| view! { ElementOutlineItem(element=elt) }, - key=|(_, elt)| elt.serial() + key=|elt| elt.serial() ) } } -- 2.43.0 From 501cd74c960434648710a4090d49e9ee7a22abad Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Sun, 4 May 2025 12:49:35 -0700 Subject: [PATCH 08/10] Use B-tree collections instead of Fx hash ones This removes a dependency. Since the collections in question should usually be pretty small, it might also reduce memory overhead at little cost in speed. Here are some relevant performance discussions: https://users.rust-lang.org/t/hashmap-vs-btreemap/13804 https://www.reddit.com/r/rust/comments/7rgowj/hashmap_vs_btreemap/ --- app-proto/Cargo.lock | 7 ------- app-proto/Cargo.toml | 1 - app-proto/src/assembly.rs | 7 +++---- app-proto/src/main.rs | 7 +++---- 4 files changed, 6 insertions(+), 16 deletions(-) diff --git a/app-proto/Cargo.lock b/app-proto/Cargo.lock index e6cb21a..3bf609c 100644 --- a/app-proto/Cargo.lock +++ b/app-proto/Cargo.lock @@ -89,7 +89,6 @@ dependencies = [ "lazy_static", "nalgebra", "readonly", - "rustc-hash", "sycamore", "wasm-bindgen-test", "web-sys", @@ -364,12 +363,6 @@ dependencies = [ "syn", ] -[[package]] -name = "rustc-hash" -version = "2.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "583034fd73374156e66797ed8e5b0d5690409c9226b22d87cb7f19821c05d152" - [[package]] name = "safe_arch" version = "0.7.2" diff --git a/app-proto/Cargo.toml b/app-proto/Cargo.toml index 662e123..844a0a6 100644 --- a/app-proto/Cargo.toml +++ b/app-proto/Cargo.toml @@ -15,7 +15,6 @@ js-sys = "0.3.70" lazy_static = "1.5.0" nalgebra = "0.33.0" readonly = "0.2.12" -rustc-hash = "2.0.0" sycamore = "0.9.0-beta.3" # The `console_error_panic_hook` crate provides better debugging of panics by diff --git a/app-proto/src/assembly.rs b/app-proto/src/assembly.rs index ab43a8c..7c6ecaf 100644 --- a/app-proto/src/assembly.rs +++ b/app-proto/src/assembly.rs @@ -1,9 +1,8 @@ use nalgebra::{DMatrix, DVector, DVectorView}; -use rustc_hash::FxHashMap; use std::{ any::{Any, TypeId}, cell::Cell, - collections::BTreeSet, + collections::{BTreeMap, BTreeSet}, cmp::Ordering, fmt, fmt::{Debug, Formatter}, @@ -522,7 +521,7 @@ pub struct Assembly { pub tangent: Signal, // indexing - pub elements_by_id: Signal>> + pub elements_by_id: Signal>> } impl Assembly { @@ -531,7 +530,7 @@ impl Assembly { elements: create_signal(BTreeSet::new()), regulators: create_signal(BTreeSet::new()), tangent: create_signal(ConfigSubspace::zero(0)), - elements_by_id: create_signal(FxHashMap::default()) + elements_by_id: create_signal(BTreeMap::default()) } } diff --git a/app-proto/src/main.rs b/app-proto/src/main.rs index 586d6d0..b76859a 100644 --- a/app-proto/src/main.rs +++ b/app-proto/src/main.rs @@ -8,8 +8,7 @@ mod specified; #[cfg(test)] mod tests; -use rustc_hash::FxHashSet; -use std::rc::Rc; +use std::{collections::BTreeSet, rc::Rc}; use sycamore::prelude::*; use add_remove::AddRemove; @@ -20,14 +19,14 @@ use outline::Outline; #[derive(Clone)] struct AppState { assembly: Assembly, - selection: Signal>> + selection: Signal>> } impl AppState { fn new() -> AppState { AppState { assembly: Assembly::new(), - selection: create_signal(FxHashSet::default()) + selection: create_signal(BTreeSet::default()) } } -- 2.43.0 From 3dd6303e99d866f2d6d9a464abfdae5b87cd895e Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Mon, 5 May 2025 21:26:02 -0700 Subject: [PATCH 09/10] Update assembly tests to use pointers --- app-proto/src/assembly.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/app-proto/src/assembly.rs b/app-proto/src/assembly.rs index 7c6ecaf..bd185c8 100644 --- a/app-proto/src/assembly.rs +++ b/app-proto/src/assembly.rs @@ -813,7 +813,7 @@ mod tests { fn unindexed_element_test() { let _ = create_root(|| { let elt = Sphere::default("sphere".to_string(), 0); - elt.pose(&mut ConstraintProblem::new(1), &Slab::new()); + elt.pose(&mut ConstraintProblem::new(1)); }); } @@ -821,18 +821,16 @@ mod tests { #[should_panic(expected = "Subjects should be indexed before inversive distance regulator writes problem data")] fn unindexed_subject_test_inversive_distance() { let _ = create_root(|| { - let mut elts = Slab::>::new(); - let subjects = [0, 1].map(|k| { - elts.insert( - Rc::new(Sphere::default(format!("sphere{k}"), k)) - ) - }); - elts[subjects[0]].set_column_index(0); + let subjects = [0, 1].map( + |k| Rc::new(Sphere::default(format!("sphere{k}"), k)) as Rc + ); + subjects[0].set_column_index(0); InversiveDistanceRegulator { subjects: subjects, measurement: create_memo(|| 0.0), - set_point: create_signal(SpecifiedValue::try_from("0.0".to_string()).unwrap()) - }.pose(&mut ConstraintProblem::new(2), &elts); + set_point: create_signal(SpecifiedValue::try_from("0.0".to_string()).unwrap()), + serial: InversiveDistanceRegulator::next_serial() + }.pose(&mut ConstraintProblem::new(2)); }); } } \ No newline at end of file -- 2.43.0 From f9ff1c0f438b124af4c0d7bad95e29e23405a406 Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Tue, 6 May 2025 10:27:03 -0700 Subject: [PATCH 10/10] Update continuous integration image to Rust 1.86 --- .forgejo/workflows/continuous-integration.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.forgejo/workflows/continuous-integration.yaml b/.forgejo/workflows/continuous-integration.yaml index daf8923..f3b0130 100644 --- a/.forgejo/workflows/continuous-integration.yaml +++ b/.forgejo/workflows/continuous-integration.yaml @@ -11,7 +11,7 @@ jobs: test: runs-on: docker container: - image: cimg/rust:1.85-node + image: cimg/rust:1.86-node defaults: run: # set the default working directory for each `run` step, relative to the -- 2.43.0