Merge pull request #401 from hannobraun/face

Distinguish between exterior and interior cycles

fj-kernel/src/algorithms/approximation.rs

@@ -45,7 +45,7 @@ impl Approximation {
         let mut points = HashSet::new();
         let mut segments = HashSet::new();
 
-        for cycle in face.cycles() {
+        for cycle in face.all_cycles() {
             let cycle_points = approximate_cycle(&cycle, tolerance);
 
             let mut cycle_segments = Vec::new();
@@ -196,7 +196,8 @@ mod tests {
         let surface = shape.geometry().add_surface(Surface::x_y_plane());
         let face = Face::Face {
             surface,
-            cycles: vec![abcd],
+            exteriors: vec![abcd],
+            interiors: Vec::new(),
             color: [255, 0, 0, 255],
         };
 

fj-kernel/src/algorithms/sweep.rs

@@ -109,22 +109,28 @@ pub fn sweep_shape(
             .geometry()
             .add_surface(surface_bottom.get().transform(&translation));
 
-        let cycles_bottom = source_to_bottom.cycles_for_face(&face_source);
-        let cycles_top = source_to_top.cycles_for_face(&face_source);
+        let exteriors_bottom =
+            source_to_bottom.exteriors_for_face(&face_source);
+        let interiors_bottom =
+            source_to_bottom.interiors_for_face(&face_source);
+        let exteriors_top = source_to_top.exteriors_for_face(&face_source);
+        let interiors_top = source_to_top.interiors_for_face(&face_source);
 
         target
             .topology()
             .add_face(Face::Face {
                 surface: surface_bottom,
-                cycles: cycles_bottom,
+                exteriors: exteriors_bottom,
+                interiors: interiors_bottom,
                 color,
             })
             .unwrap();
         target
             .topology()
             .add_face(Face::Face {
                 surface: surface_top,
-                cycles: cycles_top,
+                exteriors: exteriors_top,
+                interiors: interiors_top,
                 color,
             })
             .unwrap();
@@ -256,7 +262,8 @@ pub fn sweep_shape(
                     .topology()
                     .add_face(Face::Face {
                         surface,
-                        cycles: vec![cycle],
+                        exteriors: vec![cycle],
+                        interiors: Vec::new(),
                         color,
                     })
                     .unwrap();
@@ -300,17 +307,33 @@ impl Relation {
             .collect()
     }
 
-    fn cycles_for_face(&self, face: &Face) -> Vec<Handle<Cycle>> {
-        let cycles = match face {
-            Face::Face { cycles, .. } => cycles,
+    fn exteriors_for_face(&self, face: &Face) -> Vec<Handle<Cycle>> {
+        let exteriors = match face {
+            Face::Face { exteriors, .. } => exteriors,
             _ => {
                 // Sketches are created using boundary representation, so this
                 // case can't happen.
                 unreachable!()
             }
         };
 
-        cycles
+        exteriors
+            .iter()
+            .map(|cycle| self.cycles.get(cycle).unwrap().clone())
+            .collect()
+    }
+
+    fn interiors_for_face(&self, face: &Face) -> Vec<Handle<Cycle>> {
+        let interiors = match face {
+            Face::Face { interiors, .. } => interiors,
+            _ => {
+                // Sketches are created using boundary representation, so this
+                // case can't happen.
+                unreachable!()
+            }
+        };
+
+        interiors
             .iter()
             .map(|cycle| self.cycles.get(cycle).unwrap().clone())
             .collect()
@@ -412,7 +435,8 @@ mod tests {
             ));
             let abc = Face::Face {
                 surface,
-                cycles: vec![cycles],
+                exteriors: vec![cycles],
+                interiors: Vec::new(),
                 color: [255, 0, 0, 255],
             };
 

fj-kernel/src/shape/topology.rs

@@ -179,7 +179,10 @@ impl Topology<'_> {
     /// not the case.
     pub fn add_face(&mut self, face: Face) -> ValidationResult<Face> {
         if let Face::Face {
-            surface, cycles, ..
+            surface,
+            exteriors,
+            interiors,
+            ..
         } = &face
         {
             let mut missing_surface = None;
@@ -188,7 +191,7 @@ impl Topology<'_> {
             if !self.geometry.surfaces.contains(surface.storage()) {
                 missing_surface = Some(surface.clone());
             }
-            for cycle in cycles {
+            for cycle in exteriors.iter().chain(interiors) {
                 if !self.cycles.contains(cycle.storage()) {
                     missing_cycles.insert(cycle.clone());
                 }
@@ -349,7 +352,8 @@ mod tests {
             .topology()
             .add_face(Face::Face {
                 surface: surface.clone(),
-                cycles: vec![cycle.clone()],
+                exteriors: vec![cycle.clone()],
+                interiors: Vec::new(),
                 color: [255, 0, 0, 255],
             })
             .unwrap_err();
@@ -362,7 +366,8 @@ mod tests {
         // Everything has been added to `shape` now. Should work!
         shape.topology().add_face(Face::Face {
             surface,
-            cycles: vec![cycle],
+            exteriors: vec![cycle],
+            interiors: Vec::new(),
             color: [255, 0, 0, 255],
         })?;
 

fj-kernel/src/topology/faces.rs

@@ -22,7 +22,7 @@ pub enum Face {
         /// The surface that defines this face
         surface: Handle<Surface>,
 
-        /// The cycles that bound the face
+        /// The cycles that bound the face on the outside
         ///
         /// # Implementation Note
         ///
@@ -32,7 +32,16 @@ pub enum Face {
         ///
         /// It might be less error-prone to specify the edges in surface
         /// coordinates.
-        cycles: Vec<Handle<Cycle>>,
+        exteriors: Vec<Handle<Cycle>>,
+
+        /// The cycles that bound the face on the inside
+        ///
+        /// Each of these cycles defines a hole in the face.
+        ///
+        /// # Implementation note
+        ///
+        /// See note on `exterior` field.
+        interiors: Vec<Handle<Cycle>>,
 
         /// The color of the face
         color: [u8; 4],
@@ -63,14 +72,31 @@ impl Face {
         }
     }
 
-    /// Access the cycles that the face refers to
+    /// Access the exterior cycles that the face refers to
+    ///
+    /// This is a convenience method that saves the caller from dealing with the
+    /// [`Handle`]s.
+    pub fn exteriors(&self) -> impl Iterator<Item = Cycle> + '_ {
+        match self {
+            Self::Face { exteriors, .. } => {
+                exteriors.iter().map(|handle| handle.get().clone())
+            }
+            _ => {
+                // No code that still uses triangle representation is calling
+                // this method.
+                unreachable!()
+            }
+        }
+    }
+
+    /// Access the interior cycles that the face refers to
     ///
     /// This is a convenience method that saves the caller from dealing with the
     /// [`Handle`]s.
-    pub fn cycles(&self) -> impl Iterator<Item = Cycle> + '_ {
+    pub fn interiors(&self) -> impl Iterator<Item = Cycle> + '_ {
         match self {
-            Self::Face { cycles, .. } => {
-                cycles.iter().map(|handle| handle.get().clone())
+            Self::Face { interiors, .. } => {
+                interiors.iter().map(|handle| handle.get().clone())
             }
             _ => {
                 // No code that still uses triangle representation is calling
@@ -79,18 +105,28 @@ impl Face {
             }
         }
     }
+
+    /// Access all cycles that the face refers to
+    ///
+    /// This is equivalent to chaining the iterators returned by
+    /// [`Face::exteriors`] and [`Face::interiors`].
+    pub fn all_cycles(&self) -> impl Iterator<Item = Cycle> + '_ {
+        self.exteriors().chain(self.interiors())
+    }
 }
 
 impl PartialEq for Face {
     fn eq(&self, other: &Self) -> bool {
-        self.surface() == other.surface() && self.cycles().eq(other.cycles())
+        self.surface() == other.surface()
+            && self.exteriors().eq(other.exteriors())
+            && self.interiors().eq(other.interiors())
     }
 }
 
 impl Hash for Face {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.surface().hash(state);
-        for cycle in self.cycles() {
+        for cycle in self.all_cycles() {
             cycle.hash(state);
         }
     }

fj-operations/src/circle.rs

@@ -29,7 +29,8 @@ impl ToShape for fj::Circle {
         shape
             .topology()
             .add_face(Face::Face {
-                cycles,
+                exteriors: cycles,
+                interiors: Vec::new(),
                 surface,
                 color: self.color(),
             })

fj-operations/src/difference_2d.rs

@@ -2,7 +2,7 @@ use std::collections::HashMap;
 
 use fj_debug::DebugInfo;
 use fj_kernel::{
-    shape::Shape,
+    shape::{Handle, Shape},
     topology::{Cycle, Edge, Face, Vertex},
 };
 use fj_math::{Aabb, Scalar};
@@ -19,18 +19,18 @@ impl ToShape for fj::Difference2d {
         let [mut a, mut b] = [&self.a(), &self.b()]
             .map(|shape| shape.to_shape(tolerance, debug_info));
 
+        // Check preconditions.
+        //
+        // See issue:
+        // https://github.com/hannobraun/Fornjot/issues/95
         for shape in [&mut a, &mut b] {
             if shape.topology().cycles().count() != 1 {
-                // See issue:
-                // https://github.com/hannobraun/Fornjot/issues/95
                 todo!(
                     "The 2-dimensional difference operation only supports one \
                     cycle in each operand."
                 );
             }
             if shape.topology().faces().count() != 1 {
-                // See issue:
-                // https://github.com/hannobraun/Fornjot/issues/95
                 todo!(
                     "The 2-dimensional difference operation only supports one \
                     face in each operand."
@@ -39,45 +39,15 @@ impl ToShape for fj::Difference2d {
         }
 
         // Can't panic, as we just verified that both shapes have one cycle.
-        let cycles_orig = [&mut a, &mut b]
+        let [cycle_a, cycle_b] = [&mut a, &mut b]
             .map(|shape| shape.topology().cycles().next().unwrap());
 
         let mut vertices = HashMap::new();
-        let mut cycles = Vec::new();
-
-        for cycle in cycles_orig {
-            let mut edges = Vec::new();
-            for edge in &cycle.get().edges {
-                let edge = edge.get();
-
-                let curve = shape.geometry().add_curve(edge.curve());
-
-                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());
-                                shape
-                                    .topology()
-                                    .add_vertex(Vertex { point })
-                                    .unwrap()
-                            })
-                            .clone()
-                    })
-                });
-
-                let edge = shape
-                    .topology()
-                    .add_edge(Edge { curve, vertices })
-                    .unwrap();
-                edges.push(edge);
-            }
+        let mut exteriors = Vec::new();
+        let mut interiors = Vec::new();
 
-            let cycle = shape.topology().add_cycle(Cycle { edges }).unwrap();
-            cycles.push(cycle);
-        }
+        exteriors.push(add_cycle(cycle_a, &mut vertices, &mut shape));
+        interiors.push(add_cycle(cycle_b, &mut vertices, &mut shape));
 
         // Can't panic, as we just verified that both shapes have one face.
         let [face_a, face_b] = [&mut a, &mut b]
@@ -92,8 +62,9 @@ impl ToShape for fj::Difference2d {
         shape
             .topology()
             .add_face(Face::Face {
-                cycles,
                 surface,
+                exteriors,
+                interiors,
                 color: self.color(),
             })
             .unwrap();
@@ -108,3 +79,31 @@ impl ToShape for fj::Difference2d {
         self.a().bounding_volume()
     }
 }
+
+fn add_cycle(
+    cycle: Handle<Cycle>,
+    vertices: &mut HashMap<Vertex, Handle<Vertex>>,
+    shape: &mut Shape,
+) -> Handle<Cycle> {
+    let mut edges = Vec::new();
+    for edge in cycle.get().edges() {
+        let curve = shape.geometry().add_curve(edge.curve());
+
+        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());
+                        shape.topology().add_vertex(Vertex { point }).unwrap()
+                    })
+                    .clone()
+            })
+        });
+
+        let edge = shape.topology().add_edge(Edge { curve, vertices }).unwrap();
+        edges.push(edge);
+    }
+
+    shape.topology().add_cycle(Cycle { edges }).unwrap()
+}