diff --git a/crates/fj-kernel/src/algorithms/validate/coherence.rs b/crates/fj-kernel/src/algorithms/validate/coherence.rs index a019e2a2d..d3f679b8f 100644 --- a/crates/fj-kernel/src/algorithms/validate/coherence.rs +++ b/crates/fj-kernel/src/algorithms/validate/coherence.rs @@ -2,12 +2,43 @@ use std::fmt; use fj_math::{Point, Scalar}; -use crate::objects::Vertex; +use crate::objects::{Curve, Vertex}; + +pub fn validate_curve( + curve: &Curve, + max_distance: impl Into, +) -> Result<(), CoherenceIssues> { + let max_distance = max_distance.into(); + + let points_curve = [-2., -1., 0., 1., 2.].map(|point| Point::from([point])); + + for point_curve in points_curve { + let point_surface = curve.kind().point_from_curve_coords(point_curve); + let point_surface_as_global = + curve.surface().point_from_surface_coords(point_surface); + let point_global = + curve.global().kind().point_from_curve_coords(point_curve); + + let distance = (point_surface_as_global - point_global).magnitude(); + + if distance > max_distance { + Err(CurveCoherenceMismatch { + point_curve, + point_surface, + point_surface_as_global, + point_global, + curve: *curve, + })? + } + } + + Ok(()) +} pub fn validate_vertex( vertex: &Vertex, max_distance: impl Into, -) -> Result<(), CoherenceMismatch> { +) -> Result<(), CoherenceIssues> { let max_distance = max_distance.into(); // Validate that the local and global forms of the vertex match. As a side @@ -24,19 +55,65 @@ pub fn validate_vertex( let distance = (local_as_global - global).magnitude(); if distance > max_distance { - return Err(CoherenceMismatch { + Err(VertexCoherenceMismatch { local, local_as_global, global, - }); + })? } Ok(()) } -/// A mismatch between the local and global forms of an object +/// Issues in coherence validation +#[allow(clippy::large_enum_variant)] +#[derive(Debug, thiserror::Error)] +pub enum CoherenceIssues { + /// Mismatch between the surface and global forms of a curve + #[error("Mismatch between surface and global forms of curve")] + Curve(#[from] CurveCoherenceMismatch), + + /// Mismatch between the local and global coordinates of a vertex + #[error("Mismatch between local and global coordinates of vertex")] + Vertex(#[from] VertexCoherenceMismatch), +} + +/// A mismatch between the surface and global forms of a curve +/// +/// Used in [`CoherenceIssues`]. +#[derive(Debug, thiserror::Error)] +pub struct CurveCoherenceMismatch { + /// The curve coordinate for which a mismatch was found + pub point_curve: Point<1>, + + /// The curve coordinate, converted to surface coordinates + pub point_surface: Point<2>, + + /// The surface coordinates, converted to global coordinates + pub point_surface_as_global: Point<3>, + + /// The curve coordinate, converted to global coordinates + pub point_global: Point<3>, + + /// The incoherent curve + pub curve: Curve, +} + +impl fmt::Display for CurveCoherenceMismatch { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "local: {:?} (converted to surface: {:?}; to global: {:?}), global: {:?},", + self.point_curve, self.point_surface, self.point_surface_as_global, self.point_global, + ) + } +} + +/// A mismatch between the local and global forms of a vertex +/// +/// Used in [`CoherenceIssues`]. #[derive(Debug, Default, thiserror::Error)] -pub struct CoherenceMismatch { +pub struct VertexCoherenceMismatch { /// The local form of the object pub local: Point<1>, @@ -47,7 +124,7 @@ pub struct CoherenceMismatch { pub global: Point<3>, } -impl fmt::Display for CoherenceMismatch { +impl fmt::Display for VertexCoherenceMismatch { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!( f, diff --git a/crates/fj-kernel/src/algorithms/validate/mod.rs b/crates/fj-kernel/src/algorithms/validate/mod.rs index ddf3dbbc4..e670d6cd0 100644 --- a/crates/fj-kernel/src/algorithms/validate/mod.rs +++ b/crates/fj-kernel/src/algorithms/validate/mod.rs @@ -17,7 +17,10 @@ mod coherence; mod uniqueness; -pub use self::{coherence::CoherenceMismatch, uniqueness::UniquenessIssues}; +pub use self::{ + coherence::{CoherenceIssues, VertexCoherenceMismatch}, + uniqueness::UniquenessIssues, +}; use std::{collections::HashSet, ops::Deref}; @@ -60,6 +63,10 @@ where ) -> Result, ValidationError> { let mut global_vertices = HashSet::new(); + for curve in self.curve_iter() { + coherence::validate_curve(curve, config.identical_max_distance)?; + } + for global_vertex in self.global_vertex_iter() { uniqueness::validate_vertex( global_vertex, @@ -136,7 +143,7 @@ impl Deref for Validated { pub enum ValidationError { /// Coherence validation failed #[error("Coherence validation failed")] - Coherence(#[from] CoherenceMismatch), + Coherence(#[from] CoherenceIssues), /// Geometric validation failed #[error("Geometric validation failed")] @@ -149,7 +156,7 @@ pub enum ValidationError { #[cfg(test)] mod tests { - use fj_math::{Point, Scalar}; + use fj_math::{Line, Point, Scalar}; use crate::{ algorithms::validate::{Validate, ValidationConfig, ValidationError}, @@ -159,6 +166,22 @@ mod tests { }, }; + #[test] + fn coherence_curve() { + let line_global = Line::from_points([[0., 0., 0.], [1., 0., 0.]]); + let global_curve = GlobalCurve::from_kind(CurveKind::Line(line_global)); + + let line_surface = Line::from_points([[0., 0.], [2., 0.]]); + let curve = Curve::new( + Surface::xy_plane(), + CurveKind::Line(line_surface), + global_curve, + ); + + let result = curve.validate(); + assert!(result.is_err()); + } + #[test] fn coherence_edge() { let a = Point::from([0., 0., 0.]); diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index a5da65dea..44dce0755 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -37,6 +37,20 @@ impl Edge { assert_eq!(curve.global(), global.curve()); assert_eq!(&vertices.to_global(), global.vertices()); + // Make sure that the edge vertices are not coincident on the curve. If + // they were, the edge would have no length, and not be valid. + // + // It is perfectly fine for global forms of the the vertices to be + // coincident (in 3D space). That would just mean, that ends of the edge + // connect to each other. + if let Some([a, b]) = vertices.get() { + assert_ne!( + a.position(), + b.position(), + "Vertices of an edge must not be coincident on curve" + ); + } + Self { curve, vertices,