Merge pull request #416 from hannobraun/insert

Replace `add_*` method with `Shape::insert`

Cargo.lock

@@ -59,6 +59,12 @@ version = "1.0.56"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4361135be9122e0870de935d7c439aef945b9f9ddd4199a553b5270b49c82a27"
 
+[[package]]
+name = "anymap"
+version = "1.0.0-beta.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8f1f8f5a6f3d50d89e3797d7593a50f96bb2aaa20ca0cc7be1fb673232c91d72"
+
 [[package]]
 name = "approx"
 version = "0.3.2"
@@ -671,6 +677,7 @@ name = "fj-kernel"
 version = "0.5.0"
 dependencies = [
  "anyhow",
+ "anymap",
  "approx 0.5.1",
  "fj-debug",
  "fj-math",

fj-kernel/Cargo.toml

@@ -13,6 +13,7 @@ categories  = ["mathematics"]
 
 [dependencies]
 anyhow      = "1.0.56"
+anymap      = "1.0.0-beta.2"
 approx      = "0.5.1"
 map-macro   = "0.2.0"
 nalgebra    = "0.30.0"

fj-kernel/src/algorithms/approximation.rs

@@ -176,11 +176,11 @@ mod tests {
         let da =
             Edge::build(&mut shape).line_segment_from_vertices([v4, v1])?;
 
-        let abcd = shape.topology().add_cycle(Cycle {
+        let abcd = shape.insert(Cycle {
             edges: vec![ab, bc, cd, da],
         })?;
 
-        let surface = shape.geometry().add_surface(Surface::x_y_plane());
+        let surface = shape.insert(Surface::x_y_plane())?;
         let face = Face::Face {
             surface,
             exteriors: vec![abcd],

fj-kernel/src/algorithms/sweep.rs

@@ -26,20 +26,15 @@ pub fn sweep_shape(
 
     // Create the new vertices.
     for vertex_source in source.topology().vertices() {
-        let point_bottom =
+        let point_bottom = target.insert(vertex_source.get().point()).unwrap();
-            target.geometry().add_point(vertex_source.get().point());
+        let point_top = target.insert(point_bottom.get() + path).unwrap();
-        let point_top = target.geometry().add_point(point_bottom.get() + path);
 
         let vertex_bottom = target
-            .topology()
+            .insert(Vertex {
-            .add_vertex(Vertex {
                 point: point_bottom,
             })
             .unwrap();
-        let vertex_top = target
+        let vertex_top = target.insert(Vertex { point: point_top }).unwrap();
-            .topology()
-            .add_vertex(Vertex { point: point_top })
-            .unwrap();
 
         source_to_bottom
             .vertices
@@ -49,25 +44,22 @@ pub fn sweep_shape(
 
     // Create the new edges.
     for edge_source in source.topology().edges() {
-        let curve_bottom =
+        let curve_bottom = target.insert(edge_source.get().curve()).unwrap();
-            target.geometry().add_curve(edge_source.get().curve());
         let curve_top = target
-            .geometry()
+            .insert(curve_bottom.get().transform(&translation))
-            .add_curve(curve_bottom.get().transform(&translation));
+            .unwrap();
 
         let vertices_bottom = source_to_bottom.vertices_for_edge(&edge_source);
         let vertices_top = source_to_top.vertices_for_edge(&edge_source);
 
         let edge_bottom = target
-            .topology()
+            .insert(Edge {
-            .add_edge(Edge {
                 curve: curve_bottom,
                 vertices: vertices_bottom,
             })
             .unwrap();
         let edge_top = target
-            .topology()
+            .insert(Edge {
-            .add_edge(Edge {
                 curve: curve_top,
                 vertices: vertices_top,
             })
@@ -85,15 +77,11 @@ pub fn sweep_shape(
         let edges_top = source_to_top.edges_for_cycle(&cycle_source);
 
         let cycle_bottom = target
-            .topology()
+            .insert(Cycle {
-            .add_cycle(Cycle {
                 edges: edges_bottom,
             })
             .unwrap();
-        let cycle_top = target
+        let cycle_top = target.insert(Cycle { edges: edges_top }).unwrap();
-            .topology()
-            .add_cycle(Cycle { edges: edges_top })
-            .unwrap();
 
         source_to_bottom
             .cycles
@@ -103,11 +91,10 @@ pub fn sweep_shape(
 
     // Create top faces.
     for face_source in source.topology().faces().values() {
-        let surface_bottom =
+        let surface_bottom = target.insert(face_source.surface()).unwrap();
-            target.geometry().add_surface(face_source.surface());
         let surface_top = target
-            .geometry()
+            .insert(surface_bottom.get().transform(&translation))
-            .add_surface(surface_bottom.get().transform(&translation));
+            .unwrap();
 
         let exteriors_bottom =
             source_to_bottom.exteriors_for_face(&face_source);
@@ -117,8 +104,7 @@ pub fn sweep_shape(
         let interiors_top = source_to_top.interiors_for_face(&face_source);
 
         target
-            .topology()
+            .insert(Face::Face {
-            .add_face(Face::Face {
                 surface: surface_bottom,
                 exteriors: exteriors_bottom,
                 interiors: interiors_bottom,
@@ -126,8 +112,7 @@ pub fn sweep_shape(
             })
             .unwrap();
         target
-            .topology()
+            .insert(Face::Face {
-            .add_face(Face::Face {
                 surface: surface_top,
                 exteriors: exteriors_top,
                 interiors: interiors_top,
@@ -178,10 +163,7 @@ pub fn sweep_shape(
                 s.set_color(color);
             }
 
-            target
+            target.insert(Face::Triangles(side_face)).unwrap();
-                .topology()
-                .add_face(Face::Triangles(side_face))
-                .unwrap();
         } else {
             // If there's no continuous edge, we can create the non-
             // continuous faces using boundary representation.
@@ -208,8 +190,8 @@ pub fn sweep_shape(
                             .entry(vertex_bottom.clone())
                             .or_insert_with(|| {
                                 let curve = target
-                                    .geometry()
+                                    .insert(edge_source.get().curve())
-                                    .add_curve(edge_source.get().curve());
+                                    .unwrap();
 
                                 let vertex_top = source_to_top
                                     .vertices
@@ -218,8 +200,7 @@ pub fn sweep_shape(
                                     .clone();
 
                                 target
-                                    .topology()
+                                    .insert(Edge {
-                                    .add_edge(Edge {
                                         curve,
                                         vertices: Some([
                                             vertex_bottom,
@@ -239,16 +220,15 @@ pub fn sweep_shape(
                 let top_edge =
                     source_to_top.edges.get(edge_source).unwrap().clone();
 
-                let surface = target.geometry().add_surface(
+                let surface = target
-                    Surface::SweptCurve(SweptCurve {
+                    .insert(Surface::SweptCurve(SweptCurve {
                         curve: bottom_edge.get().curve(),
                         path,
-                    }),
+                    }))
-                );
+                    .unwrap();
 
                 let cycle = target
-                    .topology()
+                    .insert(Cycle {
-                    .add_cycle(Cycle {
                         edges: vec![
                             bottom_edge,
                             top_edge,
@@ -259,8 +239,7 @@ pub fn sweep_shape(
                     .unwrap();
 
                 target
-                    .topology()
+                    .insert(Face::Face {
-                    .add_face(Face::Face {
                         surface,
                         exteriors: vec![cycle],
                         interiors: Vec::new(),
@@ -401,13 +380,13 @@ mod tests {
         fn new([a, b, c]: [impl Into<Point<3>>; 3]) -> anyhow::Result<Self> {
             let mut shape = Shape::new();
 
-            let a = shape.geometry().add_point(a.into());
+            let a = shape.insert(a.into())?;
-            let b = shape.geometry().add_point(b.into());
+            let b = shape.insert(b.into())?;
-            let c = shape.geometry().add_point(c.into());
+            let c = shape.insert(c.into())?;
 
-            let a = shape.topology().add_vertex(Vertex { point: a })?;
+            let a = shape.insert(Vertex { point: a })?;
-            let b = shape.topology().add_vertex(Vertex { point: b })?;
+            let b = shape.insert(Vertex { point: b })?;
-            let c = shape.topology().add_vertex(Vertex { point: c })?;
+            let c = shape.insert(Vertex { point: c })?;
 
             let ab = Edge::build(&mut shape)
                 .line_segment_from_vertices([a.clone(), b.clone()])?;
@@ -416,15 +395,15 @@ mod tests {
             let ca = Edge::build(&mut shape)
                 .line_segment_from_vertices([c.clone(), a.clone()])?;
 
-            let cycles = shape.topology().add_cycle(Cycle {
+            let cycles = shape.insert(Cycle {
                 edges: vec![ab, bc, ca],
             })?;
 
-            let surface = shape.geometry().add_surface(Surface::SweptCurve(
+            let surface = shape.insert(Surface::SweptCurve(
                 SweptCurve::plane_from_points(
                     [a, b, c].map(|vertex| vertex.get().point()),
                 ),
-            ));
+            ))?;
             let abc = Face::Face {
                 surface,
                 exteriors: vec![cycles],
@@ -432,7 +411,7 @@ mod tests {
                 color: [255, 0, 0, 255],
             };
 
-            let face = shape.topology().add_face(abc)?;
+            let face = shape.insert(abc)?;
 
             Ok(Self { shape, face })
         }

fj-kernel/src/shape/api.rs

@@ -6,7 +6,7 @@ use super::{
     stores::{
         Curves, Cycles, Edges, Faces, Points, Stores, Surfaces, Vertices,
     },
-    Geometry, Topology,
+    Geometry, Object, Topology, ValidationResult,
 };
 
 /// The boundary representation of a shape
@@ -55,6 +55,19 @@ impl Shape {
         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<T>(&mut self, object: T) -> ValidationResult<T>
+    where
+        T: Object,
+    {
+        object.validate(self.min_distance, &self.stores)?;
+        let handle = self.stores.get::<T>().insert(object);
+        Ok(handle)
+    }
+
     /// Access the shape's geometry
     pub fn geometry(&mut self) -> Geometry {
         Geometry {
@@ -69,7 +82,6 @@ impl Shape {
     /// Access the shape's topology
     pub fn topology(&mut self) -> Topology {
         Topology {
-            min_distance: self.min_distance,
             stores: self.stores.clone(),
             _lifetime: PhantomData,
         }

fj-kernel/src/shape/geometry.rs

@@ -7,24 +7,10 @@ use crate::{
 
 use super::{
     stores::{Curves, Faces, Points, Surfaces},
-    Handle, Iter,
+    Iter,
 };
 
 /// API to access a shape's geometry
-///
-/// Other than topology, geometry doesn't need to be validated. Hence adding
-/// geometry is infallible.
-///
-/// There are several reasons for this:
-/// - Geometry doesn't refer to other objects, so structural validation doesn't
-///   apply.
-/// - There simply no reason that geometry needs to be unique. In addition, it's
-///   probably quite hard to rule out generating duplicate geometry. Think about
-///   line segment edges that are on identical lines, but are created
-///   separately.
-/// - Geometric validation doesn't apply either. It simply doesn't matter, if
-///   curves or surfaces intersect, for example, as long as they don't do that
-///   where an edge or face is defined.
 pub struct Geometry<'r> {
     pub(super) points: &'r mut Points,
     pub(super) curves: &'r mut Curves,
@@ -41,21 +27,6 @@ pub struct Geometry<'r> {
 }
 
 impl Geometry<'_> {
-    /// Add a point to the shape
-    pub fn add_point(&mut self, point: Point<3>) -> Handle<Point<3>> {
-        self.points.insert(point)
-    }
-
-    /// Add a curve to the shape
-    pub fn add_curve(&mut self, curve: Curve) -> Handle<Curve> {
-        self.curves.insert(curve)
-    }
-
-    /// Add a surface to the shape
-    pub fn add_surface(&mut self, surface: Surface) -> Handle<Surface> {
-        self.surfaces.insert(surface)
-    }
-
     /// Transform the geometry of the shape
     ///
     /// Since the topological types refer to geometry, and don't contain any

fj-kernel/src/shape/mod.rs

@@ -4,6 +4,7 @@
 
 mod api;
 mod geometry;
+mod object;
 mod stores;
 mod topology;
 mod validate;
@@ -11,6 +12,7 @@ mod validate;
 pub use self::{
     api::Shape,
     geometry::Geometry,
+    object::Object,
     stores::{Handle, Iter},
     topology::Topology,
     validate::{StructuralIssues, ValidationError, ValidationResult},

fj-kernel/src/shape/object.rs

@@ -0,0 +1,33 @@
+use fj_math::Point;
+
+use crate::{
+    geometry::{Curve, Surface},
+    topology::{Cycle, Edge, Face, Vertex},
+};
+
+use super::validate::Validate;
+
+/// Marker trait for geometric and topological objects
+pub trait Object: 'static + Validate + private::Sealed {}
+
+impl private::Sealed for Point<3> {}
+impl private::Sealed for Curve {}
+impl private::Sealed for Surface {}
+
+impl private::Sealed for Vertex {}
+impl private::Sealed for Edge {}
+impl private::Sealed for Cycle {}
+impl private::Sealed for Face {}
+
+impl Object for Point<3> {}
+impl Object for Curve {}
+impl Object for Surface {}
+
+impl Object for Vertex {}
+impl Object for Edge {}
+impl Object for Cycle {}
+impl Object for Face {}
+
+mod private {
+    pub trait Sealed {}
+}

fj-kernel/src/shape/stores.rs

@@ -3,6 +3,7 @@ use std::{
     sync::Arc,
 };
 
+use anymap::AnyMap;
 use fj_math::Point;
 use parking_lot::{RwLock, RwLockReadGuard};
 use slotmap::{DefaultKey, SlotMap};
@@ -12,6 +13,8 @@ use crate::{
     topology::{Cycle, Edge, Face, Vertex},
 };
 
+use super::Object;
+
 #[derive(Clone, Debug)]
 pub struct Stores {
     pub points: Points,
@@ -24,6 +27,30 @@ pub struct Stores {
     pub faces: Faces,
 }
 
+impl Stores {
+    pub fn get<T>(&mut self) -> Store<T>
+    where
+        T: Object,
+    {
+        let mut stores = AnyMap::new();
+
+        stores.insert(self.points.clone());
+        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::<Store<T>>()
+            // Can't panic, as `T` is bound by `Object`, and we added the stores
+            // for all types of objects above.
+            .expect("Invalid object type")
+    }
+}
+
 pub type Points = Store<Point<3>>;
 pub type Curves = Store<Curve>;
 pub type Surfaces = Store<Surface>;
@@ -98,6 +125,12 @@ impl<T> Clone for Store<T> {
     }
 }
 
+impl<T> Default for Store<T> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl<T> PartialEq for Store<T> {
     fn eq(&self, other: &Self) -> bool {
         self.ptr().eq(&other.ptr())

fj-kernel/src/shape/topology.rs

@@ -1,95 +1,16 @@
 use std::marker::PhantomData;
 
-use fj_math::Scalar;
-
 use crate::topology::{Cycle, Edge, Face, Vertex};
 
-use super::{stores::Stores, validate::Validate as _, Iter, ValidationResult};
+use super::{stores::Stores, Iter};
 
 /// The vertices of a shape
 pub struct Topology<'r> {
-    pub(super) min_distance: Scalar,
     pub(super) stores: Stores,
     pub(super) _lifetime: PhantomData<&'r ()>,
 }
 
 impl Topology<'_> {
-    /// Add a vertex to the shape
-    ///
-    /// Validates that the vertex is structurally sound (i.e. the point it
-    /// refers to is part of the shape). Returns an error, if that is not the
-    /// case.
-    ///
-    /// Logs a warning, if the vertex is not unique, meaning if another vertex
-    /// defined by the same point already exists.
-    ///
-    /// In the context of of vertex uniqueness, points that are close to each
-    /// other are considered identical. The minimum distance between distinct
-    /// vertices can be configured using [`Shape::with_minimum_distance`].
-    ///
-    /// # Implementation note
-    ///
-    /// This method is intended to actually validate vertex uniqueness: To
-    /// panic, if duplicate vertices are found. This is currently not possible,
-    /// as the presence of bugs in the sweep and transform code would basically
-    /// break ever model, due to validation errors.
-    ///
-    /// In the future, this method is likely to validate more than it already
-    /// does. See documentation of [`crate::kernel`] for some context on that.
-    pub fn add_vertex(&mut self, vertex: Vertex) -> ValidationResult<Vertex> {
-        vertex.validate(self.min_distance, &self.stores)?;
-        let handle = self.stores.vertices.insert(vertex);
-        Ok(handle)
-    }
-
-    /// Add an edge to the shape
-    ///
-    /// Validates that the edge is structurally sound (i.e. the curve and
-    /// vertices it refers to are part of the shape). Returns an error, if that
-    /// is not the case.
-    ///
-    /// # Vertices
-    ///
-    /// If vertices are provided in `vertices`, they must be on `curve`.
-    ///
-    /// This constructor will convert the vertices into curve coordinates. If
-    /// they are not on the curve, this will result in their projection being
-    /// converted into curve coordinates, which is likely not the caller's
-    /// intention.
-    pub fn add_edge(&mut self, edge: Edge) -> ValidationResult<Edge> {
-        edge.validate(self.min_distance, &self.stores)?;
-        let handle = self.stores.edges.insert(edge);
-        Ok(handle)
-    }
-
-    /// Add a cycle to the shape
-    ///
-    /// Validates that the cycle is structurally sound (i.e. the edges it refers
-    /// to are part of the shape). Returns an error, if that is not the case.
-    ///
-    /// # Implementation note
-    ///
-    /// The validation of the cycle should be extended to cover more cases:
-    /// - That those edges form a cycle.
-    /// - That the cycle is not self-overlapping.
-    /// - That there exists no duplicate cycle, with the same edges.
-    pub fn add_cycle(&mut self, cycle: Cycle) -> ValidationResult<Cycle> {
-        cycle.validate(self.min_distance, &self.stores)?;
-        let handle = self.stores.cycles.insert(cycle);
-        Ok(handle)
-    }
-
-    /// Add a face to the shape
-    ///
-    /// Validates that the face is structurally sound (i.e. the surface and
-    /// cycles it refers to are part of the shape). Returns an error, if that is
-    /// not the case.
-    pub fn add_face(&mut self, face: Face) -> ValidationResult<Face> {
-        face.validate(self.min_distance, &self.stores)?;
-        let handle = self.stores.faces.insert(face);
-        Ok(handle)
-    }
-
     /// Access iterator over all vertices
     ///
     /// The caller must not make any assumptions about the order of vertices.
@@ -138,24 +59,24 @@ mod tests {
         let mut shape = Shape::new().with_min_distance(MIN_DISTANCE);
         let mut other = Shape::new();
 
-        let point = shape.geometry().add_point(Point::from([0., 0., 0.]));
+        let point = shape.insert(Point::from([0., 0., 0.]))?;
-        shape.topology().add_vertex(Vertex { point })?;
+        shape.insert(Vertex { point })?;
 
         // Should fail, as `point` is not part of the shape.
-        let point = other.geometry().add_point(Point::from([1., 0., 0.]));
+        let point = other.insert(Point::from([1., 0., 0.]))?;
-        let result = shape.topology().add_vertex(Vertex { point });
+        let result = shape.insert(Vertex { point });
         assert!(matches!(result, Err(ValidationError::Structural(_))));
 
         // `point` is too close to the original point. `assert!` is commented,
         // because that only causes a warning to be logged right now.
-        let point = shape.geometry().add_point(Point::from([5e-8, 0., 0.]));
+        let point = shape.insert(Point::from([5e-8, 0., 0.]))?;
-        let result = shape.topology().add_vertex(Vertex { point });
+        let result = shape.insert(Vertex { point });
         assert!(matches!(result, Err(ValidationError::Uniqueness)));
 
         // `point` is farther than `MIN_DISTANCE` away from original point.
         // Should work.
-        let point = shape.geometry().add_point(Point::from([5e-6, 0., 0.]));
+        let point = shape.insert(Point::from([5e-6, 0., 0.]))?;
-        shape.topology().add_vertex(Vertex { point })?;
+        shape.insert(Vertex { point })?;
 
         Ok(())
     }
@@ -171,8 +92,7 @@ mod tests {
 
         // Shouldn't work. Nothing has been added to `shape`.
         let err = shape
-            .topology()
+            .insert(Edge {
-            .add_edge(Edge {
                 curve: curve.clone(),
                 vertices: Some([a.clone(), b.clone()]),
             })
@@ -186,7 +106,7 @@ mod tests {
         let b = Vertex::build(&mut shape).from_point([2., 0., 0.])?;
 
         // Everything has been added to `shape` now. Should work!
-        shape.topology().add_edge(Edge {
+        shape.insert(Edge {
             curve,
             vertices: Some([a, b]),
         })?;
@@ -202,8 +122,7 @@ mod tests {
         // Trying to refer to edge that is not from the same shape. Should fail.
         let edge = other.add_edge()?;
         let err = shape
-            .topology()
+            .insert(Cycle {
-            .add_cycle(Cycle {
                 edges: vec![edge.clone()],
             })
             .unwrap_err();
@@ -211,7 +130,7 @@ mod tests {
 
         // Referring to edge that *is* from the same shape. Should work.
         let edge = shape.add_edge()?;
-        shape.topology().add_cycle(Cycle { edges: vec![edge] })?;
+        shape.insert(Cycle { edges: vec![edge] })?;
 
         Ok(())
     }
@@ -226,8 +145,7 @@ mod tests {
 
         // Nothing has been added to `shape`. Should fail.
         let err = shape
-            .topology()
+            .insert(Face::Face {
-            .add_face(Face::Face {
                 surface: surface.clone(),
                 exteriors: vec![cycle.clone()],
                 interiors: Vec::new(),
@@ -241,7 +159,7 @@ mod tests {
         let cycle = shape.add_cycle()?;
 
         // Everything has been added to `shape` now. Should work!
-        shape.topology().add_face(Face::Face {
+        shape.insert(Face::Face {
             surface,
             exteriors: vec![cycle],
             interiors: Vec::new(),
@@ -265,11 +183,11 @@ mod tests {
         }
 
         fn add_curve(&mut self) -> Handle<Curve> {
-            self.geometry().add_curve(Curve::x_axis())
+            self.insert(Curve::x_axis()).unwrap()
         }
 
         fn add_surface(&mut self) -> Handle<Surface> {
-            self.geometry().add_surface(Surface::x_y_plane())
+            self.insert(Surface::x_y_plane()).unwrap()
         }
 
         fn add_edge(&mut self) -> anyhow::Result<Handle<Edge>> {
@@ -277,8 +195,8 @@ mod tests {
                 let point = self.next_point;
                 self.next_point.x += Scalar::ONE;
 
-                let point = self.geometry().add_point(point);
+                let point = self.insert(point).unwrap();
-                self.topology().add_vertex(Vertex { point }).unwrap()
+                self.insert(Vertex { point }).unwrap()
             });
             let edge = Edge::build(&mut self.inner)
                 .line_segment_from_vertices(vertices)?;
@@ -288,8 +206,7 @@ mod tests {
 
         fn add_cycle(&mut self) -> anyhow::Result<Handle<Cycle>> {
             let edge = self.add_edge()?;
-            let cycle =
+            let cycle = self.insert(Cycle { edges: vec![edge] })?;
-                self.topology().add_cycle(Cycle { edges: vec![edge] })?;
             Ok(cycle)
         }
     }

fj-kernel/src/shape/validate.rs

@@ -1,6 +1,6 @@
 use std::collections::HashSet;
 
-use fj_math::Scalar;
+use fj_math::{Point, Scalar};
 
 use crate::{
     geometry::{Curve, Surface},
@@ -17,7 +17,31 @@ pub trait Validate {
     ) -> Result<(), ValidationError>;
 }
 
+impl Validate for Point<3> {
+    fn validate(&self, _: Scalar, _: &Stores) -> Result<(), ValidationError> {
+        Ok(())
+    }
+}
+
+impl Validate for Curve {
+    fn validate(&self, _: Scalar, _: &Stores) -> Result<(), ValidationError> {
+        Ok(())
+    }
+}
+
+impl Validate for Surface {
+    fn validate(&self, _: Scalar, _: &Stores) -> Result<(), ValidationError> {
+        Ok(())
+    }
+}
+
 impl Validate for Vertex {
+    /// Validate the vertex
+    ///
+    /// # Implementation note
+    ///
+    /// In the future, this method is likely to validate more than it already
+    /// does. See documentation of [`crate::kernel`] for some context on that.
     fn validate(
         &self,
         min_distance: Scalar,
@@ -72,6 +96,14 @@ impl Validate for Edge {
 }
 
 impl Validate for Cycle {
+    /// Validate the cycle
+    ///
+    /// # Implementation note
+    ///
+    /// The validation of the cycle should be extended to cover more cases:
+    /// - That those edges form a cycle.
+    /// - That the cycle is not self-overlapping.
+    /// - That there exists no duplicate cycle, with the same edges.
     fn validate(
         &self,
         _: Scalar,

fj-kernel/src/topology/builder.rs

@@ -23,8 +23,8 @@ impl<'r> VertexBuilder<'r> {
         self,
         point: impl Into<Point<3>>,
     ) -> ValidationResult<Vertex> {
-        let point = self.shape.geometry().add_point(point.into());
+        let point = self.shape.insert(point.into())?;
-        let vertex = self.shape.topology().add_vertex(Vertex { point })?;
+        let vertex = self.shape.insert(Vertex { point })?;
 
         Ok(vertex)
     }
@@ -43,11 +43,11 @@ impl<'r> EdgeBuilder<'r> {
 
     /// Build a circle from a radius
     pub fn circle(self, radius: Scalar) -> ValidationResult<Edge> {
-        let curve = self.shape.geometry().add_curve(Curve::Circle(Circle {
+        let curve = self.shape.insert(Curve::Circle(Circle {
             center: Point::origin(),
             radius: Vector::from([radius, Scalar::ZERO]),
-        }));
+        }))?;
-        let edge = self.shape.topology().add_edge(Edge {
+        let edge = self.shape.insert(Edge {
             curve,
             vertices: None,
         })?;
@@ -60,13 +60,10 @@ impl<'r> EdgeBuilder<'r> {
         self,
         vertices: [Handle<Vertex>; 2],
     ) -> ValidationResult<Edge> {
-        let curve =
+        let curve = self.shape.insert(Curve::Line(Line::from_points(
-            self.shape
-                .geometry()
-                .add_curve(Curve::Line(Line::from_points(
             vertices.clone().map(|vertex| vertex.get().point()),
-                )));
+        )))?;
-        let edge = self.shape.topology().add_edge(Edge {
+        let edge = self.shape.insert(Edge {
             curve,
             vertices: Some(vertices),
         })?;

fj-kernel/src/topology/edges.rs

@@ -17,6 +17,11 @@ use super::{vertices::Vertex, EdgeBuilder};
 ///
 /// Please refer to [`crate::kernel::topology`] for documentation on the
 /// equality of topological objects.
+///
+/// # Validation
+///
+/// A cycle that is part of a [`Shape`] must be structurally sound. That means
+/// the edges it refers to, must be part of the same shape.
 #[derive(Clone, Debug, Eq, Ord, PartialOrd)]
 pub struct Cycle {
     /// The edges that make up the cycle
@@ -53,6 +58,12 @@ impl Hash for Cycle {
 ///
 /// Please refer to [`crate::kernel::topology`] for documentation on the
 /// equality of topological objects.
+///
+/// # Validation
+///
+/// An edge that is part of a [`Shape`] must be structurally sound. That means
+/// the curve and any vertices that it refers to, must be part of the same
+/// shape.
 #[derive(Clone, Debug, Eq, Ord, PartialOrd)]
 pub struct Edge {
     /// Access the curve that defines the edge's geometry

fj-kernel/src/topology/faces.rs

@@ -12,6 +12,11 @@ use super::edges::Cycle;
 ///
 /// Please refer to [`crate::kernel::topology`] for documentation on the
 /// equality of topological objects.
+///
+/// # Validation
+///
+/// A face that is part of a [`Shape`] must be structurally sound. That means
+/// the surface and any cycles it refers to, must be part of the same shape.
 #[derive(Clone, Debug, Eq, Ord, PartialOrd)]
 pub enum Face {
     /// A face of a shape

fj-kernel/src/topology/vertices.rs

@@ -19,6 +19,17 @@ use super::VertexBuilder;
 ///
 /// Please refer to [`crate::kernel::topology`] for documentation on the
 /// equality of topological objects.
+///
+/// # Validation
+///
+/// A vertex that is part of a [`Shape`] must be structurally sound. That means
+/// the point it refers to must be part of the same shape.
+///
+/// Vertices must be unique within a shape, meaning another vertex defined by
+/// the same shape must not exist. In the context of vertex uniqueness, points
+/// that are close to each other are considered identical. The minimum distance
+/// between distinct vertices can be configured using
+/// [`Shape::with_minimum_distance`].
 #[derive(Clone, Debug, Eq, Ord, PartialOrd)]
 pub struct Vertex {
     /// The point that defines the location of the vertex

fj-operations/src/circle.rs

@@ -18,16 +18,12 @@ impl ToShape for fj::Circle {
         let edge = Edge::build(&mut shape)
             .circle(Scalar::from_f64(self.radius()))
             .unwrap();
-        shape
+        shape.insert(Cycle { edges: vec![edge] }).unwrap();
-            .topology()
-            .add_cycle(Cycle { edges: vec![edge] })
-            .unwrap();
 
         let cycles = shape.topology().cycles().collect();
-        let surface = shape.geometry().add_surface(Surface::x_y_plane());
+        let surface = shape.insert(Surface::x_y_plane()).unwrap();
         shape
-            .topology()
+            .insert(Face::Face {
-            .add_face(Face::Face {
                 exteriors: cycles,
                 interiors: Vec::new(),
                 surface,

fj-operations/src/difference_2d.rs

@@ -61,11 +61,10 @@ impl ToShape for fj::Difference2d {
             face_a.surface() == face_b.surface(),
             "Trying to subtract sketches with different surfaces."
         );
-        let surface = shape.geometry().add_surface(face_a.surface());
+        let surface = shape.insert(face_a.surface()).unwrap();
 
         shape
-            .topology()
+            .insert(Face::Face {
-            .add_face(Face::Face {
                 surface,
                 exteriors,
                 interiors,
@@ -91,23 +90,23 @@ fn add_cycle(
 ) -> Handle<Cycle> {
     let mut edges = Vec::new();
     for edge in cycle.get().edges() {
-        let curve = shape.geometry().add_curve(edge.curve());
+        let curve = shape.insert(edge.curve()).unwrap();
 
         let vertices = edge.vertices().clone().map(|vs| {
             vs.map(|vertex| {
                 vertices
                     .entry(vertex.clone())
                     .or_insert_with(|| {
-                        let point = shape.geometry().add_point(vertex.point());
+                        let point = shape.insert(vertex.point()).unwrap();
-                        shape.topology().add_vertex(Vertex { point }).unwrap()
+                        shape.insert(Vertex { point }).unwrap()
                     })
                     .clone()
             })
         });
 
-        let edge = shape.topology().add_edge(Edge { curve, vertices }).unwrap();
+        let edge = shape.insert(Edge { curve, vertices }).unwrap();
         edges.push(edge);
     }
 
-    shape.topology().add_cycle(Cycle { edges }).unwrap()
+    shape.insert(Cycle { edges }).unwrap()
 }

fj-operations/src/group.rs

@@ -40,22 +40,21 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) {
     let mut cycles = HashMap::new();
 
     for point_orig in orig.geometry().points() {
-        let point = target.geometry().add_point(point_orig.get());
+        let point = target.insert(point_orig.get()).unwrap();
         points.insert(point_orig, point);
     }
     for curve_orig in orig.geometry().curves() {
-        let curve = target.geometry().add_curve(curve_orig.get());
+        let curve = target.insert(curve_orig.get()).unwrap();
         curves.insert(curve_orig, curve);
     }
     for surface_orig in orig.geometry().surfaces() {
-        let surface = target.geometry().add_surface(surface_orig.get());
+        let surface = target.insert(surface_orig.get()).unwrap();
         surfaces.insert(surface_orig, surface);
     }
 
     for vertex_orig in orig.topology().vertices() {
         let vertex = target
-            .topology()
+            .insert(Vertex {
-            .add_vertex(Vertex {
                 point: points[&vertex_orig.get().point].clone(),
             })
             .unwrap();
@@ -63,8 +62,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) {
     }
     for edge_orig in orig.topology().edges() {
         let edge = target
-            .topology()
+            .insert(Edge {
-            .add_edge(Edge {
                 curve: curves[&edge_orig.get().curve].clone(),
                 vertices: edge_orig.get().vertices.as_ref().map(|vs| {
                     vs.clone().map(|vertex| vertices[&vertex].clone())
@@ -75,8 +73,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) {
     }
     for cycle_orig in orig.topology().cycles() {
         let cycle = target
-            .topology()
+            .insert(Cycle {
-            .add_cycle(Cycle {
                 edges: cycle_orig
                     .get()
                     .edges
@@ -97,8 +94,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) {
                 color,
             } => {
                 target
-                    .topology()
+                    .insert(Face::Face {
-                    .add_face(Face::Face {
                         surface: surfaces[&surface].clone(),
                         exteriors: exteriors
                             .iter()
@@ -113,7 +109,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) {
                     .unwrap();
             }
             face @ Face::Triangles(_) => {
-                target.topology().add_face(face.clone()).unwrap();
+                target.insert(face.clone()).unwrap();
             }
         }
     }

fj-operations/src/sketch.rs

@@ -14,8 +14,8 @@ impl ToShape for fj::Sketch {
         let mut vertices = Vec::new();
 
         for [x, y] in self.to_points() {
-            let point = shape.geometry().add_point(Point::from([x, y, 0.]));
+            let point = shape.insert(Point::from([x, y, 0.])).unwrap();
-            let vertex = shape.topology().add_vertex(Vertex { point }).unwrap();
+            let vertex = shape.insert(Vertex { point }).unwrap();
             vertices.push(vertex);
         }
 
@@ -42,17 +42,17 @@ impl ToShape for fj::Sketch {
                 edges.push(edge);
             }
 
-            shape.topology().add_cycle(Cycle { edges }).unwrap();
+            shape.insert(Cycle { edges }).unwrap();
         };
 
-        let surface = shape.geometry().add_surface(Surface::x_y_plane());
+        let surface = shape.insert(Surface::x_y_plane()).unwrap();
         let face = Face::Face {
             exteriors: shape.topology().cycles().collect(),
             interiors: Vec::new(),
             surface,
             color: self.color(),
         };
-        shape.topology().add_face(face).unwrap();
+        shape.insert(face).unwrap();
 
         shape
     }