diff --git a/fj-kernel/src/shape/mod.rs b/fj-kernel/src/shape/mod.rs index 18cf72a9f..6abc0ff98 100644 --- a/fj-kernel/src/shape/mod.rs +++ b/fj-kernel/src/shape/mod.rs @@ -13,7 +13,7 @@ pub use self::{ handle::Handle, iter::Iter, topology::Topology, - validate::{ValidationError, ValidationResult}, + validate::{StructuralIssues, ValidationError, ValidationResult}, }; use fj_math::{Point, Scalar}; diff --git a/fj-kernel/src/shape/topology.rs b/fj-kernel/src/shape/topology.rs index aa8779edb..9ec413bc2 100644 --- a/fj-kernel/src/shape/topology.rs +++ b/fj-kernel/src/shape/topology.rs @@ -9,7 +9,8 @@ use crate::{ use super::{ handle::{Handle, Storage}, - Cycles, Edges, Geometry, Iter, ValidationError, ValidationResult, Vertices, + Cycles, Edges, Geometry, Iter, StructuralIssues, ValidationError, + ValidationResult, Vertices, }; /// The vertices of a shape @@ -48,7 +49,7 @@ impl Topology<'_> { /// does. See documentation of [`crate::kernel`] for some context on that. pub fn add_vertex(&mut self, vertex: Vertex) -> ValidationResult { if !self.geometry.points.contains(vertex.point.storage()) { - return Err(ValidationError::Structural(())); + return Err(StructuralIssues::default().into()); } for existing in &*self.vertices { let distance = @@ -96,10 +97,12 @@ impl Topology<'_> { } if missing_curve.is_some() || !missing_vertices.is_empty() { - return Err(ValidationError::Structural(( + return Err(StructuralIssues { missing_curve, missing_vertices, - ))); + ..StructuralIssues::default() + } + .into()); } let storage = Storage::new(edge); @@ -162,7 +165,11 @@ impl Topology<'_> { } if !missing_edges.is_empty() { - return Err(ValidationError::Structural(missing_edges)); + return Err(StructuralIssues { + missing_edges, + ..StructuralIssues::default() + } + .into()); } let storage = Storage::new(cycle); @@ -198,10 +205,12 @@ impl Topology<'_> { } if missing_surface.is_some() || !missing_cycles.is_empty() { - return Err(ValidationError::Structural(( + return Err(StructuralIssues { missing_surface, missing_cycles, - ))); + ..StructuralIssues::default() + } + .into()); } } @@ -267,7 +276,7 @@ mod tests { // Should fail, as `point` is not part of the shape. let point = other.geometry().add_point(Point::from([1., 0., 0.])); let result = shape.topology().add_vertex(Vertex { point }); - assert!(matches!(result, Err(ValidationError::Structural(())))); + assert!(matches!(result, Err(ValidationError::Structural(_)))); // `point` is too close to the original point. `assert!` is commented, // because that only causes a warning to be logged right now. diff --git a/fj-kernel/src/shape/validate.rs b/fj-kernel/src/shape/validate.rs index 167e37089..3739ca0c2 100644 --- a/fj-kernel/src/shape/validate.rs +++ b/fj-kernel/src/shape/validate.rs @@ -2,23 +2,23 @@ use std::collections::HashSet; use crate::{ geometry::{Curve, Surface}, - topology::{Cycle, Edge, Face, Vertex}, + topology::{Cycle, Edge, Vertex}, }; use super::Handle; /// Returned by the various `add_` methods of the [`Shape`] API -pub type ValidationResult = Result, ValidationError>; +pub type ValidationResult = Result, ValidationError>; /// An error that can occur during a validation #[derive(Debug, thiserror::Error)] -pub enum ValidationError { +pub enum ValidationError { /// 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(T::Structural), + Structural(StructuralIssues), /// Uniqueness validation failed /// @@ -39,12 +39,12 @@ pub enum ValidationError { Geometric, } -impl ValidationError { +impl ValidationError { /// Indicate whether validation found a missing curve #[cfg(test)] pub fn missing_curve(&self, curve: &Handle) -> bool { - if let Self::Structural(missing) = self { - return missing.0.as_ref() == Some(curve); + if let Self::Structural(StructuralIssues { missing_curve, .. }) = self { + return missing_curve.as_ref() == Some(curve); } false @@ -53,32 +53,34 @@ impl ValidationError { /// Indicate whether validation found a missing vertex #[cfg(test)] pub fn missing_vertex(&self, vertex: &Handle) -> bool { - if let Self::Structural(missing) = self { - return missing.1.contains(vertex); + if let Self::Structural(StructuralIssues { + missing_vertices, .. + }) = self + { + return missing_vertices.contains(vertex); } false } -} -impl ValidationError { /// Indicate whether validation found a missing edge #[cfg(test)] - pub fn missing_edge(&self, vertex: &Handle) -> bool { - if let Self::Structural(missing) = self { - return missing.contains(vertex); + pub fn missing_edge(&self, edge: &Handle) -> bool { + if let Self::Structural(StructuralIssues { missing_edges, .. }) = self { + return missing_edges.contains(edge); } false } -} -impl ValidationError { /// Indicate whether validation found a missing surface #[cfg(test)] pub fn missing_surface(&self, surface: &Handle) -> bool { - if let Self::Structural(missing) = self { - return missing.0.as_ref() == Some(surface); + if let Self::Structural(StructuralIssues { + missing_surface, .. + }) = self + { + return missing_surface.as_ref() == Some(surface); } false @@ -87,33 +89,38 @@ impl ValidationError { /// Indicate whether validation found a missing cycle #[cfg(test)] pub fn missing_cycle(&self, cycle: &Handle) -> bool { - if let Self::Structural(missing) = self { - return missing.1.contains(cycle); + if let Self::Structural(StructuralIssues { missing_cycles, .. }) = self + { + return missing_cycles.contains(cycle); } false } } -/// Implemented for topological types, which can be validated -/// -/// Used by [`ValidationError`] to provide context on how validation failed. -pub trait Validatable { - type Structural; +impl From for ValidationError { + fn from(issues: StructuralIssues) -> Self { + Self::Structural(issues) + } } -impl Validatable for Vertex { - type Structural = (); -} +/// Structural issues found during validation +/// +/// Used by [`ValidationError`]. +#[derive(Debug, Default)] +pub struct StructuralIssues { + /// Missing curve found in edge validation + pub missing_curve: Option>, -impl Validatable for Edge { - type Structural = (Option>, HashSet>); -} + /// Missing vertices found in edge validation + pub missing_vertices: HashSet>, -impl Validatable for Cycle { - type Structural = HashSet>; -} + /// Missing edges found in cycle validation + pub missing_edges: HashSet>, + + /// Missing surface found in face validation + pub missing_surface: Option>, -impl Validatable for Face { - type Structural = (Option>, HashSet>); + /// Missing cycles found in cycle validation + pub missing_cycles: HashSet>, }