From ecf4202bf995bb43dcfe80d5ed294bd63431a530 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 19:06:36 +0200 Subject: [PATCH 1/8] Generalize `ObjectIters` implementation Implement `ObjectIters` for all iterators over `ObjectIters`. --- crates/fj-kernel/src/iter.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index f5c9f1cfa..5c3b36ed2 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -328,9 +328,10 @@ impl ObjectIters for Shape { // This implementation exists to paper over the lack of any "top-level" objects // that are an entry point into a shape (basically, the lack of `Sketch` and // `Solid`). -impl ObjectIters for T +impl ObjectIters for T where - for<'r> &'r T: IntoIterator, + for<'r> &'r T: IntoIterator, + O: ObjectIters, { fn curve_iter(&self) -> Iter> { let mut iter = Iter::empty(); From 9829f70c6137818f628644d94eb23e6520bcc049 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 19:07:13 +0200 Subject: [PATCH 2/8] Update comment --- crates/fj-kernel/src/iter.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 5c3b36ed2..5a7dbaa38 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -328,6 +328,8 @@ impl ObjectIters for Shape { // This implementation exists to paper over the lack of any "top-level" objects // that are an entry point into a shape (basically, the lack of `Sketch` and // `Solid`). +// +// It is also very useful in test code. impl ObjectIters for T where for<'r> &'r T: IntoIterator, From 0f4a11476a84058ea8b03e912711c4bee5a29e08 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Jun 2022 15:56:10 +0200 Subject: [PATCH 3/8] Refactor test to work without `Shape` --- crates/fj-kernel/src/validation/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index eec968753..484adccc6 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -142,7 +142,7 @@ mod tests { use crate::{ objects::{Curve, Edge, Vertex, VerticesOfEdge}, - shape::{LocalForm, Shape}, + shape::LocalForm, validation::{validate, ValidationConfig, ValidationError}, }; @@ -188,7 +188,7 @@ mod tests { #[test] fn uniqueness_vertex() -> anyhow::Result<()> { - let mut shape = Shape::new(); + let mut shape = Vec::new(); let deviation = Scalar::from_f64(0.25); @@ -203,11 +203,11 @@ mod tests { }; // Adding a vertex should work. - shape.insert(Vertex { point: a }); + shape.push(Vertex { point: a }); validate(shape.clone(), &config)?; // Adding a second vertex that is considered identical should fail. - shape.insert(Vertex { point: b }); + shape.push(Vertex { point: b }); let result = validate(shape, &config); assert!(matches!(result, Err(ValidationError::Uniqueness(_)))); From 43c6a56764d7cec8187a6304c8c4231464c98866 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Jun 2022 15:57:01 +0200 Subject: [PATCH 4/8] Remove implementation of `ObjectIters` for `Shape` It was added to ease the transition away from `Shape`. Now that transition is coming to an end. --- crates/fj-kernel/src/iter.rs | 39 +----------------------------------- 1 file changed, 1 insertion(+), 38 deletions(-) diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 5a7dbaa38..c5e72990c 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -2,10 +2,7 @@ use std::collections::VecDeque; -use crate::{ - objects::{Curve, Cycle, Edge, Face, Surface, Vertex}, - shape::Shape, -}; +use crate::objects::{Curve, Cycle, Edge, Face, Surface, Vertex}; /// Access iterators over all objects of a shape, or part of it /// @@ -297,34 +294,6 @@ impl ObjectIters for Vertex { } } -// This implementation exists to ease the transition away from `Shape`. It will -// likely be removed at some point, together with `Shape`. -impl ObjectIters for Shape { - fn curve_iter(&self) -> Iter> { - Iter::from_iter(self.curves().map(|handle| handle.get())) - } - - fn cycle_iter(&self) -> Iter> { - Iter::from_iter(self.cycles().map(|handle| handle.get())) - } - - fn edge_iter(&self) -> Iter> { - Iter::from_iter(self.edges().map(|handle| handle.get())) - } - - fn face_iter(&self) -> Iter { - Iter::from_iter(self.faces().map(|handle| handle.get())) - } - - fn surface_iter(&self) -> Iter { - Iter::from_iter(self.surfaces().map(|handle| handle.get())) - } - - fn vertex_iter(&self) -> Iter { - Iter::from_iter(self.vertices().map(|handle| handle.get())) - } -} - // This implementation exists to paper over the lack of any "top-level" objects // that are an entry point into a shape (basically, the lack of `Sketch` and // `Solid`). @@ -412,12 +381,6 @@ impl Iter { Self(objects) } - fn from_iter(iter: impl IntoIterator) -> Self { - let mut objects = VecDeque::new(); - objects.extend(iter); - Self(objects) - } - fn with(mut self, other: Self) -> Self where T: PartialEq, From 46461024fe5d42951d8d484f967e5f8bdc1fc0da Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Jun 2022 16:10:13 +0200 Subject: [PATCH 5/8] Simplify `LocalForm::canonical` --- crates/fj-kernel/src/algorithms/reverse.rs | 8 ++++---- crates/fj-kernel/src/algorithms/sweep.rs | 7 ++++--- crates/fj-kernel/src/algorithms/transform.rs | 17 +++++++++-------- crates/fj-kernel/src/iter.rs | 3 ++- crates/fj-kernel/src/objects/cycle.rs | 7 ++++--- crates/fj-kernel/src/objects/edge.rs | 8 ++++---- crates/fj-kernel/src/objects/face.rs | 2 +- crates/fj-kernel/src/shape/local.rs | 4 ++-- crates/fj-kernel/src/shape/object.rs | 6 +++--- crates/fj-operations/src/circle.rs | 2 +- crates/fj-operations/src/difference_2d.rs | 2 +- 11 files changed, 35 insertions(+), 31 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse.rs index 6fc1e1da7..bc108f96e 100644 --- a/crates/fj-kernel/src/algorithms/reverse.rs +++ b/crates/fj-kernel/src/algorithms/reverse.rs @@ -43,17 +43,17 @@ fn reverse_local_coordinates_in_cycle(cycles: &CyclesInFace) -> CyclesInFace { // a work in progress, this doesn't lead to any observable // bugs. *edge.local().curve.local(), - edge.local().curve.canonical(), + *edge.local().curve.canonical(), ); let vertices = edge.local().vertices.clone().map(|vertex| { - LocalForm::new(*vertex.local(), vertex.canonical()) + LocalForm::new(*vertex.local(), *vertex.canonical()) }); let local = Edge { curve, vertices }; - LocalForm::new(local, edge.canonical()) + LocalForm::new(local, edge.canonical().clone()) }) .collect(); let local = Cycle { edges }; - LocalForm::new(local, cycle.canonical()) + LocalForm::new(local, cycle.canonical().clone()) }); CyclesInFace::new(cycles) diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index 6dff942b3..f14d2f29e 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -149,7 +149,7 @@ fn create_non_continuous_side_face( }; let global = Edge { - curve: LocalForm::canonical_only(curve.canonical()), + curve: LocalForm::canonical_only(*curve.canonical()), vertices, }; @@ -162,8 +162,9 @@ fn create_non_continuous_side_face( let cycle = { let local = Cycle { edges }; - let global = - Cycle::new(local.edges.iter().map(|edge| edge.canonical())); + let global = Cycle::new( + local.edges.iter().map(|edge| edge.canonical().clone()), + ); LocalForm::new(local, global) }; diff --git a/crates/fj-kernel/src/algorithms/transform.rs b/crates/fj-kernel/src/algorithms/transform.rs index 066a48e53..cb2916d02 100644 --- a/crates/fj-kernel/src/algorithms/transform.rs +++ b/crates/fj-kernel/src/algorithms/transform.rs @@ -56,15 +56,16 @@ pub fn transform_cycles( let curve_canonical = edge.canonical().curve().transform(transform); - let vertices = edge.canonical().vertices.map(|vertex| { - let point = vertex.canonical().point; - let point = transform.transform_point(&point); + let vertices = + edge.canonical().clone().vertices.map(|vertex| { + let point = vertex.canonical().point; + let point = transform.transform_point(&point); - let local = *vertex.local(); - let canonical = Vertex { point }; + let local = *vertex.local(); + let canonical = Vertex { point }; - LocalForm::new(local, canonical) - }); + LocalForm::new(local, canonical) + }); let edge_local = Edge { curve: LocalForm::new(curve_local, curve_canonical), @@ -89,7 +90,7 @@ pub fn transform_cycles( let curve = edge.curve().transform(transform); LocalForm::canonical_only(curve) }; - let vertices = edge.vertices.map(|vertex| { + let vertices = edge.vertices.clone().map(|vertex| { let point = vertex.canonical().point; let point = transform.transform_point(&point); diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index c5e72990c..8f76769a2 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -427,7 +427,8 @@ mod tests { &Surface::xy_plane(), [[0., 0.], [1., 0.], [0., 1.]], ) - .canonical(); + .canonical() + .clone(); assert_eq!(3, cycle.curve_iter().count()); assert_eq!(1, cycle.cycle_iter().count()); diff --git a/crates/fj-kernel/src/objects/cycle.rs b/crates/fj-kernel/src/objects/cycle.rs index 98b2d82db..350535c60 100644 --- a/crates/fj-kernel/src/objects/cycle.rs +++ b/crates/fj-kernel/src/objects/cycle.rs @@ -56,7 +56,7 @@ impl Cycle<3> { let edge_local = Edge { curve: LocalForm::new( Curve::Line(Line::from_points(points)), - edge_canonical.curve.canonical(), + *edge_canonical.curve.canonical(), ), vertices: edge_canonical.vertices.clone(), }; @@ -68,7 +68,8 @@ impl Cycle<3> { edges: edges.clone(), }; - let edges_canonical = edges.into_iter().map(|edge| edge.canonical()); + let edges_canonical = + edges.into_iter().map(|edge| edge.canonical().clone()); let canonical = Cycle::new(edges_canonical); LocalForm::new(local, canonical) @@ -79,6 +80,6 @@ impl Cycle<3> { /// This is a convenience method that saves the caller from dealing with the /// [`Handle`]s. pub fn edges(&self) -> impl Iterator> + '_ { - self.edges.iter().map(|handle| handle.canonical()) + self.edges.iter().map(|handle| handle.canonical().clone()) } } diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index ea2998998..06fad9666 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -35,7 +35,7 @@ impl Edge { /// This is a convenience method that saves the caller from dealing with the /// [`Handle`]. pub fn curve(&self) -> Curve<3> { - self.curve.canonical() + *self.curve.canonical() } /// Access the vertices that the edge refers to @@ -46,7 +46,7 @@ impl Edge { self.vertices .0 .as_ref() - .map(|[a, b]| [a.canonical(), b.canonical()]) + .map(|[a, b]| [*a.canonical(), *b.canonical()]) } } @@ -185,8 +185,8 @@ impl VerticesOfEdge { pub fn reverse(self) -> Self { Self(self.0.map(|[a, b]| { [ - LocalForm::new(-(*b.local()), b.canonical()), - LocalForm::new(-(*a.local()), a.canonical()), + LocalForm::new(-(*b.local()), *b.canonical()), + LocalForm::new(-(*a.local()), *a.canonical()), ] })) } diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index e5bcee02a..19275c3f5 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -208,7 +208,7 @@ impl CyclesInFace { /// Access an iterator over the canonical forms of the cycles pub fn as_canonical(&self) -> impl Iterator> + '_ { - self.0.iter().map(|cycle| cycle.canonical()) + self.0.iter().map(|cycle| cycle.canonical().clone()) } /// Access an iterator over local forms of the cycles diff --git a/crates/fj-kernel/src/shape/local.rs b/crates/fj-kernel/src/shape/local.rs index 2e6ec3159..887cd5f1e 100644 --- a/crates/fj-kernel/src/shape/local.rs +++ b/crates/fj-kernel/src/shape/local.rs @@ -26,8 +26,8 @@ impl LocalForm { } /// Access the canonical form of the referenced object - pub fn canonical(&self) -> Canonical { - self.canonical.clone() + pub fn canonical(&self) -> &Canonical { + &self.canonical } } diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs index 54f7d0369..e6318e8cc 100644 --- a/crates/fj-kernel/src/shape/object.rs +++ b/crates/fj-kernel/src/shape/object.rs @@ -59,7 +59,7 @@ impl Object for Cycle<3> { fn merge_into(self, shape: &mut Shape) -> Handle { let mut edges = Vec::new(); for edge in self.edges { - let edge = edge.canonical(); + let edge = edge.canonical().clone(); let edge = edge.merge_into(shape); edges.push(edge.get()); } @@ -76,7 +76,7 @@ impl Object for Face { let mut exts = Vec::new(); for cycle in face.exteriors.as_local_form() { - let merged = cycle.canonical().merge_into(shape); + let merged = cycle.canonical().clone().merge_into(shape); exts.push(LocalForm::new( cycle.local().clone(), merged.get(), @@ -85,7 +85,7 @@ impl Object for Face { let mut ints = Vec::new(); for cycle in face.interiors.as_local_form() { - let merged = cycle.canonical().merge_into(shape); + let merged = cycle.canonical().clone().merge_into(shape); ints.push(LocalForm::new( cycle.local().clone(), merged.get(), diff --git a/crates/fj-operations/src/circle.rs b/crates/fj-operations/src/circle.rs index 83ddd28dd..4e7049e08 100644 --- a/crates/fj-operations/src/circle.rs +++ b/crates/fj-operations/src/circle.rs @@ -26,7 +26,7 @@ impl ToShape for fj::Circle { let cycle_local = Cycle { edges: vec![edge.clone()], }; - let cycle_canonical = Cycle::new(vec![edge.canonical()]); + let cycle_canonical = Cycle::new(vec![edge.canonical().clone()]); let surface = Surface::xy_plane(); let face = tmp diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index ef6784042..eb0e69cb0 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -137,7 +137,7 @@ fn add_cycle( edges: edges.clone(), }; let cycle_canonical = - Cycle::new(edges.into_iter().map(|edge| edge.canonical())); + Cycle::new(edges.into_iter().map(|edge| edge.canonical().clone())); LocalForm::new(cycle_local, cycle_canonical) } From 7ab165ab3d55526b15cc287674fb819139f97947 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Jun 2022 16:11:24 +0200 Subject: [PATCH 6/8] Remove unnecessary `Object` bound --- crates/fj-kernel/src/shape/local.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/shape/local.rs b/crates/fj-kernel/src/shape/local.rs index 887cd5f1e..753893a89 100644 --- a/crates/fj-kernel/src/shape/local.rs +++ b/crates/fj-kernel/src/shape/local.rs @@ -1,17 +1,15 @@ -use super::Object; - /// A reference to an object, which includes a local form /// /// This type is used by topological objects to reference other objects, while /// also keeping track of a local representation of that object, which is often /// more appropriate for various tasks. #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct LocalForm { +pub struct LocalForm { local: Local, canonical: Canonical, } -impl LocalForm { +impl LocalForm { /// Construct a new instance of `LocalForm` /// /// It is the caller's responsibility to make sure that the local and @@ -31,13 +29,16 @@ impl LocalForm { } } -impl LocalForm { +impl LocalForm { /// Construct a new instance of `LocalForm` without a local form /// /// It's possible that an object's local and canonical forms are the same. /// This is a convenience constructor that constructs a `LocalForm` instance /// for such a situation. - pub fn canonical_only(canonical: Canonical) -> Self { + pub fn canonical_only(canonical: Canonical) -> Self + where + Canonical: Clone, + { Self::new(canonical.clone(), canonical) } } From a4021720650fdb4c954d6a2976f2dc50f3e12765 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Jun 2022 16:12:57 +0200 Subject: [PATCH 7/8] Remove unnecessary use of `Shape` --- crates/fj-operations/src/circle.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/crates/fj-operations/src/circle.rs b/crates/fj-operations/src/circle.rs index 4e7049e08..ff88eec1a 100644 --- a/crates/fj-operations/src/circle.rs +++ b/crates/fj-operations/src/circle.rs @@ -2,7 +2,7 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::Tolerance, objects::{Cycle, Edge, Face, Surface}, - shape::{LocalForm, Shape}, + shape::LocalForm, validation::{validate, Validated, ValidationConfig, ValidationError}, }; use fj_math::{Aabb, Point, Scalar}; @@ -16,8 +16,6 @@ impl ToShape for fj::Circle { _: Tolerance, _: &mut DebugInfo, ) -> Result>, ValidationError> { - let mut tmp = Shape::new(); - // Circles have just a single round edge with no vertices. So none need // to be added here. @@ -29,14 +27,12 @@ impl ToShape for fj::Circle { let cycle_canonical = Cycle::new(vec![edge.canonical().clone()]); let surface = Surface::xy_plane(); - let face = tmp - .insert(Face::new( - surface, - vec![LocalForm::new(cycle_local, cycle_canonical)], - Vec::new(), - self.color(), - )) - .get(); + let face = Face::new( + surface, + vec![LocalForm::new(cycle_local, cycle_canonical)], + Vec::new(), + self.color(), + ); validate(vec![face], config) } From eb721499d5ab25d06b11c6c8e506547ab3b5735f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Jun 2022 16:13:52 +0200 Subject: [PATCH 8/8] Remove `Shape` and supporting infrastructure --- crates/fj-kernel/src/shape/api.rs | 206 ------------------------- crates/fj-kernel/src/shape/mod.rs | 10 +- crates/fj-kernel/src/shape/object.rs | 109 ------------- crates/fj-kernel/src/shape/stores.rs | 222 --------------------------- 4 files changed, 1 insertion(+), 546 deletions(-) delete mode 100644 crates/fj-kernel/src/shape/api.rs delete mode 100644 crates/fj-kernel/src/shape/object.rs delete mode 100644 crates/fj-kernel/src/shape/stores.rs diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs deleted file mode 100644 index 6e239f12d..000000000 --- a/crates/fj-kernel/src/shape/api.rs +++ /dev/null @@ -1,206 +0,0 @@ -use crate::objects::{Curve, Cycle, Edge, Face, Surface, Vertex}; - -use super::{ - stores::{Store, Stores}, - Handle, Iter, Object, -}; - -/// The boundary representation of a shape -#[derive(Clone, Debug)] -pub struct Shape { - stores: Stores, -} - -impl Shape { - /// Construct a new shape - pub fn new() -> Self { - Self { - stores: Stores { - curves: Store::new(), - surfaces: Store::new(), - - vertices: Store::new(), - edges: Store::new(), - cycles: Store::new(), - faces: Store::new(), - }, - } - } - - /// Assign a label to the shape - /// - /// The assigned label will be part of the `Debug` representation of - /// `Handle`s, making it way easier to understand which `Handle`s belong to - /// which `Shape`. - pub fn with_label(mut self, label: impl Into) -> Self { - let label = label.into(); - - self.stores.curves.label = Some(label.clone()); - self.stores.surfaces.label = Some(label.clone()); - - self.stores.vertices.label = Some(label.clone()); - self.stores.edges.label = Some(label.clone()); - self.stores.cycles.label = Some(label.clone()); - self.stores.faces.label = Some(label); - - self - } - - /// Insert an object into the shape - /// - /// Validates the object, and returns an error if it is not valid. See the - /// documentation of each object for validation requirements. - pub fn insert(&mut self, object: T) -> Handle { - self.stores.get::().insert(object) - } - - /// Access the handle of an object - /// - /// Returns the handle that refers to the given object, if it is part of the - /// shape. Returns `None`, if it isn't. - /// - /// # Implementation note - /// - /// If `object` is present multiple times, the handle of the first that is - /// found is returned. This is weird. It would be better, if objects were - /// unique, but currently they are stored in `Vec`s. - /// - /// This probably isn't worth thinking too much about right now. At some - /// point, we need smarter and probably more performant object storage - /// anyway. - pub fn get_handle(&self, object: &T) -> Option> { - self.stores - .get::() - .iter() - .find(|obj| &obj.get() == object) - } - - /// Get handle of an identical object, if it exists, or add the object - /// - /// In any case, returns a handle that refers to an object that is identical - /// to the provided object. - pub fn get_handle_or_insert(&mut self, object: T) -> Handle { - if let Some(handle) = self.get_handle(&object) { - return handle; - } - - self.insert(object) - } - - /// Merge the provided object into the shape - /// - /// The provided object is inserted into the shape. Each objects it - /// references is either also inserted, or, if the shape already contains an - /// object that is identical, the referencing object will reference the - /// already present object. - /// - /// This is done recursively. - pub fn merge(&mut self, object: T) -> Handle { - object.merge_into(self) - } - - /// Access an iterator over all curves - /// - /// The caller must not make any assumptions about the order of curves. - pub fn curves(&self) -> Iter> { - self.stores.curves.iter() - } - - /// Access an iterator over all surfaces - /// - /// The caller must not make any assumptions about the order of surfaces. - pub fn surfaces(&self) -> Iter { - self.stores.surfaces.iter() - } - - /// Access iterator over all vertices - /// - /// The caller must not make any assumptions about the order of vertices. - pub fn vertices(&self) -> Iter { - self.stores.vertices.iter() - } - - /// Access iterator over all edges - /// - /// The caller must not make any assumptions about the order of edges. - pub fn edges(&self) -> Iter> { - self.stores.edges.iter() - } - - /// Access an iterator over all cycles - /// - /// The caller must not make any assumptions about the order of cycles. - pub fn cycles(&self) -> Iter> { - self.stores.cycles.iter() - } - - /// Access an iterator over all faces - /// - /// The caller must not make any assumptions about the order of faces. - pub fn faces(&self) -> Iter { - self.stores.faces.iter() - } -} - -impl Default for Shape { - fn default() -> Self { - Self::new() - } -} - -#[cfg(test)] -mod tests { - use fj_math::Point; - - use crate::{ - objects::{Curve, Cycle, Edge, Face, Surface, Vertex, VerticesOfEdge}, - shape::{LocalForm, Shape}, - }; - - #[test] - - fn get_handle() { - let mut shape = Shape::new(); - - let point = Point::from([1., 0., 0.]); - let curve = Curve::x_axis(); - let surface = Surface::xy_plane(); - - assert!(shape.get_handle(&curve).is_none()); - assert!(shape.get_handle(&surface).is_none()); - - let curve = shape.insert(curve); - let surface = shape.insert(surface); - - assert!(shape.get_handle(&curve.get()).as_ref() == Some(&curve)); - assert!(shape.get_handle(&surface.get()).as_ref() == Some(&surface)); - - let vertex = Vertex { point }; - let edge = Edge { - curve: LocalForm::canonical_only(curve.get()), - vertices: VerticesOfEdge::none(), - }; - - assert!(shape.get_handle(&vertex).is_none()); - assert!(shape.get_handle(&edge).is_none()); - - let vertex = shape.insert(vertex); - let edge = shape.insert(edge); - - assert!(shape.get_handle(&vertex.get()).as_ref() == Some(&vertex)); - assert!(shape.get_handle(&edge.get()).as_ref() == Some(&edge)); - - let cycle = Cycle::new(vec![edge.get()]); - assert!(shape.get_handle(&cycle).is_none()); - - let cycle = shape.insert(cycle); - assert!(shape.get_handle(&cycle.get()).as_ref() == Some(&cycle)); - - let face = - Face::new(surface.get(), Vec::new(), Vec::new(), [0, 0, 0, 0]); - assert!(shape.get_handle(&face).is_none()); - - let face = shape.insert(face); - assert!(shape.get_handle(&face.get()).as_ref() == Some(&face)); - } -} diff --git a/crates/fj-kernel/src/shape/mod.rs b/crates/fj-kernel/src/shape/mod.rs index b71f45e78..741ffdec3 100644 --- a/crates/fj-kernel/src/shape/mod.rs +++ b/crates/fj-kernel/src/shape/mod.rs @@ -2,14 +2,6 @@ //! //! See [`Shape`], which is the main entry point to this API. -mod api; mod local; -mod object; -mod stores; -pub use self::{ - api::Shape, - local::LocalForm, - object::Object, - stores::{Handle, Iter}, -}; +pub use self::local::LocalForm; diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs deleted file mode 100644 index e6318e8cc..000000000 --- a/crates/fj-kernel/src/shape/object.rs +++ /dev/null @@ -1,109 +0,0 @@ -use crate::objects::{ - Curve, Cycle, Edge, Face, Surface, Vertex, VerticesOfEdge, -}; - -use super::{Handle, LocalForm, Shape}; - -/// Marker trait for geometric and topological objects -pub trait Object: 'static + Clone + PartialEq + private::Sealed { - /// Internal function - /// - /// Please consider using [`Shape::merge`] instead. - fn merge_into(self, shape: &mut Shape) -> Handle; -} - -impl private::Sealed for Curve<3> {} -impl private::Sealed for Surface {} - -impl private::Sealed for Vertex {} -impl private::Sealed for Edge<3> {} -impl private::Sealed for Cycle<3> {} -impl private::Sealed for Face {} - -impl Object for Curve<3> { - fn merge_into(self, shape: &mut Shape) -> Handle { - shape.get_handle_or_insert(self) - } -} - -impl Object for Surface { - fn merge_into(self, shape: &mut Shape) -> Handle { - shape.get_handle_or_insert(self) - } -} - -impl Object for Vertex { - fn merge_into(self, shape: &mut Shape) -> Handle { - shape.get_handle_or_insert(Vertex { point: self.point }) - } -} - -impl Object for Edge<3> { - fn merge_into(self, shape: &mut Shape) -> Handle { - let curve = self.curve().merge_into(shape); - - let vertices = self.vertices.convert(|vertex| { - let canonical = vertex.canonical(); - let canonical = canonical.merge_into(shape); - LocalForm::new(*vertex.local(), canonical.get()) - }); - - shape.get_handle_or_insert(Edge { - curve: LocalForm::canonical_only(curve.get()), - vertices: VerticesOfEdge::new(vertices), - }) - } -} - -impl Object for Cycle<3> { - fn merge_into(self, shape: &mut Shape) -> Handle { - let mut edges = Vec::new(); - for edge in self.edges { - let edge = edge.canonical().clone(); - let edge = edge.merge_into(shape); - edges.push(edge.get()); - } - - shape.get_handle_or_insert(Cycle::new(edges)) - } -} - -impl Object for Face { - fn merge_into(self, shape: &mut Shape) -> Handle { - match self { - Face::Face(face) => { - let surface = face.surface.merge_into(shape); - - let mut exts = Vec::new(); - for cycle in face.exteriors.as_local_form() { - let merged = cycle.canonical().clone().merge_into(shape); - exts.push(LocalForm::new( - cycle.local().clone(), - merged.get(), - )); - } - - let mut ints = Vec::new(); - for cycle in face.interiors.as_local_form() { - let merged = cycle.canonical().clone().merge_into(shape); - ints.push(LocalForm::new( - cycle.local().clone(), - merged.get(), - )); - } - - shape.get_handle_or_insert(Face::new( - surface.get(), - exts, - ints, - face.color, - )) - } - Face::Triangles(_) => shape.get_handle_or_insert(self), - } - } -} - -mod private { - pub trait Sealed {} -} diff --git a/crates/fj-kernel/src/shape/stores.rs b/crates/fj-kernel/src/shape/stores.rs deleted file mode 100644 index 468781bbe..000000000 --- a/crates/fj-kernel/src/shape/stores.rs +++ /dev/null @@ -1,222 +0,0 @@ -use std::{ - fmt, - hash::{Hash, Hasher}, - sync::Arc, -}; - -use anymap::AnyMap; -use parking_lot::{RwLock, RwLockReadGuard}; -use slotmap::{DefaultKey, SlotMap}; - -use crate::objects::{Curve, Cycle, Edge, Face, Surface, Vertex}; - -use super::Object; - -#[derive(Clone, Debug)] -pub struct Stores { - pub curves: Store>, - pub surfaces: Store, - - pub vertices: Store, - pub edges: Store>, - pub cycles: Store>, - pub faces: Store, -} - -impl Stores { - pub fn get(&self) -> Store { - let mut stores = AnyMap::new(); - - stores.insert(self.curves.clone()); - stores.insert(self.surfaces.clone()); - - stores.insert(self.vertices.clone()); - stores.insert(self.edges.clone()); - stores.insert(self.cycles.clone()); - stores.insert(self.faces.clone()); - - stores - .remove::>() - // Can't panic, as `T` is bound by `Object`, and we added the stores - // for all types of objects above. - .expect("Invalid object type") - } -} - -#[derive(Debug)] -pub struct Store { - pub(super) label: Option, - objects: Arc>>, -} - -impl Store { - pub fn new() -> Self { - Self { - label: None, - objects: Arc::new(RwLock::new(SlotMap::new())), - } - } - - pub fn insert(&mut self, object: T) -> Handle { - let key = self.objects.write().insert(object); - Handle::new(key, self.clone()) - } - - pub fn read(&self) -> RwLockReadGuard> { - self.objects.read() - } - - pub fn iter(&self) -> Iter { - // The allocation here is unfortunate, but I think it's fine for now. If - // this turns into a performance issue, it should be possible to avoid - // it by adding methods to `Store`, that are geared towards implementing - // iterators. - Iter { - elements: self - .objects - .read() - .iter() - .map(|(key, _)| Handle::new(key, self.clone())) - .collect(), - } - } - - fn ptr(&self) -> *const () { - Arc::as_ptr(&self.objects) as _ - } -} - -// Deriving `Clone` would only derive `Clone` where `T: Clone`. This -// implementation doesn't have that limitation, providing `Clone` for all -// `Store`s instead. -impl Clone for Store { - fn clone(&self) -> Self { - Self { - label: self.label.clone().map(|label| format!("{} (clone)", label)), - objects: self.objects.clone(), - } - } -} - -impl Default for Store { - fn default() -> Self { - Self::new() - } -} - -impl PartialEq for Store { - fn eq(&self, other: &Self) -> bool { - self.ptr().eq(&other.ptr()) - } -} - -impl Eq for Store {} - -impl Ord for Store { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.ptr().cmp(&other.ptr()) - } -} - -impl PartialOrd for Store { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Hash for Store { - fn hash(&self, state: &mut H) { - self.ptr().hash(state); - } -} - -pub type Objects = SlotMap; - -/// A handle to an object stored within [`Shape`] -/// -/// If an object of type `T` (this could be `Curve`, `Vertex`, etc.) is added to -/// `Shape`, a `Handle` is returned. This handle is then used in topological -/// types to refer to the object, instead of having those types own an instance -/// of the object. -/// -/// This approach has two advantages: -/// -/// 1. The object can't be mutated through the handle. Since an object can be -/// referred to by multiple other objects, mutating it locally would have no -/// effect on those other references. `Handle` preventing that removes this -/// source of errors. -/// 2. The object is guaranteed to be in `Shape`, as `Handle`s can't be created -/// any other way. This means that if the `Shape` needs to be modified, any -/// objects can be updated once, without requiring an update of all the other -/// objects that reference it. -/// -/// # Equality -/// -/// The equality of [`Handle`] is very strictly defined in terms of identity. -/// Two [`Handle`]s are considered equal, if they refer to objects in the same -/// memory location. -#[derive(Clone, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Handle { - key: DefaultKey, - store: Store, -} - -impl Handle { - pub(super) fn new(key: DefaultKey, store: Store) -> Self { - Self { key, store } - } - - /// Access the object that the handle references - pub fn get(&self) -> T - where - T: Clone, - { - self.store - .read() - .get(self.key) - // Can't panic, unless the handle was invalid in the first place. - // Objects are never removed from `Store`, so if we have a handle - // pointing to it, it should be there. - .expect("Invalid handle") - .clone() - } -} - -impl fmt::Debug for Handle -where - T: fmt::Debug, -{ - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("Handle") - .field("shape", &self.store.label.as_deref().unwrap_or("unnamed")) - .field("object", &self.get()) - .finish() - } -} - -/// An iterator over geometric or topological objects -/// -/// Returned by various methods of the [`Shape`] API. -pub struct Iter { - elements: Vec>, -} - -impl Iter { - /// Convert this iterator over handles into an iterator over values - /// - /// This is a convenience method, for cases where no `Handle` is needed. - pub fn values(self) -> impl Iterator - where - T: Clone, - { - self.elements.into_iter().map(|handle| handle.get()) - } -} - -impl Iterator for Iter { - type Item = Handle; - - fn next(&mut self) -> Option { - self.elements.pop() - } -}