Merge pull request #597 from hannobraun/face

Make working with b-rep face easier

crates/fj-kernel/src/algorithms/sweep.rs

@@ -256,32 +256,16 @@ impl Relation {
     }
 
     fn exteriors_for_face(&self, face: &Face) -> Vec<Handle<Cycle<3>>> {
-        let exteriors = match face {
-            Face::Face { exteriors, .. } => exteriors,
-            _ => {
-                // Sketches are created using boundary representation, so this
-                // case can't happen.
-                unreachable!()
-            }
-        };
-
-        exteriors
+        face.brep()
+            .exteriors
             .as_handle()
             .map(|cycle| self.cycles.get(cycle).unwrap().clone())
             .collect()
     }
 
     fn interiors_for_face(&self, face: &Face) -> Vec<Handle<Cycle<3>>> {
-        let interiors = match face {
-            Face::Face { interiors, .. } => interiors,
-            _ => {
-                // Sketches are created using boundary representation, so this
-                // case can't happen.
-                unreachable!()
-            }
-        };
-
-        interiors
+        face.brep()
+            .interiors
             .as_handle()
             .map(|cycle| self.cycles.get(cycle).unwrap().clone())
             .collect()

crates/fj-kernel/src/algorithms/triangulation/mod.rs

@@ -22,8 +22,8 @@ pub fn triangulate(
     for face in shape.faces() {
         let face = face.get();
         match &face {
-            Face::Face { surface, color, .. } => {
-                let surface = surface.get();
+            Face::Face(brep) => {
+                let surface = brep.surface.get();
                 let approx = FaceApprox::new(&face, tolerance);
 
                 let points: Vec<_> = approx
@@ -68,7 +68,7 @@ pub fn triangulate(
 
                 for triangle in triangles {
                     let points = triangle.map(|point| point.canonical());
-                    mesh.push_triangle(points, *color);
+                    mesh.push_triangle(points, brep.color);
                 }
             }
             Face::Triangles(triangles) => {

crates/fj-kernel/src/shape/object.rs

@@ -84,28 +84,24 @@ impl Object for Cycle<3> {
 impl Object for Face {
     fn merge_into(self, shape: &mut Shape) -> ValidationResult<Self> {
         match self {
-            Face::Face {
-                surface,
-                exteriors,
-                interiors,
-                color,
-            } => {
-                let surface = surface.get().merge_into(shape)?;
+            Face::Face(face) => {
+                let surface = face.surface.get().merge_into(shape)?;
 
                 let mut exts = Vec::new();
-                for cycle in exteriors.as_canonical() {
+                for cycle in face.exteriors.as_canonical() {
                     let cycle = cycle.merge_into(shape)?;
                     exts.push(cycle);
                 }
 
                 let mut ints = Vec::new();
-                for cycle in interiors.as_canonical() {
+                for cycle in face.interiors.as_canonical() {
                     let cycle = cycle.merge_into(shape)?;
                     ints.push(cycle);
                 }
 
-                shape
-                    .get_handle_or_insert(Face::new(surface, exts, ints, color))
+                shape.get_handle_or_insert(Face::new(
+                    surface, exts, ints, face.color,
+                ))
             }
             Face::Triangles(_) => shape.get_handle_or_insert(self),
         }

crates/fj-kernel/src/shape/validate.rs

@@ -136,20 +136,16 @@ impl Validate for Face {
         _: Scalar,
         stores: &Stores,
     ) -> Result<(), ValidationError> {
-        if let Face::Face {
-            surface,
-            exteriors,
-            interiors,
-            ..
-        } = self
-        {
+        if let Face::Face(face) = self {
             let mut missing_surface = None;
             let mut missing_cycles = HashSet::new();
 
-            if !stores.surfaces.contains(surface) {
-                missing_surface = Some(surface.clone());
+            if !stores.surfaces.contains(&face.surface) {
+                missing_surface = Some(face.surface.clone());
             }
-            for cycle in exteriors.as_handle().chain(interiors.as_handle()) {
+            for cycle in
+                face.exteriors.as_handle().chain(face.interiors.as_handle())
+            {
                 if !stores.cycles.contains(cycle) {
                     missing_cycles.insert(cycle.clone());
                 }

crates/fj-kernel/src/topology/face.rs

@@ -21,40 +21,13 @@ use super::{Cycle, FaceBuilder};
 ///
 /// 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)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub enum Face {
     /// A face of a shape
     ///
     /// A face is defined by a surface, and is bounded by edges that lie in that
     /// surface.
-    Face {
-        /// The surface that defines this face
-        surface: Handle<Surface>,
-
-        /// The cycles that bound the face on the outside
-        ///
-        /// # Implementation Note
-        ///
-        /// Since these cycles bound the face, the edges they consist of must
-        /// lie in the surface. The data we're using here is 3-dimensional
-        /// though, so no such limitation is enforced.
-        ///
-        /// It might be less error-prone to specify the cycles in surface
-        /// coordinates.
-        exteriors: CyclesInFace,
-
-        /// 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: CyclesInFace,
-
-        /// The color of the face
-        color: [u8; 4],
-    },
+    Face(FaceBRep),
 
     /// The triangles of the face
     ///
@@ -76,25 +49,22 @@ impl Face {
         let exteriors = CyclesInFace::from_canonical(exteriors);
         let interiors = CyclesInFace::from_canonical(interiors);
 
-        Self::Face {
+        Self::Face(FaceBRep {
             surface,
             exteriors,
             interiors,
             color,
-        }
+        })
     }
     /// Build a face using the [`FaceBuilder`] API
     pub fn builder(surface: Surface, shape: &mut Shape) -> FaceBuilder {
         FaceBuilder::new(surface, shape)
     }
 
-    /// Access the surface that the face refers to
-    ///
-    /// This is a convenience method that saves the caller from dealing with the
-    /// [`Handle`].
-    pub fn surface(&self) -> Surface {
+    /// Access the boundary representation of the face
+    pub fn brep(&self) -> &FaceBRep {
         match self {
-            Self::Face { surface, .. } => surface.get(),
+            Self::Face(face) => face,
             _ => {
                 // No code that still uses triangle representation is calling
                 // this method.
@@ -103,34 +73,97 @@ impl Face {
         }
     }
 
+    /// Access the surface that the face refers to
+    ///
+    /// This is a convenience method that saves the caller from dealing with the
+    /// [`Handle`].
+    pub fn surface(&self) -> Surface {
+        self.brep().surface()
+    }
+
     /// 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<3>> + '_ {
-        match self {
-            Self::Face { exteriors, .. } => exteriors.as_canonical(),
-            _ => {
-                // No code that still uses triangle representation is calling
-                // this method.
-                unreachable!()
-            }
-        }
+        self.brep().exteriors()
     }
 
     /// 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 interiors(&self) -> impl Iterator<Item = Cycle<3>> + '_ {
-        match self {
-            Self::Face { interiors, .. } => interiors.as_canonical(),
-            _ => {
-                // No code that still uses triangle representation is calling
-                // this method.
-                unreachable!()
-            }
-        }
+        self.brep().interiors()
+    }
+
+    /// 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<3>> + '_ {
+        self.exteriors().chain(self.interiors())
+    }
+}
+
+/// The boundary representation of a face
+///
+/// This type exists to ease the handling of faces that use boundary
+/// representation. It will eventually be merged into `Face`, once
+/// `Face::Triangles` can finally be removed.
+#[derive(Clone, Debug, Eq, Ord, PartialOrd)]
+pub struct FaceBRep {
+    /// The surface that defines this face
+    pub surface: Handle<Surface>,
+
+    /// The cycles that bound the face on the outside
+    ///
+    /// # Implementation Note
+    ///
+    /// Since these cycles bound the face, the edges they consist of must
+    /// lie in the surface. The data we're using here is 3-dimensional
+    /// though, so no such limitation is enforced.
+    ///
+    /// It might be less error-prone to specify the cycles in surface
+    /// coordinates.
+    pub exteriors: CyclesInFace,
+
+    /// 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.
+    pub interiors: CyclesInFace,
+
+    /// The color of the face
+    pub color: [u8; 4],
+}
+
+impl FaceBRep {
+    /// Access the surface that the face refers to
+    ///
+    /// This is a convenience method that saves the caller from dealing with the
+    /// [`Handle`].
+    pub fn surface(&self) -> Surface {
+        self.surface.get()
+    }
+
+    /// 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<3>> + '_ {
+        self.exteriors.as_canonical()
+    }
+
+    /// 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 interiors(&self) -> impl Iterator<Item = Cycle<3>> + '_ {
+        self.interiors.as_canonical()
     }
 
     /// Access all cycles that the face refers to
@@ -142,15 +175,15 @@ impl Face {
     }
 }
 
-impl PartialEq for Face {
+impl PartialEq for FaceBRep {
     fn eq(&self, other: &Self) -> bool {
         self.surface() == other.surface()
             && self.exteriors().eq(other.exteriors())
             && self.interiors().eq(other.interiors())
     }
 }
 
-impl Hash for Face {
+impl Hash for FaceBRep {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.surface().hash(state);
         for cycle in self.all_cycles() {