Merge pull request #317 from hannobraun/diff

Generate more correct shapes as result of 2D difference

src/kernel/shape/cycles.rs

@@ -2,25 +2,36 @@ use crate::kernel::topology::edges::Cycle;
 
 use super::{
     handle::{Handle, Storage},
-    CyclesInner,
+    CyclesInner, EdgesInner,
 };
 
 /// The cycles of a shape
 pub struct Cycles<'r> {
+    pub(super) edges: &'r mut EdgesInner,
     pub(super) cycles: &'r mut CyclesInner,
 }
 
 impl Cycles<'_> {
     /// Add a cycle to the shape
     ///
+    /// # Panics
+    ///
+    /// Panics, if the edges of the cycles are not part of this shape.
+    ///
     /// # Implementation note
     ///
-    /// This method should at some point validate the cycle:
-    /// - That it refers to valid edges that are part of `Shape`.
+    /// 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(&mut self, cycle: Cycle) -> Handle<Cycle> {
+        for edge in &cycle.edges {
+            assert!(
+                self.edges.contains(edge.storage()),
+                "Cycle validation failed: {edge:?} is not part of the shape",
+            );
+        }
+
         let storage = Storage::new(cycle);
         let handle = storage.handle();
         self.cycles.push(storage);
@@ -33,3 +44,37 @@ impl Cycles<'_> {
         self.cycles.iter().map(|storage| storage.handle())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::{
+        kernel::{shape::Shape, topology::edges::Cycle},
+        math::Point,
+    };
+
+    #[test]
+    fn add_valid() {
+        let mut shape = Shape::new();
+
+        let a = shape.vertices().add(Point::from([0., 0., 0.]));
+        let b = shape.vertices().add(Point::from([1., 0., 0.]));
+
+        let edge = shape.edges().add_line_segment([a, b]);
+
+        shape.cycles().add(Cycle { edges: vec![edge] });
+    }
+
+    #[test]
+    #[should_panic]
+    fn add_invalid() {
+        let mut shape = Shape::new();
+        let mut other = Shape::new();
+
+        let a = other.vertices().add(Point::from([0., 0., 0.]));
+        let b = other.vertices().add(Point::from([1., 0., 0.]));
+
+        let edge = other.edges().add_line_segment([a, b]);
+
+        shape.cycles().add(Cycle { edges: vec![edge] });
+    }
+}

src/kernel/shape/edges.rs

@@ -9,14 +9,17 @@ use crate::{
 use super::{
     curves::Curves,
     handle::{Handle, Storage},
+    EdgesInner, VerticesInner,
 };
 
 /// The edges of a shape
-pub struct Edges {
+pub struct Edges<'r> {
     pub(super) curves: Curves,
+    pub(super) vertices: &'r mut VerticesInner,
+    pub(super) edges: &'r mut EdgesInner,
 }
 
-impl Edges {
+impl Edges<'_> {
     /// Add an edge to the shape
     ///
     /// If vertices are provided in `vertices`, they must be on `curve`.
@@ -32,7 +35,21 @@ impl Edges {
     /// the future, it can add the edge to the proper internal data structures,
     /// and validate any constraints that apply to edge creation.
     pub fn add(&mut self, edge: Edge) -> Handle<Edge> {
-        Storage::new(edge).handle()
+        for vertices in &edge.vertices {
+            for vertex in vertices {
+                assert!(
+                    self.vertices.contains(vertex.storage()),
+                    "Edge validation failed: {vertex:?} is not part of shape",
+                );
+            }
+        }
+
+        let storage = Storage::new(edge);
+        let handle = storage.handle();
+
+        self.edges.push(storage);
+
+        handle
     }
 
     /// Add a circle to the shape
@@ -67,3 +84,30 @@ impl Edges {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::{kernel::shape::Shape, math::Point};
+
+    #[test]
+    fn add_valid() {
+        let mut shape = Shape::new();
+
+        let a = shape.vertices().add(Point::from([0., 0., 0.]));
+        let b = shape.vertices().add(Point::from([1., 0., 0.]));
+
+        shape.edges().add_line_segment([a, b]);
+    }
+
+    #[test]
+    #[should_panic]
+    fn add_invalid() {
+        let mut shape = Shape::new();
+        let mut other = Shape::new();
+
+        let a = other.vertices().add(Point::from([0., 0., 0.]));
+        let b = other.vertices().add(Point::from([1., 0., 0.]));
+
+        shape.edges().add_line_segment([a, b]);
+    }
+}

src/kernel/shape/handle.rs

@@ -28,6 +28,11 @@ impl<T> Handle<T> {
     pub fn get(&self) -> &T {
         self.0.deref()
     }
+
+    /// Internal method to access the [`Storage`] this handle refers to
+    pub(super) fn storage(&self) -> &Storage<T> {
+        &self.0
+    }
 }
 
 impl<T> Deref for Handle<T> {

src/kernel/shape/mod.rs

@@ -8,7 +8,11 @@ pub mod vertices;
 
 use crate::math::Scalar;
 
-use super::topology::{edges::Cycle, faces::Face, vertices::Vertex};
+use super::topology::{
+    edges::{Cycle, Edge},
+    faces::Face,
+    vertices::Vertex,
+};
 
 use self::{
     curves::Curves, cycles::Cycles, edges::Edges, faces::Faces,
@@ -24,6 +28,7 @@ pub struct Shape {
     min_distance: Scalar,
 
     vertices: VerticesInner,
+    edges: EdgesInner,
     cycles: CyclesInner,
     faces: FacesInner,
 }
@@ -38,6 +43,7 @@ impl Shape {
             min_distance: Scalar::from_f64(5e-7), // 0.5 µm
 
             vertices: VerticesInner::new(),
+            edges: EdgesInner::new(),
             cycles: CyclesInner::new(),
             faces: FacesInner::new(),
         }
@@ -78,12 +84,17 @@ impl Shape {
 
     /// Access the shape's edges
     pub fn edges(&mut self) -> Edges {
-        Edges { curves: Curves }
+        Edges {
+            curves: Curves,
+            vertices: &mut self.vertices,
+            edges: &mut self.edges,
+        }
     }
 
     /// Access the shape's cycles
     pub fn cycles(&mut self) -> Cycles {
         Cycles {
+            edges: &mut self.edges,
             cycles: &mut self.cycles,
         }
     }
@@ -97,5 +108,6 @@ impl Shape {
 }
 
 type VerticesInner = Vec<Storage<Vertex>>;
+type EdgesInner = Vec<Storage<Edge>>;
 type CyclesInner = Vec<Storage<Cycle>>;
 type FacesInner = Vec<Storage<Face>>;

src/kernel/shape/vertices.rs

@@ -66,7 +66,7 @@ mod tests {
     const MIN_DISTANCE: f64 = 5e-7;
 
     #[test]
-    fn create_valid() {
+    fn add_valid() {
         let mut shape = Shape::new().with_min_distance(MIN_DISTANCE);
 
         shape.vertices().add(Point::from([0., 0., 0.]));
@@ -76,7 +76,7 @@ mod tests {
     #[test]
     #[ignore]
     #[should_panic]
-    fn create_invalid() {
+    fn add_invalid() {
         // Test is ignored, until vertex validation can be enabled for real.
         // See implementation note on `Vertices::create`.
 

src/kernel/shapes/difference_2d.rs

@@ -1,8 +1,13 @@
+use std::collections::HashMap;
+
 use crate::{
     debug::DebugInfo,
     kernel::{
         shape::Shape,
-        topology::{edges::Cycle, faces::Face},
+        topology::{
+            edges::{Cycle, Edge},
+            faces::Face,
+        },
     },
     math::{Aabb, Scalar},
 };
@@ -42,10 +47,27 @@ impl ToShape for fj::Difference2d {
         let cycles =
             [&mut a, &mut b].map(|shape| shape.cycles().all().next().unwrap());
 
+        let mut vertices = HashMap::new();
         for cycle in cycles {
-            shape.cycles().add(Cycle {
-                edges: cycle.edges.clone(),
-            });
+            let mut edges = Vec::new();
+            for edge in &cycle.edges {
+                let curve = shape.curves().add(*edge.curve.get());
+
+                let vertices = edge.vertices.clone().map(|vs| {
+                    vs.map(|vertex| {
+                        let vertex = *vertex.get();
+                        vertices
+                            .entry(vertex)
+                            .or_insert_with(|| shape.vertices().add(vertex))
+                            .clone()
+                    })
+                });
+
+                let edge = shape.edges().add(Edge { curve, vertices });
+                edges.push(edge);
+            }
+
+            shape.cycles().add(Cycle { edges });
         }
 
         // Can't panic, as we just verified that both shapes have one face.