diff --git a/Cargo.toml b/Cargo.toml index 0110604..8062cf1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ members = [ version = "0.0.0" edition = "2024" license = "AGPL-3.0-or-later" -rust-version = "1.95" +rust-version = "1.96" [workspace.lints.rust] unsafe_code = "forbid" diff --git a/crates/bone-document/src/document/feature_tree.rs b/crates/bone-document/src/document/feature_tree.rs index d46f159..1eb5394 100644 --- a/crates/bone-document/src/document/feature_tree.rs +++ b/crates/bone-document/src/document/feature_tree.rs @@ -18,7 +18,7 @@ pub enum FeatureNode { Extrude(ExtrudeId), } -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] pub enum FeatureEdge { SketchToExtrude { sketch: FeatureId, @@ -114,6 +114,7 @@ impl FeatureTree { let edge = self.sketch_edge(id, feature); self.drop_edges_incident(id); self.edges.extend(edge); + self.edges.sort_unstable(); id } @@ -133,12 +134,13 @@ impl FeatureTree { &mut self, extrudes: impl Iterator, ) { - let rebuilt: Vec = extrudes + let mut rebuilt: Vec = extrudes .filter_map(|(extrude, feature)| { let extrude = self.feature_of_extrude(extrude)?; self.sketch_edge(extrude, feature) }) .collect(); + rebuilt.sort_unstable(); self.edges = rebuilt; } @@ -314,4 +316,35 @@ mod tests { }; assert!(back.edges().is_empty()); } + + fn extrude_key(n: u64) -> ExtrudeId { + use slotmap::KeyData; + ExtrudeId::from(KeyData::from_ffi((1u64 << 32) | n)) + } + + #[test] + fn edges_canonical_regardless_of_push_order() { + let sketch = SketchId::default(); + let high = extrude_key(5); + let low = extrude_key(2); + + let mut pushed = FeatureTree::seeded(); + pushed.push_sketch(sketch); + pushed.push_extrude(high, &sample_blind_extrude(sketch)); + pushed.push_extrude(low, &sample_blind_extrude(sketch)); + + let mut rebuilt = pushed.clone(); + let extrudes = std::collections::BTreeMap::from([ + (low, sample_blind_extrude(sketch)), + (high, sample_blind_extrude(sketch)), + ]); + rebuilt.rebuild_edges(extrudes.iter().map(|(id, feature)| (*id, feature))); + + assert_eq!( + pushed.edges(), + rebuilt.edges(), + "live edge order must match the load rebuild irrespective of push order" + ); + assert_eq!(pushed.edges().len(), 2); + } } diff --git a/crates/bone-document/src/evaluator.rs b/crates/bone-document/src/evaluator.rs index 14f26f9..7e34ae9 100644 --- a/crates/bone-document/src/evaluator.rs +++ b/crates/bone-document/src/evaluator.rs @@ -34,7 +34,6 @@ pub struct EvaluatedExtrude { } impl EvaluatedExtrude { - #[must_use] pub fn result(&self) -> &Result { &self.result } diff --git a/crates/bone-document/src/profile.rs b/crates/bone-document/src/profile.rs index aaefee9..d8b85f6 100644 --- a/crates/bone-document/src/profile.rs +++ b/crates/bone-document/src/profile.rs @@ -501,8 +501,7 @@ mod tests { panic!("arc edge present in the loop"); }; assert!( - forward.sweep_rad() > 0.0 - && (forward.sweep_rad() - core::f64::consts::PI).abs() < 1e-9, + forward.sweep_rad() > 0.0 && (forward.sweep_rad() - core::f64::consts::PI).abs() < 1e-9, "forward ordering sweeps ccw through the bottom semicircle" ); let Ok(solid) = extrude(&sketch) else { diff --git a/crates/bone-document/tests/folder_roundtrip.rs b/crates/bone-document/tests/folder_roundtrip.rs index 27598a1..d9c21b3 100644 --- a/crates/bone-document/tests/folder_roundtrip.rs +++ b/crates/bone-document/tests/folder_roundtrip.rs @@ -581,6 +581,25 @@ fn extrude_roundtrips_through_folder() { assert_eq!(loaded.feature_tree().edges().len(), 1); } +#[test] +fn two_extrudes_keep_canonical_edge_order_through_folder() { + let dir = ok_dir(); + let folder = DocumentFolder::new(dir.path().join("two_extrude.bone")); + let mut doc = Document::new(document_id(1), "two".to_owned()); + doc.insert_sketch(sketch_id(1), "S".to_owned(), rectangle()); + doc.insert_extrude(extrude_id(5), blind_extrude(sketch_id(1))); + doc.insert_extrude(extrude_id(2), blind_extrude(sketch_id(1))); + assert_save(&doc, &folder); + + let loaded = assert_load(&folder); + assert_eq!( + loaded.feature_tree(), + doc.feature_tree(), + "save then load must preserve feature-tree equality with multiple edges" + ); + assert_eq!(loaded.feature_tree().edges().len(), 2); +} + #[test] fn load_refuses_tree_extrude_without_entry() { let dir = ok_dir(); diff --git a/crates/bone-document/tests/undo.rs b/crates/bone-document/tests/undo.rs index 27425be..f392b3b 100644 --- a/crates/bone-document/tests/undo.rs +++ b/crates/bone-document/tests/undo.rs @@ -1,7 +1,13 @@ use std::num::NonZeroUsize; -use bone_document::{Document, Sketch, UndoStack}; -use bone_types::{DocumentId, Point3, SketchId, SketchPlaneBasis, Tolerance, UnitVec3}; +use bone_document::{Document, EditOutcome, Sketch, SketchEdit, SketchEntity, UndoStack}; +use bone_kernel::{ + ExtrudeDirection, ExtrudeEndCondition, ExtrudeFeature, ExtrudeSense, MergeResult, +}; +use bone_types::{ + DocumentId, ExtrudeId, Length, Point2, Point3, PositiveLength, SketchEntityId, SketchId, + SketchPlaneBasis, Tolerance, UnitVec3, millimeter, +}; use slotmap::KeyData; fn plane() -> SketchPlaneBasis { @@ -40,6 +46,54 @@ fn with_sketch(mut doc: Document, sid: SketchId) -> Document { doc } +fn extrude_id(idx: u32) -> ExtrudeId { + ExtrudeId::from(KeyData::from_ffi((1u64 << 32) | u64::from(idx))) +} + +fn entity_of(outcome: EditOutcome) -> SketchEntityId { + let EditOutcome::Entity(id) = outcome else { + panic!("entity outcome"); + }; + id +} + +fn rectangle() -> Sketch { + let Ok((with_points, outcomes)) = Sketch::new(plane()).apply_all(vec![ + SketchEdit::AddEntity(SketchEntity::point(Point2::from_mm(0.0, 0.0))), + SketchEdit::AddEntity(SketchEntity::point(Point2::from_mm(10.0, 0.0))), + SketchEdit::AddEntity(SketchEntity::point(Point2::from_mm(10.0, 5.0))), + SketchEdit::AddEntity(SketchEntity::point(Point2::from_mm(0.0, 5.0))), + ]) else { + panic!("rectangle corners"); + }; + let [c0, c1, c2, c3] = [0, 1, 2, 3].map(|i| entity_of(outcomes[i])); + let Ok((closed, _)) = with_points.apply_all(vec![ + SketchEdit::AddEntity(SketchEntity::line(c0, c1, false)), + SketchEdit::AddEntity(SketchEntity::line(c1, c2, false)), + SketchEdit::AddEntity(SketchEntity::line(c2, c3, false)), + SketchEdit::AddEntity(SketchEntity::line(c3, c0, false)), + ]) else { + panic!("rectangle edges"); + }; + closed +} + +fn blind_extrude(sketch: SketchId) -> ExtrudeFeature { + let Ok(depth) = PositiveLength::new(Length::new::(10.0)) else { + panic!("10 mm is positive"); + }; + ExtrudeFeature { + sketch, + direction: ExtrudeDirection::Normal { + sense: ExtrudeSense::Forward, + }, + end_condition: ExtrudeEndCondition::Blind { depth }, + draft: None, + thin_wall: None, + merge_result: MergeResult::Merge, + } +} + #[test] fn fresh_stack_has_no_history() { let stack = UndoStack::with_capacity(cap(5)); @@ -148,3 +202,46 @@ fn undo_redo_cycle_preserves_determinism() { assert!(stack.redo(&mut live)); assert_eq!(live, c); } + +#[test] +fn undo_extrude_restores_sketch_only_document() { + let sid = sketch_id(1); + let xid = extrude_id(1); + + let mut live = base_doc(); + live.insert_sketch(sid, "Sketch1".to_owned(), rectangle()); + let pre_extrude = live.clone(); + + let mut stack = UndoStack::with_capacity(cap(5)); + stack.record(pre_extrude.clone()); + + live.insert_extrude(xid, blind_extrude(sid)); + assert_ne!( + live, pre_extrude, + "inserting an extrude must change the document" + ); + + assert!(stack.undo(&mut live)); + assert_eq!( + live, pre_extrude, + "undo restores the sketch-only document exactly" + ); + assert!( + live.feature_tree().feature_of_extrude(xid).is_none(), + "no extrude node survives the undo" + ); + assert!( + live.header().extrudes.is_empty(), + "no extrude payload survives the undo" + ); + + assert!(stack.redo(&mut live)); + let Some(feature) = live.feature_tree().feature_of_extrude(xid) else { + panic!("redo restores the extrude node"); + }; + assert_eq!( + live.extrude_of_feature(feature), + Some(&blind_extrude(sid)), + "redo restores the extrude payload" + ); +} diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 6360c18..4f0430e 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "1.95.0" +channel = "1.96.0" components = ["rustfmt", "clippy"]