Merge pull request #2277 from hannobraun/validation

Port face boundary check to new validation infrastructure

Author: hannobraun

crates/fj-core/src/validate/face.rs

@@ -3,30 +3,31 @@ use fj_math::Winding;
 use crate::{
     geometry::Geometry,
     objects::Face,
-    validation::{ValidationConfig, ValidationError},
+    validation::{
+        checks::FaceHasNoBoundary, ValidationCheck, ValidationConfig,
+        ValidationError,
+    },
 };
 
 use super::Validate;
 
 impl Validate for Face {
     fn validate(
         &self,
-        _: &ValidationConfig,
+        config: &ValidationConfig,
         errors: &mut Vec<ValidationError>,
         geometry: &Geometry,
     ) {
-        FaceValidationError::check_boundary(self, errors);
+        errors.extend(
+            FaceHasNoBoundary::check(self, geometry, config).map(Into::into),
+        );
         FaceValidationError::check_interior_winding(self, geometry, errors);
     }
 }
 
 /// [`Face`] validation error
 #[derive(Clone, Debug, thiserror::Error)]
 pub enum FaceValidationError {
-    /// The [`Face`] has no exterior cycle
-    #[error("The `Face` has no exterior cycle")]
-    MissingBoundary,
-
     /// Interior of [`Face`] has invalid winding; must be opposite of exterior
     #[error(
         "Interior of `Face` has invalid winding; must be opposite of exterior\n\
@@ -47,15 +48,6 @@ pub enum FaceValidationError {
 }
 
 impl FaceValidationError {
-    fn check_boundary(face: &Face, errors: &mut Vec<ValidationError>) {
-        if face.region().exterior().half_edges().is_empty() {
-            errors.push(ValidationError::from(Self::MissingBoundary));
-        }
-
-        // Checking *that* a boundary exists is enough. There are validation
-        // checks for `Cycle` to make sure that the cycle is closed properly.
-    }
-
     fn check_interior_winding(
         face: &Face,
         geometry: &Geometry,
@@ -95,50 +87,19 @@ impl FaceValidationError {
 mod tests {
     use crate::{
         assert_contains_err,
-        objects::{Cycle, Face, HalfEdge, Region},
+        objects::{Cycle, Face, Region},
         operations::{
-            build::{BuildCycle, BuildFace, BuildHalfEdge},
+            build::{BuildCycle, BuildFace},
             derive::DeriveFrom,
             insert::Insert,
             reverse::Reverse,
-            update::{UpdateCycle, UpdateFace, UpdateRegion},
+            update::{UpdateFace, UpdateRegion},
         },
         validate::{FaceValidationError, Validate},
         validation::ValidationError,
         Core,
     };
 
-    #[test]
-    fn boundary() -> anyhow::Result<()> {
-        let mut core = Core::new();
-
-        let invalid =
-            Face::unbound(core.layers.objects.surfaces.xy_plane(), &mut core);
-        let valid = invalid.update_region(
-            |region, core| {
-                region.update_exterior(
-                    |cycle, core| {
-                        cycle.add_half_edges(
-                            [HalfEdge::circle([0., 0.], 1., core)],
-                            core,
-                        )
-                    },
-                    core,
-                )
-            },
-            &mut core,
-        );
-
-        valid.validate_and_return_first_error(&core.layers.geometry)?;
-        assert_contains_err!(
-            core,
-            invalid,
-            ValidationError::Face(FaceValidationError::MissingBoundary)
-        );
-
-        Ok(())
-    }
-
     #[test]
     fn interior_winding() -> anyhow::Result<()> {
         let mut core = Core::new();

crates/fj-core/src/validation/checks/face_boundary.rs

@@ -0,0 +1,73 @@
+use crate::{
+    geometry::Geometry,
+    objects::Face,
+    validation::{ValidationCheck, ValidationConfig},
+};
+
+/// [`Face`] has no boundary
+///
+/// A face must have a boundary, meaning its exterior cycle must not be empty.
+/// Checking *that* the exterior cycle is not empty is enough, as
+/// [`AdjacentHalfEdgesNotConnected`] makes sure that any cycle that is not
+/// empty, is closed.
+///
+/// [`AdjacentHalfEdgesNotConnected`]: super::AdjacentHalfEdgesNotConnected
+#[derive(Clone, Debug, thiserror::Error)]
+#[error("`Face` has no boundary")]
+pub struct FaceHasNoBoundary {}
+
+impl ValidationCheck<Face> for FaceHasNoBoundary {
+    fn check(
+        object: &Face,
+        _: &Geometry,
+        _: &ValidationConfig,
+    ) -> impl Iterator<Item = Self> {
+        let error = if object.region().exterior().half_edges().is_empty() {
+            Some(FaceHasNoBoundary {})
+        } else {
+            None
+        };
+
+        error.into_iter()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::{
+        objects::{Cycle, Face},
+        operations::{
+            build::{BuildCycle, BuildFace},
+            update::{UpdateFace, UpdateRegion},
+        },
+        validation::{checks::FaceHasNoBoundary, ValidationCheck},
+        Core,
+    };
+
+    #[test]
+    fn face_has_no_boundary() -> anyhow::Result<()> {
+        let mut core = Core::new();
+
+        let valid = Face::circle(
+            core.layers.objects.surfaces.xy_plane(),
+            [0., 0.],
+            1.,
+            &mut core,
+        );
+        FaceHasNoBoundary::check_and_return_first_error(
+            &valid,
+            &core.layers.geometry,
+        )?;
+
+        let invalid = valid.update_region(
+            |region, core| region.update_exterior(|_, _| Cycle::empty(), core),
+            &mut core,
+        );
+        FaceHasNoBoundary::check_and_expect_one_error(
+            &invalid,
+            &core.layers.geometry,
+        );
+
+        Ok(())
+    }
+}

crates/fj-core/src/validation/checks/half_edge_connection.rs

@@ -88,7 +88,7 @@ mod tests {
     use super::AdjacentHalfEdgesNotConnected;
 
     #[test]
-    fn adjacent_half_edges_connected() -> anyhow::Result<()> {
+    fn adjacent_half_edges_not_connected() -> anyhow::Result<()> {
         let mut core = Core::new();
 
         let valid = Cycle::polygon([[0., 0.], [1., 0.], [1., 1.]], &mut core);

crates/fj-core/src/validation/checks/mod.rs

@@ -2,6 +2,10 @@
 //!
 //! See documentation of [parent module](super) for more information.
 
+mod face_boundary;
 mod half_edge_connection;
 
-pub use self::half_edge_connection::AdjacentHalfEdgesNotConnected;
+pub use self::{
+    face_boundary::FaceHasNoBoundary,
+    half_edge_connection::AdjacentHalfEdgesNotConnected,
+};

crates/fj-core/src/validation/error.rs

@@ -5,14 +5,18 @@ use crate::validate::{
     SketchValidationError, SolidValidationError,
 };
 
-use super::checks::AdjacentHalfEdgesNotConnected;
+use super::checks::{AdjacentHalfEdgesNotConnected, FaceHasNoBoundary};
 
 /// An error that can occur during a validation
 #[derive(Clone, Debug, thiserror::Error)]
 pub enum ValidationError {
-    /// `HalfEdge`s in `Cycle` not connected
+    /// Adjacent half-edges are not connected
     #[error(transparent)]
-    HalfEdgesInCycleNotConnected(#[from] AdjacentHalfEdgesNotConnected),
+    AdjacentHalfEdgesNotConnected(#[from] AdjacentHalfEdgesNotConnected),
+
+    /// Face has no boundary
+    #[error(transparent)]
+    FaceHasNoBoundary(#[from] FaceHasNoBoundary),
 
     /// `Edge` validation error
     #[error("`Edge` validation error")]