From c6b628d424b04c00d52c82690738047cc68b9f87 Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Sun, 4 May 2025 12:24:17 -0700 Subject: [PATCH] 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() ) } }