Merge pull request #1030 from hannobraun/validate

Clean up validation code

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

@@ -10,5 +10,6 @@ pub mod intersect;
 pub mod reverse;
 pub mod sweep;
 pub mod transform;
+pub mod validate;
 
 pub use self::triangulate::triangulate;

crates/fj-kernel/src/algorithms/validate/coherence.rs

@@ -0,0 +1,58 @@
+use std::fmt;
+
+use fj_math::{Point, Scalar};
+
+use crate::objects::Vertex;
+
+pub fn validate_vertex(
+    vertex: &Vertex,
+    max_distance: impl Into<Scalar>,
+) -> Result<(), CoherenceMismatch> {
+    let max_distance = max_distance.into();
+
+    // Validate that the local and global forms of the vertex match. As a side
+    // effect, this also happens to validate that the global form of the vertex
+    // lies on the curve.
+
+    let local = vertex.position();
+    let local_as_global = vertex
+        .curve()
+        .global()
+        .kind()
+        .point_from_curve_coords(local);
+    let global = vertex.global().position();
+    let distance = (local_as_global - global).magnitude();
+
+    if distance > max_distance {
+        return Err(CoherenceMismatch {
+            local,
+            local_as_global,
+            global,
+        });
+    }
+
+    Ok(())
+}
+
+/// A mismatch between the local and global forms of an object
+#[derive(Debug, Default, thiserror::Error)]
+pub struct CoherenceMismatch {
+    /// The local form of the object
+    pub local: Point<1>,
+
+    /// The local form of the object, converted into the global form
+    pub local_as_global: Point<3>,
+
+    /// The global form of the object
+    pub global: Point<3>,
+}
+
+impl fmt::Display for CoherenceMismatch {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(
+            f,
+            "local: {:?} (converted to global: {:?}), global: {:?},",
+            self.local, self.local_as_global, self.global,
+        )
+    }
+}

crates/fj-kernel/src/validation/mod.rs → crates/fj-kernel/src/algorithms/validate/mod.rs

@@ -17,42 +17,64 @@
 mod coherence;
 mod uniqueness;
 
-pub use self::{
-    coherence::{CoherenceIssues, CoherenceMismatch},
-    uniqueness::UniquenessIssues,
-};
+pub use self::{coherence::CoherenceMismatch, uniqueness::UniquenessIssues};
 
 use std::{collections::HashSet, ops::Deref};
 
 use fj_math::Scalar;
 
 use crate::iter::ObjectIters;
 
-/// Validate the given object
-pub fn validate<T>(
-    object: T,
-    config: &ValidationConfig,
-) -> Result<Validated<T>, ValidationError>
+/// Validate an object
+pub trait Validate: Sized {
+    /// Validate the object using default configuration
+    ///
+    /// The following calls are equivalent:
+    /// ``` rust
+    /// # use fj_kernel::{
+    /// #     algorithms::validate::{Validate, ValidationConfig},
+    /// #     objects::GlobalVertex,
+    /// # };
+    /// # let object = GlobalVertex::from_position([0., 0., 0.]);
+    /// object.validate();
+    /// object.validate_with_config(&ValidationConfig::default());
+    /// ```
+    fn validate(self) -> Result<Validated<Self>, ValidationError> {
+        self.validate_with_config(&ValidationConfig::default())
+    }
+
+    /// Validate the object
+    fn validate_with_config(
+        self,
+        config: &ValidationConfig,
+    ) -> Result<Validated<Self>, ValidationError>;
+}
+
+impl<T> Validate for T
 where
     T: for<'r> ObjectIters<'r>,
 {
-    let mut vertices = HashSet::new();
-
-    for vertex in object.global_vertex_iter() {
-        uniqueness::validate_vertex(
-            vertex,
-            &vertices,
-            config.distinct_min_distance,
-        )?;
-
-        vertices.insert(*vertex);
-    }
+    fn validate_with_config(
+        self,
+        config: &ValidationConfig,
+    ) -> Result<Validated<Self>, ValidationError> {
+        let mut global_vertices = HashSet::new();
+
+        for global_vertex in self.global_vertex_iter() {
+            uniqueness::validate_vertex(
+                global_vertex,
+                &global_vertices,
+                config.distinct_min_distance,
+            )?;
+
+            global_vertices.insert(*global_vertex);
+        }
+        for vertex in self.vertex_iter() {
+            coherence::validate_vertex(vertex, config.identical_max_distance)?;
+        }
 
-    for edge in object.edge_iter() {
-        coherence::validate_edge(edge, config.identical_max_distance)?;
+        Ok(Validated(self))
     }
-
-    Ok(Validated(object))
 }
 
 /// Configuration required for the validation process
@@ -114,7 +136,7 @@ impl<T> Deref for Validated<T> {
 pub enum ValidationError {
     /// Coherence validation failed
     #[error("Coherence validation failed")]
-    Coherence(#[from] CoherenceIssues),
+    Coherence(#[from] CoherenceMismatch),
 
     /// Geometric validation failed
     #[error("Geometric validation failed")]
@@ -130,11 +152,11 @@ mod tests {
     use fj_math::{Point, Scalar};
 
     use crate::{
+        algorithms::validate::{Validate, ValidationConfig, ValidationError},
         objects::{
             Curve, CurveKind, Edge, GlobalCurve, GlobalVertex, Surface, Vertex,
             VerticesOfEdge,
         },
-        validation::{validate, ValidationConfig, ValidationError},
     };
 
     #[test]
@@ -160,22 +182,16 @@ mod tests {
 
         let edge = Edge::from_curve_and_vertices(curve, vertices);
 
-        let result = validate(
-            edge,
-            &ValidationConfig {
-                identical_max_distance: deviation * 2.,
-                ..ValidationConfig::default()
-            },
-        );
+        let result = edge.validate_with_config(&ValidationConfig {
+            identical_max_distance: deviation * 2.,
+            ..ValidationConfig::default()
+        });
         assert!(result.is_ok());
 
-        let result = validate(
-            edge,
-            &ValidationConfig {
-                identical_max_distance: deviation / 2.,
-                ..ValidationConfig::default()
-            },
-        );
+        let result = edge.validate_with_config(&ValidationConfig {
+            identical_max_distance: deviation / 2.,
+            ..ValidationConfig::default()
+        });
         assert!(result.is_err());
     }
 
@@ -197,11 +213,11 @@ mod tests {
 
         // Adding a vertex should work.
         shape.push(GlobalVertex::from_position(a));
-        validate(shape.clone(), &config)?;
+        shape.clone().validate_with_config(&config)?;
 
         // Adding a second vertex that is considered identical should fail.
         shape.push(GlobalVertex::from_position(b));
-        let result = validate(shape, &config);
+        let result = shape.validate_with_config(&config);
         assert!(matches!(result, Err(ValidationError::Uniqueness(_))));
 
         Ok(())

crates/fj-kernel/src/lib.rs

@@ -91,4 +91,3 @@ pub mod algorithms;
 pub mod builder;
 pub mod iter;
 pub mod objects;
-pub mod validation;

crates/fj-kernel/src/objects/vertex.rs

@@ -70,7 +70,7 @@ impl Vertex {
 /// between distinct vertices can be configured using the respective field in
 /// [`ValidationConfig`].
 ///
-/// [`ValidationConfig`]: crate::validation::ValidationConfig
+/// [`ValidationConfig`]: crate::algorithms::validate::ValidationConfig
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct GlobalVertex {
     position: Point<3>,

crates/fj-operations/src/difference_2d.rs

@@ -1,9 +1,12 @@
 use fj_interop::{debug::DebugInfo, mesh::Color};
 use fj_kernel::{
-    algorithms::{approx::Tolerance, reverse::Reverse},
+    algorithms::{
+        approx::Tolerance,
+        reverse::Reverse,
+        validate::{Validate, Validated, ValidationConfig, ValidationError},
+    },
     iter::ObjectIters,
     objects::{Face, Sketch},
-    validation::{validate, Validated, ValidationConfig, ValidationError},
 };
 use fj_math::Aabb;
 
@@ -75,7 +78,7 @@ impl Shape for fj::Difference2d {
         }
 
         let difference = Sketch::new().with_faces(faces);
-        validate(difference, config)
+        difference.validate_with_config(config)
     }
 
     fn bounding_volume(&self) -> Aabb<3> {

crates/fj-operations/src/group.rs

@@ -1,8 +1,10 @@
 use fj_interop::debug::DebugInfo;
 use fj_kernel::{
-    algorithms::approx::Tolerance,
+    algorithms::{
+        approx::Tolerance,
+        validate::{Validate, Validated, ValidationConfig, ValidationError},
+    },
     objects::Face,
-    validation::{validate, Validated, ValidationConfig, ValidationError},
 };
 use fj_math::Aabb;
 
@@ -25,7 +27,7 @@ impl Shape for fj::Group {
         faces.extend(a.into_inner());
         faces.extend(b.into_inner());
 
-        validate(faces, config)
+        faces.validate_with_config(config)
     }
 
     fn bounding_volume(&self) -> Aabb<3> {