diff --git a/crates/fj-kernel/src/objects/vertex.rs b/crates/fj-kernel/src/objects/vertex.rs index b7febcfc1d..62a6db6a4d 100644 --- a/crates/fj-kernel/src/objects/vertex.rs +++ b/crates/fj-kernel/src/objects/vertex.rs @@ -1,5 +1,4 @@ use fj_math::Point; -use pretty_assertions::assert_eq; use crate::storage::Handle; @@ -29,12 +28,6 @@ impl Vertex { ) -> Self { let position = position.into(); - assert_eq!( - curve.surface().id(), - surface_form.surface().id(), - "Surface form of vertex must be defined on same surface as curve", - ); - Self { position, curve, diff --git a/crates/fj-kernel/src/validate/coherence.rs b/crates/fj-kernel/src/validate/coherence.rs deleted file mode 100644 index dc7842d4b6..0000000000 --- a/crates/fj-kernel/src/validate/coherence.rs +++ /dev/null @@ -1,69 +0,0 @@ -use std::fmt; - -use fj_math::{Point, Scalar}; - -use crate::objects::Vertex; - -pub fn validate_vertex( - vertex: &Vertex, - max_distance: impl Into, -) -> Result<(), CoherenceIssues> { - 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_surface = vertex.curve().path().point_from_path_coords(local); - let local_as_global = vertex - .curve() - .surface() - .point_from_surface_coords(local_as_surface); - let global = vertex.global_form().position(); - let distance = (local_as_global - global).magnitude(); - - if distance > max_distance { - Err(VertexCoherenceMismatch { - local, - local_as_global, - global, - })? - } - - Ok(()) -} - -/// Issues in coherence validation -#[allow(clippy::large_enum_variant)] -#[derive(Debug, thiserror::Error)] -pub enum CoherenceIssues { - /// 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 local and global forms of a vertex -/// -/// Used in [`CoherenceIssues`]. -#[derive(Debug, Default, thiserror::Error)] -pub struct VertexCoherenceMismatch { - /// 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 VertexCoherenceMismatch { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "local: {:?} (converted to global: {:?}), global: {:?},", - self.local, self.local_as_global, self.global, - ) - } -} diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 27ced8c480..8bace57373 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -14,7 +14,6 @@ //! Please note that not all of these validation categories are fully //! implemented, as of this writing. -mod coherence; mod curve; mod cycle; mod edge; @@ -27,8 +26,8 @@ mod uniqueness; mod vertex; pub use self::{ - coherence::{CoherenceIssues, VertexCoherenceMismatch}, uniqueness::UniquenessIssues, + vertex::{SurfaceVertexPositionMismatch, VertexValidationError}, }; use std::{collections::HashSet, convert::Infallible, ops::Deref}; @@ -94,9 +93,6 @@ where global_vertices.insert(*global_vertex); } - for vertex in self.vertex_iter() { - coherence::validate_vertex(vertex, config.identical_max_distance)?; - } Ok(Validated(self)) } @@ -176,10 +172,6 @@ impl Deref for Validated { #[allow(clippy::large_enum_variant)] #[derive(Debug, thiserror::Error)] pub enum ValidationError { - /// Coherence validation failed - #[error("Coherence validation failed")] - Coherence(#[from] CoherenceIssues), - /// Geometric validation failed #[error("Geometric validation failed")] Geometric, @@ -187,6 +179,14 @@ pub enum ValidationError { /// Uniqueness validation failed #[error("Uniqueness validation failed")] Uniqueness(#[from] UniquenessIssues), + + /// `SurfaceVertex` position didn't match `GlobalVertex` + #[error(transparent)] + SurfaceVertexPositionMismatch(#[from] SurfaceVertexPositionMismatch), + + /// `Vertex` position didn't match `SurfaceVertex` + #[error(transparent)] + Vertex(#[from] VertexValidationError), } impl From for ValidationError { @@ -197,91 +197,13 @@ impl From for ValidationError { #[cfg(test)] mod tests { - use fj_interop::ext::ArrayExt; use fj_math::{Point, Scalar}; use crate::{ - objects::{ - Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, - SurfaceVertex, Vertex, - }, - partial::HasPartial, - path::SurfacePath, + objects::{GlobalVertex, Objects}, validate::{Validate, ValidationConfig, ValidationError}, }; - #[test] - fn coherence_edge() -> anyhow::Result<()> { - let objects = Objects::new(); - - let surface = objects.surfaces.xy_plane(); - - let points_surface = [[0., 0.], [1., 0.]]; - let points_global = [[0., 0., 0.], [1., 0., 0.]]; - - let curve = { - let path = SurfacePath::line_from_points(points_surface); - let global_form = objects.global_curves.insert(GlobalCurve)?; - - objects.curves.insert(Curve::new( - surface.clone(), - path, - global_form, - ))? - }; - - let vertices_global = points_global.try_map_ext(|point| { - objects - .global_vertices - .insert(GlobalVertex::from_position(point)) - })?; - - let [a_surface, b_surface] = points_surface - .zip_ext(vertices_global) - .try_map_ext(|(point_surface, vertex_global)| { - objects.surface_vertices.insert(SurfaceVertex::new( - point_surface, - surface.clone(), - vertex_global, - )) - })?; - - let deviation = Scalar::from_f64(0.25); - - let a = objects.vertices.insert(Vertex::new( - Point::from([Scalar::ZERO + deviation]), - curve.clone(), - a_surface, - ))?; - let b = objects.vertices.insert(Vertex::new( - Point::from([Scalar::ONE]), - curve.clone(), - b_surface, - ))?; - let vertices = [a, b]; - - let global_edge = GlobalEdge::partial() - .from_curve_and_vertices(&curve, &vertices) - .build(&objects)?; - let half_edge = objects - .half_edges - .insert(HalfEdge::new(vertices, global_edge)); - - let result = - half_edge.clone().validate_with_config(&ValidationConfig { - identical_max_distance: deviation * 2., - ..ValidationConfig::default() - }); - assert!(result.is_ok()); - - let result = half_edge.validate_with_config(&ValidationConfig { - identical_max_distance: deviation / 2., - ..ValidationConfig::default() - }); - assert!(result.is_err()); - Ok(()) - } - #[test] fn uniqueness_vertex() -> anyhow::Result<()> { let objects = Objects::new(); diff --git a/crates/fj-kernel/src/validate/vertex.rs b/crates/fj-kernel/src/validate/vertex.rs index 3058f447fa..5a40862398 100644 --- a/crates/fj-kernel/src/validate/vertex.rs +++ b/crates/fj-kernel/src/validate/vertex.rs @@ -1,31 +1,140 @@ -use std::convert::Infallible; +use std::{convert::Infallible, ops::Deref}; -use crate::objects::{GlobalVertex, SurfaceVertex, Vertex}; +use fj_math::{Point, Scalar}; + +use crate::{ + objects::{GlobalVertex, Surface, SurfaceVertex, Vertex}, + storage::Handle, +}; use super::{Validate2, ValidationConfig}; impl Validate2 for Vertex { - type Error = Infallible; + type Error = VertexValidationError; fn validate_with_config( &self, - _: &ValidationConfig, + config: &ValidationConfig, ) -> Result<(), Self::Error> { + let curve_surface = self.curve().surface(); + let surface_form_surface = self.surface_form().surface(); + if curve_surface.id() != surface_form_surface.id() { + return Err(VertexValidationError::SurfaceMismatch { + curve_surface: curve_surface.clone(), + surface_form_surface: surface_form_surface.clone(), + }); + } + + let curve_position_as_surface = + self.curve().path().point_from_path_coords(self.position()); + let surface_position = self.surface_form().position(); + + let distance = curve_position_as_surface.distance_to(&surface_position); + + if distance > config.identical_max_distance { + return Err(VertexValidationError::PositionMismatch { + vertex: self.clone(), + surface_vertex: self.surface_form().clone_object(), + curve_position_as_surface, + distance, + }); + } + Ok(()) } } +/// [`Vertex`] validation failed +#[derive(Debug, thiserror::Error)] +pub enum VertexValidationError { + /// Mismatch between the surface's of the curve and surface form + #[error( + "Surface form of vertex must be defined on same surface as curve\n\ + `Surface` of curve: {curve_surface:?} -> {:?}\n\ + `Surface` of surface form: {surface_form_surface:?} -> {:?}", + .curve_surface.deref(), + .surface_form_surface.deref(), + )] + SurfaceMismatch { + /// The surface of the vertex' curve + curve_surface: Handle, + + /// The surface of the vertex' surface form + surface_form_surface: Handle, + }, + + /// Mismatch between position of the vertex and position of its surface form + #[error( + "`Vertex` position doesn't match position of its surface form\n\ + `Vertex`: {vertex:#?}\n\ + `SurfaceVertex`: {surface_vertex:#?}\n\ + `Vertex` position as surface: {curve_position_as_surface:?}\n\ + Distance between the positions: {distance}" + )] + PositionMismatch { + /// The vertex + vertex: Vertex, + + /// The mismatched surface vertex + surface_vertex: SurfaceVertex, + + /// The curve position converted into a surface position + curve_position_as_surface: Point<2>, + + /// The distance between the positions + distance: Scalar, + }, +} + impl Validate2 for SurfaceVertex { - type Error = Infallible; + type Error = SurfaceVertexPositionMismatch; fn validate_with_config( &self, - _: &ValidationConfig, + config: &ValidationConfig, ) -> Result<(), Self::Error> { + let surface_position_as_global = + self.surface().point_from_surface_coords(self.position()); + let global_position = self.global_form().position(); + + let distance = surface_position_as_global.distance_to(&global_position); + + if distance > config.identical_max_distance { + return Err(SurfaceVertexPositionMismatch { + surface_vertex: self.clone(), + global_vertex: self.global_form().clone_object(), + surface_position_as_global, + distance, + }); + } + Ok(()) } } +/// Mismatch between position of surface vertex and position of its global form +#[derive(Debug, thiserror::Error)] +#[error( + "`SurfaceVertex` position doesn't match position of its global form\n\ + `SurfaceVertex`: {surface_vertex:#?}\n\ + `GlobalVertex`: {global_vertex:#?}\n\ + `SurfaceVertex` position as global: {surface_position_as_global:?}\n\ + Distance between the positions: {distance}" +)] +pub struct SurfaceVertexPositionMismatch { + /// The surface vertex + pub surface_vertex: SurfaceVertex, + + /// The mismatched global vertex + pub global_vertex: GlobalVertex, + + /// The surface position converted into a global position + pub surface_position_as_global: Point<3>, + + /// The distance between the positions + pub distance: Scalar, +} + impl Validate2 for GlobalVertex { type Error = Infallible; @@ -36,3 +145,103 @@ impl Validate2 for GlobalVertex { Ok(()) } } + +#[cfg(test)] +mod tests { + use crate::{ + objects::{Curve, GlobalVertex, Objects, SurfaceVertex, Vertex}, + partial::HasPartial, + validate::Validate2, + }; + + #[test] + fn vertex_surface_mismatch() -> anyhow::Result<()> { + let objects = Objects::new(); + + let valid = Vertex::new( + [0.], + Curve::partial() + .with_surface(Some(objects.surfaces.xy_plane())) + .as_u_axis() + .build(&objects)?, + SurfaceVertex::partial() + .with_surface(Some(objects.surfaces.xy_plane())) + .with_position(Some([0., 0.])) + .build(&objects)?, + ); + let invalid = Vertex::new( + [0.], + Curve::partial() + .with_surface(Some(objects.surfaces.xy_plane())) + .as_u_axis() + .build(&objects)?, + SurfaceVertex::partial() + .with_surface(Some(objects.surfaces.xz_plane())) + .with_position(Some([0., 0.])) + .build(&objects)?, + ); + + assert!(valid.validate().is_ok()); + assert!(invalid.validate().is_err()); + + Ok(()) + } + + #[test] + fn vertex_position_mismatch() -> anyhow::Result<()> { + let objects = Objects::new(); + + let valid = Vertex::new( + [0.], + Curve::partial() + .with_surface(Some(objects.surfaces.xy_plane())) + .as_u_axis() + .build(&objects)?, + SurfaceVertex::partial() + .with_surface(Some(objects.surfaces.xy_plane())) + .with_position(Some([0., 0.])) + .build(&objects)?, + ); + let invalid = Vertex::new( + [0.], + Curve::partial() + .with_surface(Some(objects.surfaces.xy_plane())) + .as_u_axis() + .build(&objects)?, + SurfaceVertex::partial() + .with_surface(Some(objects.surfaces.xy_plane())) + .with_position(Some([1., 0.])) + .build(&objects)?, + ); + + assert!(valid.validate().is_ok()); + assert!(invalid.validate().is_err()); + + Ok(()) + } + + #[test] + fn surface_vertex_position_mismatch() -> anyhow::Result<()> { + let objects = Objects::new(); + + let valid = SurfaceVertex::new( + [0., 0.], + objects.surfaces.xy_plane(), + objects + .global_vertices + .insert(GlobalVertex::from_position([0., 0., 0.]))?, + ); + let invalid = SurfaceVertex::new( + [0., 0.], + objects.surfaces.xy_plane(), + objects + .global_vertices + .insert(GlobalVertex::from_position([1., 0., 0.]))?, + ); + + assert!(valid.validate().is_ok()); + assert!(invalid.validate().is_err()); + + Ok(()) + } +}