Merge pull request #2235 from hannobraun/validation

Expand new validation infrastructure; migrate first validation check

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

@@ -1,9 +1,6 @@
-use fj_math::{Point, Scalar};
-
 use crate::{
-    objects::{Cycle, HalfEdge},
-    storage::Handle,
-    validation::{ValidationConfig, ValidationError},
+    objects::Cycle,
+    validation::{ValidationCheck, ValidationConfig, ValidationError},
 };
 
 use super::Validate;
@@ -14,106 +11,6 @@ impl Validate for Cycle {
         config: &ValidationConfig,
         errors: &mut Vec<ValidationError>,
     ) {
-        CycleValidationError::check_half_edge_connections(self, config, errors);
-    }
-}
-
-/// [`Cycle`] validation failed
-#[derive(Clone, Debug, thiserror::Error)]
-pub enum CycleValidationError {
-    /// [`Cycle`]'s edges are not connected
-    #[error(
-        "Adjacent `HalfEdge`s are not connected\n\
-        - End position of first `HalfEdge`: {end_of_first:?}\n\
-        - Start position of second `HalfEdge`: {start_of_second:?}\n\
-        - Distance between vertices: {distance}\n\
-        - `HalfEdge`s: {half_edges:#?}"
-    )]
-    HalfEdgesNotConnected {
-        /// The end position of the first [`HalfEdge`]
-        end_of_first: Point<2>,
-
-        /// The start position of the second [`HalfEdge`]
-        start_of_second: Point<2>,
-
-        /// The distance between the two vertices
-        distance: Scalar,
-
-        /// The edges
-        half_edges: [Handle<HalfEdge>; 2],
-    },
-}
-
-impl CycleValidationError {
-    fn check_half_edge_connections(
-        cycle: &Cycle,
-        config: &ValidationConfig,
-        errors: &mut Vec<ValidationError>,
-    ) {
-        for (first, second) in cycle.half_edges().pairs() {
-            let end_of_first = {
-                let [_, end] = first.boundary().inner;
-                first.path().point_from_path_coords(end)
-            };
-            let start_of_second = second.start_position();
-
-            let distance = (end_of_first - start_of_second).magnitude();
-
-            if distance > config.identical_max_distance {
-                errors.push(
-                    Self::HalfEdgesNotConnected {
-                        end_of_first,
-                        start_of_second,
-                        distance,
-                        half_edges: [first.clone(), second.clone()],
-                    }
-                    .into(),
-                );
-            }
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests {
-
-    use crate::{
-        assert_contains_err,
-        objects::{Cycle, HalfEdge},
-        operations::{
-            build::{BuildCycle, BuildHalfEdge},
-            update::UpdateCycle,
-        },
-        validate::{cycle::CycleValidationError, Validate},
-        validation::ValidationError,
-        Core,
-    };
-
-    #[test]
-    fn edges_connected() -> anyhow::Result<()> {
-        let mut core = Core::new();
-
-        let valid =
-            Cycle::polygon([[0.0, 0.0], [1.0, 0.0], [1.0, 1.0]], &mut core);
-
-        valid.validate_and_return_first_error()?;
-
-        let disconnected = {
-            let edges = [
-                HalfEdge::line_segment([[0., 0.], [1., 0.]], None, &mut core),
-                HalfEdge::line_segment([[0., 0.], [1., 0.]], None, &mut core),
-            ];
-
-            Cycle::empty().add_half_edges(edges, &mut core)
-        };
-
-        assert_contains_err!(
-            disconnected,
-            ValidationError::Cycle(
-                CycleValidationError::HalfEdgesNotConnected { .. }
-            )
-        );
-
-        Ok(())
+        errors.extend(self.check(config).map(Into::into));
     }
 }

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

@@ -76,9 +76,9 @@ mod vertex;
 use crate::validation::{ValidationConfig, ValidationError};
 
 pub use self::{
-    cycle::CycleValidationError, edge::EdgeValidationError,
-    face::FaceValidationError, shell::ShellValidationError,
-    sketch::SketchValidationError, solid::SolidValidationError,
+    edge::EdgeValidationError, face::FaceValidationError,
+    shell::ShellValidationError, sketch::SketchValidationError,
+    solid::SolidValidationError,
 };
 
 /// Assert that some object has a validation error which matches a specific

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

@@ -0,0 +1,101 @@
+use fj_math::{Point, Scalar};
+
+use crate::{
+    objects::{Cycle, HalfEdge},
+    storage::Handle,
+    validation::{validation_check::ValidationCheck, ValidationConfig},
+};
+
+/// Adjacent [`HalfEdge`]s in [`Cycle`] are not connected
+///
+/// Each [`HalfEdge`] only references its start vertex. The end vertex is always
+/// assumed to be the start vertex of the next [`HalfEdge`] in the cycle. This
+/// part of the definition carries no redundancy, and thus doesn't need to be
+/// subject to a validation check.
+///
+/// However, the *position* of that shared vertex is redundantly defined in both
+/// [`HalfEdge`]s. This check verifies that both positions are the same.
+#[derive(Clone, Debug, thiserror::Error)]
+#[error(
+    "Adjacent `HalfEdge`s in `Cycle` are not connected\n\
+    - End position of first `HalfEdge`: {end_pos_of_first_half_edge:?}\n\
+    - Start position of second `HalfEdge`: {start_pos_of_second_half_edge:?}\n\
+    - Distance between vertices: {distance_between_positions}\n\
+    - The unconnected `HalfEdge`s: {unconnected_half_edges:#?}"
+)]
+pub struct AdjacentHalfEdgesNotConnected {
+    /// The end position of the first [`HalfEdge`]
+    pub end_pos_of_first_half_edge: Point<2>,
+
+    /// The start position of the second [`HalfEdge`]
+    pub start_pos_of_second_half_edge: Point<2>,
+
+    /// The distance between the two positions
+    pub distance_between_positions: Scalar,
+
+    /// The edges
+    pub unconnected_half_edges: [Handle<HalfEdge>; 2],
+}
+
+impl ValidationCheck<AdjacentHalfEdgesNotConnected> for Cycle {
+    fn check(
+        &self,
+        config: &ValidationConfig,
+    ) -> impl Iterator<Item = AdjacentHalfEdgesNotConnected> {
+        self.half_edges().pairs().filter_map(|(first, second)| {
+            let end_pos_of_first_half_edge = {
+                let [_, end] = first.boundary().inner;
+                first.path().point_from_path_coords(end)
+            };
+            let start_pos_of_second_half_edge = second.start_position();
+
+            let distance_between_positions = (end_pos_of_first_half_edge
+                - start_pos_of_second_half_edge)
+                .magnitude();
+
+            if distance_between_positions > config.identical_max_distance {
+                return Some(AdjacentHalfEdgesNotConnected {
+                    end_pos_of_first_half_edge,
+                    start_pos_of_second_half_edge,
+                    distance_between_positions,
+                    unconnected_half_edges: [first.clone(), second.clone()],
+                });
+            }
+
+            None
+        })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+
+    use crate::{
+        objects::{Cycle, HalfEdge},
+        operations::{
+            build::{BuildCycle, BuildHalfEdge},
+            update::UpdateCycle,
+        },
+        validation::ValidationCheck,
+        Core,
+    };
+
+    #[test]
+    fn adjacent_half_edges_connected() -> anyhow::Result<()> {
+        let mut core = Core::new();
+
+        let valid = Cycle::polygon([[0., 0.], [1., 0.], [1., 1.]], &mut core);
+        valid.check_and_return_first_error()?;
+
+        let invalid = valid.update_half_edge(
+            valid.half_edges().first(),
+            |_, core| {
+                [HalfEdge::line_segment([[0., 0.], [2., 0.]], None, core)]
+            },
+            &mut core,
+        );
+        invalid.check_and_expect_one_error();
+
+        Ok(())
+    }
+}

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

@@ -0,0 +1,7 @@
+//! All validation checks
+//!
+//! See documentation of [parent module](super) for more information.
+
+mod half_edge_connection;
+
+pub use self::half_edge_connection::AdjacentHalfEdgesNotConnected;

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

@@ -1,16 +1,18 @@
 use std::{convert::Infallible, fmt};
 
 use crate::validate::{
-    CycleValidationError, EdgeValidationError, FaceValidationError,
-    ShellValidationError, SketchValidationError, SolidValidationError,
+    EdgeValidationError, FaceValidationError, ShellValidationError,
+    SketchValidationError, SolidValidationError,
 };
 
+use super::checks::AdjacentHalfEdgesNotConnected;
+
 /// An error that can occur during a validation
 #[derive(Clone, Debug, thiserror::Error)]
 pub enum ValidationError {
-    /// `Cycle` validation error
-    #[error("`Cycle` validation error")]
-    Cycle(#[from] CycleValidationError),
+    /// `HalfEdge`s in `Cycle` not connected
+    #[error(transparent)]
+    HalfEdgesInCycleNotConnected(#[from] AdjacentHalfEdgesNotConnected),
 
     /// `Edge` validation error
     #[error("`Edge` validation error")]

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

@@ -23,9 +23,13 @@
 mod config;
 mod error;
 mod validation;
+mod validation_check;
+
+pub mod checks;
 
 pub use self::{
     config::ValidationConfig,
     error::{ValidationError, ValidationErrors},
     validation::Validation,
+    validation_check::ValidationCheck,
 };

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

@@ -0,0 +1,53 @@
+use std::fmt::Display;
+
+use super::ValidationConfig;
+
+/// Run a specific validation check on an object
+///
+/// This trait is implemented once per validation check and object it applies
+/// to. `Self` is the object, while `T` identifies the validation check.
+pub trait ValidationCheck<T> {
+    /// Run the validation check on the implementing object
+    fn check(&self, config: &ValidationConfig) -> impl Iterator<Item = T>;
+
+    /// Convenience method to run the check return the first error
+    ///
+    /// This method is designed for convenience over flexibility (it is intended
+    /// for use in unit tests), and thus always uses the default configuration.
+    fn check_and_return_first_error(&self) -> Result<(), T> {
+        let errors =
+            self.check(&ValidationConfig::default()).collect::<Vec<_>>();
+
+        if let Some(err) = errors.into_iter().next() {
+            return Err(err);
+        }
+
+        Ok(())
+    }
+
+    /// Convenience method to run the check and expect one error
+    ///
+    /// This method is designed for convenience over flexibility (it is intended
+    /// for use in unit tests), and thus always uses the default configuration.
+    fn check_and_expect_one_error(&self)
+    where
+        T: Display,
+    {
+        let config = ValidationConfig::default();
+        let mut errors = self.check(&config).peekable();
+
+        errors
+            .next()
+            .expect("Expected one validation error; none found");
+
+        if errors.peek().is_some() {
+            println!("Unexpected validation errors:");
+
+            for err in errors {
+                println!("{err}");
+            }
+
+            panic!("Expected only one validation error")
+        }
+    }
+}