diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index af00299b77..da79951016 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -4,7 +4,8 @@ use fj_math::{Line, Scalar, Transform, Triangle, Vector}; use crate::{ objects::{Curve, Cycle, Edge, Face, Surface, SweptCurve, Vertex}, - shape::{Handle, LocalForm, Mapping, Shape, ValidationError}, + shape::{Handle, LocalForm, Mapping, Shape}, + validation::ValidationError, }; use super::{transform_shape, CycleApprox, Tolerance}; diff --git a/crates/fj-kernel/src/algorithms/transform.rs b/crates/fj-kernel/src/algorithms/transform.rs index 45a0c9a5ac..9a237305b2 100644 --- a/crates/fj-kernel/src/algorithms/transform.rs +++ b/crates/fj-kernel/src/algorithms/transform.rs @@ -2,7 +2,8 @@ use fj_math::Transform; use crate::{ objects::{Curve, Face, Surface, Vertex}, - shape::{Shape, ValidationError}, + shape::Shape, + validation::ValidationError, }; /// Transform the geometry of the shape diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index 7e62f59b7f..b977fffaef 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -4,7 +4,8 @@ use fj_math::{Circle, Line, Point, Scalar, Vector}; use crate::{ objects::{Curve, Cycle, Edge, Face, Surface, Vertex, VerticesOfEdge}, - shape::{Handle, LocalForm, Shape, ValidationError, ValidationResult}, + shape::{Handle, LocalForm, Shape, ValidationResult}, + validation::ValidationError, }; /// API for building a [`Vertex`] diff --git a/crates/fj-kernel/src/lib.rs b/crates/fj-kernel/src/lib.rs index f2f344afcc..fd58d7887f 100644 --- a/crates/fj-kernel/src/lib.rs +++ b/crates/fj-kernel/src/lib.rs @@ -92,3 +92,4 @@ pub mod builder; pub mod geometry; pub mod objects; pub mod shape; +pub mod validation; diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index 7070a83cff..dc82f6e829 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -1,10 +1,13 @@ use fj_math::Scalar; -use crate::objects::{Curve, Cycle, Edge, Face, Surface, Vertex}; +use crate::{ + objects::{Curve, Cycle, Edge, Face, Surface, Vertex}, + validation::ValidationError, +}; use super::{ stores::{Store, Stores}, - Handle, Iter, Mapping, Object, Update, ValidationError, ValidationResult, + Handle, Iter, Mapping, Object, Update, ValidationResult, }; /// The boundary representation of a shape @@ -293,7 +296,8 @@ mod tests { use crate::{ objects::{Curve, Cycle, Edge, Face, Surface, Vertex, VerticesOfEdge}, - shape::{Handle, LocalForm, Shape, ValidationError, ValidationResult}, + shape::{Handle, LocalForm, Shape, ValidationResult}, + validation::ValidationError, }; #[test] diff --git a/crates/fj-kernel/src/shape/mod.rs b/crates/fj-kernel/src/shape/mod.rs index 21182ac6d3..f633b3df7f 100644 --- a/crates/fj-kernel/src/shape/mod.rs +++ b/crates/fj-kernel/src/shape/mod.rs @@ -19,6 +19,6 @@ pub use self::{ update::Update, validate::{ CoherenceIssues, CoherenceMismatch, DuplicateEdge, StructuralIssues, - UniquenessIssues, ValidationError, ValidationResult, + UniquenessIssues, ValidationResult, }, }; diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs index a85bc20d2a..b5a382ef55 100644 --- a/crates/fj-kernel/src/shape/object.rs +++ b/crates/fj-kernel/src/shape/object.rs @@ -1,10 +1,10 @@ -use crate::objects::{ - Curve, Cycle, Edge, Face, Surface, Vertex, VerticesOfEdge, +use crate::{ + objects::{Curve, Cycle, Edge, Face, Surface, Vertex, VerticesOfEdge}, + validation::ValidationError, }; use super::{ - validate::Validate, Handle, LocalForm, Mapping, Shape, ValidationError, - ValidationResult, + validate::Validate, Handle, LocalForm, Mapping, Shape, ValidationResult, }; /// Marker trait for geometric and topological objects diff --git a/crates/fj-kernel/src/shape/update.rs b/crates/fj-kernel/src/shape/update.rs index a90de32683..9651a1a5bf 100644 --- a/crates/fj-kernel/src/shape/update.rs +++ b/crates/fj-kernel/src/shape/update.rs @@ -1,6 +1,8 @@ use fj_math::Scalar; -use super::{stores::Stores, validate::Validate as _, Object, ValidationError}; +use crate::validation::ValidationError; + +use super::{stores::Stores, validate::Validate as _, Object}; /// API to update a `Shape` /// diff --git a/crates/fj-kernel/src/shape/validate/mod.rs b/crates/fj-kernel/src/shape/validate/mod.rs index 6882d9da5a..2e18185c68 100644 --- a/crates/fj-kernel/src/shape/validate/mod.rs +++ b/crates/fj-kernel/src/shape/validate/mod.rs @@ -10,7 +10,10 @@ pub use self::{ use fj_math::Scalar; -use crate::objects::{Curve, Cycle, Edge, Face, Surface, Vertex}; +use crate::{ + objects::{Curve, Cycle, Edge, Face, Surface, Vertex}, + validation::ValidationError, +}; use super::{stores::Stores, Handle, Object}; @@ -122,97 +125,3 @@ impl Validate for Face { /// Returned by the various `add_` methods of the [`Shape`] API pub type ValidationResult = Result, ValidationError>; - -/// An error that can occur during a validation -#[allow(clippy::large_enum_variant)] -#[derive(Debug, thiserror::Error)] -pub enum ValidationError { - /// Coherence validation failed - /// - /// Coherence validation verifies, that local forms of an objects are - /// consistent with their canonical forms. - #[error("Coherence validation failed")] - Coherence(#[from] CoherenceIssues), - - /// Geometric validation failed - /// - /// Geometric validation verifies, that various geometric constraints of an - /// object are upheld. For example, edges or faces might not be allowed to - /// intersect. - #[error("Geometric validation failed")] - Geometric, - - /// Structural validation failed - /// - /// Structural validation verifies, that all the object that an object - /// refers to are already part of the shape. - #[error("Structural validation failed")] - Structural(#[from] StructuralIssues), - - /// Uniqueness validation failed - /// - /// Uniqueness validation verifies, that an object is unique. Uniqueness is - /// only required for topological objects, as there's no harm in geometric - /// objects being duplicated. - #[error("Uniqueness validation failed")] - Uniqueness(#[from] UniquenessIssues), -} - -impl ValidationError { - /// Indicate whether validation found a missing curve - #[cfg(test)] - pub fn missing_curve(&self, curve: &Handle>) -> bool { - if let Self::Structural(StructuralIssues { missing_curve, .. }) = self { - return missing_curve.as_ref() == Some(curve); - } - - false - } - - /// Indicate whether validation found a missing vertex - #[cfg(test)] - pub fn missing_vertex(&self, vertex: &Handle) -> bool { - if let Self::Structural(StructuralIssues { - missing_vertices, .. - }) = self - { - return missing_vertices.contains(vertex); - } - - false - } - - /// Indicate whether validation found a missing edge - #[cfg(test)] - pub fn missing_edge(&self, edge: &Handle>) -> bool { - if let Self::Structural(StructuralIssues { missing_edges, .. }) = self { - return missing_edges.contains(edge); - } - - false - } - - /// Indicate whether validation found a missing surface - #[cfg(test)] - pub fn missing_surface(&self, surface: &Handle) -> bool { - if let Self::Structural(StructuralIssues { - missing_surface, .. - }) = self - { - return missing_surface.as_ref() == Some(surface); - } - - false - } - - /// Indicate whether validation found a missing cycle - #[cfg(test)] - pub fn missing_cycle(&self, cycle: &Handle>) -> bool { - if let Self::Structural(StructuralIssues { missing_cycles, .. }) = self - { - return missing_cycles.contains(cycle); - } - - false - } -} diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs new file mode 100644 index 0000000000..c4b52f5c03 --- /dev/null +++ b/crates/fj-kernel/src/validation/mod.rs @@ -0,0 +1,166 @@ +//! Infrastructure for validating shapes +//! +//! Validation enforces various constraints about shapes and the objects that +//! constitute them. These constraints fall into 4 categories: +//! +//! - **Coherence:** Local forms of objects must be consistent with their +//! canonical forms. +//! - **Geometric:** Comprises various object-specific constraints, for example +//! edges or faces might not be allowed to intersect. +//! - **Structural:** All other objects that an object references must be part +//! of the same shape. +//! - **Uniqueness:** Objects within a shape must be unique. +//! +//! Please note that not all of these validation categories are fully +//! implemented, as of this writing. +//! +//! # Implementation Note +//! +//! There is an ongoing effort to abolish [`Shape`] and replace it with a much +//! simpler data structure: +//! https://github.com/hannobraun/Fornjot/issues/697 +//! +//! Once completed, this would make structural and uniqueness validation moot. +//! +//! [`Shape`]: crate::shape::Shape + +use std::ops::Deref; + +use fj_math::Scalar; + +use crate::{ + objects::{Curve, Cycle, Edge, Surface, Vertex}, + shape::{ + CoherenceIssues, Handle, Shape, StructuralIssues, UniquenessIssues, + }, +}; + +/// Validate the given [`Shape`] +pub fn validate( + shape: Shape, + _: &Config, +) -> Result, ValidationError> { + Ok(Validated(shape)) +} + +/// Configuration required for the validation process +#[derive(Debug, Clone, Copy)] +pub struct Config { + /// The minimum distance between distinct objects + /// + /// Objects whose distance is less than the value defined in this field, are + /// considered identical. + pub distinct_min_distance: Scalar, + + /// The maximum distance between identical objects + /// + /// Objects that are considered identical might still have a distance + /// between them, due to inaccuracies of the numerical representation. If + /// that distance is less than the one defined in this field, can not be + /// considered identical. + pub identical_max_distance: Scalar, +} + +impl Default for Config { + fn default() -> Self { + Self { + distinct_min_distance: Scalar::from_f64(5e-7), // 0.5 µm, + identical_max_distance: Scalar::from_f64(5e-14), + } + } +} + +/// Wrapper around an object that indicates the object has been validated +/// +/// Returned by implementations of `Validate`. +pub struct Validated(T); + +impl Validated { + /// Consume this instance of `Validated` and return the wrapped object + pub fn into_inner(self) -> T { + self.0 + } +} + +impl Deref for Validated { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +/// An error that can occur during a validation +#[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, + + /// Structural validation failed + #[error("Structural validation failed")] + Structural(#[from] StructuralIssues), + + /// Uniqueness validation failed + #[error("Uniqueness validation failed")] + Uniqueness(#[from] UniquenessIssues), +} + +impl ValidationError { + /// Indicate whether validation found a missing curve + pub fn missing_curve(&self, curve: &Handle>) -> bool { + if let Self::Structural(StructuralIssues { missing_curve, .. }) = self { + return missing_curve.as_ref() == Some(curve); + } + + false + } + + /// Indicate whether validation found a missing vertex + pub fn missing_vertex(&self, vertex: &Handle) -> bool { + if let Self::Structural(StructuralIssues { + missing_vertices, .. + }) = self + { + return missing_vertices.contains(vertex); + } + + false + } + + /// Indicate whether validation found a missing edge + pub fn missing_edge(&self, edge: &Handle>) -> bool { + if let Self::Structural(StructuralIssues { missing_edges, .. }) = self { + return missing_edges.contains(edge); + } + + false + } + + /// Indicate whether validation found a missing surface + pub fn missing_surface(&self, surface: &Handle) -> bool { + if let Self::Structural(StructuralIssues { + missing_surface, .. + }) = self + { + return missing_surface.as_ref() == Some(surface); + } + + false + } + + /// Indicate whether validation found a missing cycle + pub fn missing_cycle(&self, cycle: &Handle>) -> bool { + if let Self::Structural(StructuralIssues { missing_cycles, .. }) = self + { + return missing_cycles.contains(cycle); + } + + false + } +} diff --git a/crates/fj-operations/src/circle.rs b/crates/fj-operations/src/circle.rs index 7d1c568276..a9b49b0b09 100644 --- a/crates/fj-operations/src/circle.rs +++ b/crates/fj-operations/src/circle.rs @@ -2,7 +2,8 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::Tolerance, objects::{Cycle, Edge, Face, Surface}, - shape::{LocalForm, Shape, ValidationError}, + shape::{LocalForm, Shape}, + validation::{self, validate, Validated, ValidationError}, }; use fj_math::{Aabb, Point, Scalar}; @@ -11,9 +12,10 @@ use super::ToShape; impl ToShape for fj::Circle { fn to_shape( &self, + config: &validation::Config, _: Tolerance, _: &mut DebugInfo, - ) -> Result { + ) -> Result, ValidationError> { let mut shape = Shape::new(); // Circles have just a single round edge with no vertices. So none need @@ -36,6 +38,8 @@ impl ToShape for fj::Circle { self.color(), ))?; + let shape = validate(shape, config)?; + Ok(shape) } diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 414873b8b1..553ed672a9 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -2,7 +2,8 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::Tolerance, objects::{Cycle, Edge, Face}, - shape::{LocalForm, Shape, ValidationError}, + shape::{LocalForm, Shape}, + validation::{self, validate, Validated, ValidationError}, }; use fj_math::Aabb; @@ -11,9 +12,10 @@ use super::ToShape; impl ToShape for fj::Difference2d { fn to_shape( &self, + config: &validation::Config, tolerance: Tolerance, debug_info: &mut DebugInfo, - ) -> Result { + ) -> Result, ValidationError> { // This method assumes that `b` is fully contained within `a`: // https://github.com/hannobraun/Fornjot/issues/92 @@ -26,7 +28,8 @@ impl ToShape for fj::Difference2d { // - https://doc.rust-lang.org/std/primitive.array.html#method.each_ref // - https://doc.rust-lang.org/std/primitive.array.html#method.try_map let [a, b] = self.shapes(); - let [a, b] = [a, b].map(|shape| shape.to_shape(tolerance, debug_info)); + let [a, b] = + [a, b].map(|shape| shape.to_shape(config, tolerance, debug_info)); let [a, b] = [a?, b?]; if let Some(face) = a.faces().next() { @@ -78,6 +81,8 @@ impl ToShape for fj::Difference2d { ))?; } + let difference = validate(difference, config)?; + Ok(difference) } diff --git a/crates/fj-operations/src/group.rs b/crates/fj-operations/src/group.rs index 99ffc75fda..230c5fd70c 100644 --- a/crates/fj-operations/src/group.rs +++ b/crates/fj-operations/src/group.rs @@ -1,7 +1,8 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::Tolerance, - shape::{Shape, ValidationError}, + shape::Shape, + validation::{self, validate, Validated, ValidationError}, }; use fj_math::Aabb; @@ -10,17 +11,20 @@ use super::ToShape; impl ToShape for fj::Group { fn to_shape( &self, + config: &validation::Config, tolerance: Tolerance, debug_info: &mut DebugInfo, - ) -> Result { + ) -> Result, ValidationError> { let mut shape = Shape::new(); - let a = self.a.to_shape(tolerance, debug_info)?; - let b = self.b.to_shape(tolerance, debug_info)?; + let a = self.a.to_shape(config, tolerance, debug_info)?; + let b = self.b.to_shape(config, tolerance, debug_info)?; shape.merge_shape(&a)?; shape.merge_shape(&b)?; + let shape = validate(shape, config)?; + Ok(shape) } diff --git a/crates/fj-operations/src/lib.rs b/crates/fj-operations/src/lib.rs index 71a445b8b2..f19d2f0867 100644 --- a/crates/fj-operations/src/lib.rs +++ b/crates/fj-operations/src/lib.rs @@ -28,7 +28,8 @@ mod transform; use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::Tolerance, - shape::{Shape, ValidationError}, + shape::Shape, + validation::{self, Validated, ValidationError}, }; use fj_math::Aabb; @@ -37,9 +38,10 @@ pub trait ToShape { /// Compute the boundary representation of the shape fn to_shape( &self, + config: &validation::Config, tolerance: Tolerance, debug_info: &mut DebugInfo, - ) -> Result; + ) -> Result, ValidationError>; /// Access the axis-aligned bounding box of a shape /// @@ -89,8 +91,9 @@ macro_rules! dispatch { dispatch! { to_shape( + config: &validation::Config, tolerance: Tolerance, debug_info: &mut DebugInfo, - ) -> Result; + ) -> Result, ValidationError>; bounding_volume() -> Aabb<3>; } diff --git a/crates/fj-operations/src/shape_processor.rs b/crates/fj-operations/src/shape_processor.rs index d3693da849..23e1af55e3 100644 --- a/crates/fj-operations/src/shape_processor.rs +++ b/crates/fj-operations/src/shape_processor.rs @@ -3,7 +3,7 @@ use fj_interop::{debug::DebugInfo, mesh::Mesh}; use fj_kernel::{ algorithms::{triangulate, InvalidTolerance, Tolerance}, - shape::ValidationError, + validation::{self, ValidationError}, }; use fj_math::{Aabb, Point, Scalar}; @@ -38,12 +38,10 @@ impl ShapeProcessor { Some(user_defined_tolerance) => user_defined_tolerance, }; + let config = validation::Config::default(); let mut debug_info = DebugInfo::new(); - let mesh = triangulate( - shape.to_shape(tolerance, &mut debug_info)?, - tolerance, - &mut debug_info, - ); + let shape = shape.to_shape(&config, tolerance, &mut debug_info)?; + let mesh = triangulate(shape.into_inner(), tolerance, &mut debug_info); Ok(ProcessedShape { aabb, diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index f9f24256c3..387ae08cab 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -2,7 +2,8 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::Tolerance, objects::{Face, Surface}, - shape::{Shape, ValidationError}, + shape::Shape, + validation::{self, validate, Validated, ValidationError}, }; use fj_math::{Aabb, Point}; @@ -11,9 +12,10 @@ use super::ToShape; impl ToShape for fj::Sketch { fn to_shape( &self, + config: &validation::Config, _: Tolerance, _: &mut DebugInfo, - ) -> Result { + ) -> Result, ValidationError> { let mut shape = Shape::new(); let surface = Surface::xy_plane(); @@ -24,6 +26,8 @@ impl ToShape for fj::Sketch { .with_color(self.color()) .build()?; + let shape = validate(shape, config)?; + Ok(shape) } diff --git a/crates/fj-operations/src/sweep.rs b/crates/fj-operations/src/sweep.rs index ca5f05df65..36aca4e15a 100644 --- a/crates/fj-operations/src/sweep.rs +++ b/crates/fj-operations/src/sweep.rs @@ -1,7 +1,8 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::{sweep_shape, Tolerance}, - shape::{Shape, ValidationError}, + shape::Shape, + validation::{self, validate, Validated, ValidationError}, }; use fj_math::{Aabb, Vector}; @@ -10,15 +11,18 @@ use super::ToShape; impl ToShape for fj::Sweep { fn to_shape( &self, + config: &validation::Config, tolerance: Tolerance, debug_info: &mut DebugInfo, - ) -> Result { - sweep_shape( - self.shape().to_shape(tolerance, debug_info)?, - Vector::from(self.path()), - tolerance, - self.shape().color(), - ) + ) -> Result, ValidationError> { + let shape = self.shape().to_shape(config, tolerance, debug_info)?; + let path = Vector::from(self.path()); + let color = self.shape().color(); + + let swept = sweep_shape(shape.into_inner(), path, tolerance, color)?; + let swept = validate(swept, config)?; + + Ok(swept) } fn bounding_volume(&self) -> Aabb<3> { diff --git a/crates/fj-operations/src/transform.rs b/crates/fj-operations/src/transform.rs index 68b9c413c7..6b0b530f8c 100644 --- a/crates/fj-operations/src/transform.rs +++ b/crates/fj-operations/src/transform.rs @@ -1,7 +1,8 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::{transform_shape, Tolerance}, - shape::{Shape, ValidationError}, + shape::Shape, + validation::{self, validate, Validated, ValidationError}, }; use fj_math::{Aabb, Transform, Vector}; @@ -10,13 +11,17 @@ use super::ToShape; impl ToShape for fj::Transform { fn to_shape( &self, + config: &validation::Config, tolerance: Tolerance, debug_info: &mut DebugInfo, - ) -> Result { - let mut shape = self.shape.to_shape(tolerance, debug_info)?; + ) -> Result, ValidationError> { + let shape = self.shape.to_shape(config, tolerance, debug_info)?; + let mut shape = shape.into_inner(); + let transform = transform(self); transform_shape(&mut shape, &transform)?; + let shape = validate(shape, config)?; Ok(shape) }