diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index dfe033f8d..e52bd5db5 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -37,8 +37,8 @@ impl Sweep for (&HalfEdge, &Handle, &Surface, Option) { [ Some(edge.global_form().clone()), Some(edge_up), - Some(edge_down), None, + Some(edge_down), ], ) }; diff --git a/crates/fj-kernel/src/algorithms/sweep/mod.rs b/crates/fj-kernel/src/algorithms/sweep/mod.rs index e2e98704a..6cdc14430 100644 --- a/crates/fj-kernel/src/algorithms/sweep/mod.rs +++ b/crates/fj-kernel/src/algorithms/sweep/mod.rs @@ -11,7 +11,7 @@ use std::collections::BTreeMap; use fj_math::Vector; use crate::{ - objects::{Objects, Vertex}, + objects::{GlobalEdge, Objects, Vertex}, services::Service, storage::{Handle, ObjectId}, }; @@ -47,4 +47,6 @@ pub trait Sweep: Sized { pub struct SweepCache { /// Cache for global vertices pub global_vertex: BTreeMap>, + /// Cache for global edges + pub global_edge: BTreeMap>, } diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index 868f8e9ef..d2f86193c 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -26,7 +26,11 @@ impl Sweep for Handle { .clone(); let vertices = [a, b]; - let global_edge = GlobalEdge::new().insert(objects); + let global_edge = cache + .global_edge + .entry(self.id()) + .or_insert_with(|| GlobalEdge::new().insert(objects)) + .clone(); // The vertices of the returned `GlobalEdge` are in normalized order, // which means the order can't be relied upon by the caller. Return the diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 2f5a8cbb1..92fc63cd8 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -44,9 +44,10 @@ impl CycleBuilder { let half_edges = edges .into_iter() .circular_tuple_windows() - .map(|((prev, _, _), (_, curve, boundary))| { + .map(|((prev, _, _), (half_edge, curve, boundary))| { HalfEdgeBuilder::new(curve, boundary) .with_start_vertex(prev.start_vertex().clone()) + .with_global_form(half_edge.global_form().clone()) }) .collect(); diff --git a/crates/fj-kernel/src/objects/full/face.rs b/crates/fj-kernel/src/objects/full/face.rs index 95738ca87..1b22a0962 100644 --- a/crates/fj-kernel/src/objects/full/face.rs +++ b/crates/fj-kernel/src/objects/full/face.rs @@ -75,7 +75,7 @@ impl Face { self.interiors.iter() } - /// Access all cycles of the face + /// Access all cycles of the face (both exterior and interior) pub fn all_cycles(&self) -> impl Iterator> + '_ { [self.exterior()].into_iter().chain(self.interiors()) } diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index bc2a96f31..7b638f438 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -42,6 +42,7 @@ pub enum CycleValidationError { /// The half-edge half_edges: Box<(HalfEdge, HalfEdge)>, }, + /// [`Cycle`]'s should have at least one `HalfEdge` #[error("Expected at least one `HalfEdge`\n")] NotEnoughHalfEdges, } @@ -94,6 +95,7 @@ impl CycleValidationError { mod tests { use crate::{ + assert_contains_err, builder::{CycleBuilder, HalfEdgeBuilder}, objects::Cycle, services::Services, @@ -120,21 +122,19 @@ mod tests { .add_half_edge(second) .build(&mut services.objects) }; - assert!(matches!( - disconnected.validate_and_return_first_error(), - Err(ValidationError::Cycle( + + assert_contains_err!( + disconnected, + ValidationError::Cycle( CycleValidationError::HalfEdgesDisconnected { .. } - )) - )); + ) + ); let empty = Cycle::new([]); - assert!(matches!( - empty.validate_and_return_first_error(), - Err(ValidationError::Cycle( - CycleValidationError::NotEnoughHalfEdges - )) - )); - + assert_contains_err!( + empty, + ValidationError::Cycle(CycleValidationError::NotEnoughHalfEdges) + ); Ok(()) } } diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 0f41f6629..e5399a7ce 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -76,6 +76,7 @@ mod tests { use fj_math::Point; use crate::{ + assert_contains_err, builder::HalfEdgeBuilder, objects::HalfEdge, services::Services, @@ -100,12 +101,12 @@ mod tests { }; valid.validate_and_return_first_error()?; - assert!(matches!( - invalid.validate_and_return_first_error(), - Err(ValidationError::HalfEdge( + assert_contains_err!( + invalid, + ValidationError::HalfEdge( HalfEdgeValidationError::VerticesAreCoincident { .. } - )) - )); + ) + ); Ok(()) } diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 556725030..bf4857d27 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -72,6 +72,7 @@ impl FaceValidationError { mod tests { use crate::{ algorithms::reverse::Reverse, + assert_contains_err, builder::{CycleBuilder, FaceBuilder}, objects::Face, services::Services, @@ -110,12 +111,12 @@ mod tests { }; valid.validate_and_return_first_error()?; - assert!(matches!( - invalid.validate_and_return_first_error(), - Err(ValidationError::Face( + assert_contains_err!( + invalid, + ValidationError::Face( FaceValidationError::InvalidInteriorWinding { .. } - )) - )); + ) + ); Ok(()) } diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 5a0dcfc84..7d155d3d3 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -9,13 +9,29 @@ mod solid; mod surface; mod vertex; -use self::cycle::CycleValidationError; -pub use self::{edge::HalfEdgeValidationError, face::FaceValidationError}; +pub use self::{ + cycle::CycleValidationError, edge::HalfEdgeValidationError, + face::FaceValidationError, shell::ShellValidationError, + solid::SolidValidationError, +}; use std::convert::Infallible; use fj_math::Scalar; +/// Assert that some object has a validation error which matches a specifc pattern. +/// This is preferred to matching on [`Validate::validate_and_return_first_error`], since usually we don't care about the order. +#[macro_export] +macro_rules! assert_contains_err { + ($o:tt,$p:pat) => { + assert!({ + let mut errors = Vec::new(); + $o.validate(&mut errors); + errors.iter().any(|e| matches!(e, $p)) + }) + }; +} + /// Validate an object /// /// This trait is used automatically when inserting an object into a store. @@ -92,6 +108,14 @@ pub enum ValidationError { /// `Cycle` validation error #[error("`Cycle` validation error:\n{0}")] Cycle(#[from] CycleValidationError), + + /// `Shell` validation error + #[error("`Shell` validation error:\n{0}")] + Shell(#[from] ShellValidationError), + + /// `Solid` validation error + #[error("`Solid` validation error:\n{0}")] + Solid(#[from] SolidValidationError), } impl From for ValidationError { diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index f6603ad55..109c6f452 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -1,12 +1,249 @@ -use crate::objects::Shell; +use std::{collections::HashMap, iter::repeat}; + +use fj_math::{Point, Scalar}; + +use crate::{ + geometry::surface::SurfaceGeometry, + objects::{HalfEdge, Shell}, + storage::{Handle, ObjectId}, +}; use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Shell { fn validate_with_config( &self, + config: &ValidationConfig, + errors: &mut Vec, + ) { + ShellValidationError::validate_edges_coincident(self, config, errors); + ShellValidationError::validate_watertight(self, config, errors); + } +} + +/// [`Shell`] validation failed +#[derive(Clone, Debug, thiserror::Error)] +pub enum ShellValidationError { + /// [`Shell`] contains global_edges not referred to by two half_edges + #[error("Shell is not watertight")] + NotWatertight, + /// [`Shell`] contains half_edges that are coincident, but refer to different global_edges + #[error( + "Shell contains HalfEdges that are coinciendent but refer to different GlobalEdges\n + Edge 1: {0:#?} + Edge 2: {1:#?} + " + )] + CoincidentEdgesNotIdentical(Handle, Handle), + /// [`Shell`] contains half_edges that are identical, but do not coincide + #[error( + "Shell contains HalfEdges that are identical but do not coincide\n + Edge 1: {0:#?} + Edge 2: {1:#?} + " + )] + IdenticalEdgesNotCoincident(Handle, Handle), +} + +/// Sample two edges at various (currently 3) points in 3D along them. +/// +/// Returns an [`Iterator`] of the distance at each sample. +fn distances( + config: &ValidationConfig, + (edge1, surface1): (Handle, SurfaceGeometry), + (edge2, surface2): (Handle, SurfaceGeometry), +) -> impl Iterator { + fn sample( + percent: f64, + (edge, surface): (&Handle, SurfaceGeometry), + ) -> Point<3> { + let boundary = edge.boundary(); + let path_coords = boundary[0] + (boundary[1] - boundary[0]) * percent; + let surface_coords = edge.curve().point_from_path_coords(path_coords); + surface.point_from_surface_coords(surface_coords) + } + + // Check whether start positions do not match. If they don't treat second edge as flipped + let flip = sample(0.0, (&edge1, surface1)) + .distance_to(&sample(0.0, (&edge2, surface2))) + > config.identical_max_distance; + + // Three samples (start, middle, end), are enough to detect weather lines + // and circles match. If we were to add more complicated curves, this might + // need to change. + let sample_count = 3; + let step = 1.0 / (sample_count as f64 - 1.0); + + let mut distances = Vec::new(); + for i in 0..sample_count { + let percent = i as f64 * step; + let sample1 = sample(percent, (&edge1, surface1)); + let sample2 = sample( + if flip { 1.0 - percent } else { percent }, + (&edge2, surface2), + ); + distances.push(sample1.distance_to(&sample2)) + } + distances.into_iter() +} + +impl ShellValidationError { + fn validate_edges_coincident( + shell: &Shell, + config: &ValidationConfig, + errors: &mut Vec, + ) { + let faces: Vec<(Handle, SurfaceGeometry)> = shell + .faces() + .into_iter() + .flat_map(|face| { + face.all_cycles() + .flat_map(|cycle| cycle.half_edges().cloned()) + .zip(repeat(face.surface().geometry())) + }) + .collect(); + + // This is O(N^2) which isn't great, but we can't use a HashMap since we + // need to deal with float inaccuracies. Maybe we could use some smarter + // data-structure like an octree. + for edge in &faces { + for other_edge in &faces { + let id = edge.0.global_form().id(); + let other_id = other_edge.0.global_form().id(); + let identical = id == other_id; + match identical { + true => { + // All points on identical curves should be within + // identical_max_distance, so we shouldn't have any + // greater than the max + if distances(config, edge.clone(), other_edge.clone()) + .any(|d| d > config.identical_max_distance) + { + errors.push( + Self::IdenticalEdgesNotCoincident( + edge.0.clone(), + other_edge.0.clone(), + ) + .into(), + ) + } + } + false => { + // If all points on distinct curves are within + // distinct_min_distance, that's a problem. + if distances(config, edge.clone(), other_edge.clone()) + .all(|d| d < config.distinct_min_distance) + { + errors.push( + Self::CoincidentEdgesNotIdentical( + edge.0.clone(), + other_edge.0.clone(), + ) + .into(), + ) + } + } + } + } + } + } + + fn validate_watertight( + shell: &Shell, _: &ValidationConfig, - _: &mut Vec, + errors: &mut Vec, ) { + let faces = shell.faces(); + let mut half_edge_to_faces: HashMap = HashMap::new(); + for face in faces { + for cycle in face.all_cycles() { + for half_edge in cycle.half_edges() { + let id = half_edge.global_form().id(); + let entry = half_edge_to_faces.entry(id); + *entry.or_insert(0) += 1; + } + } + } + + // Each global edge should have exactly two half edges that are part of the shell + if half_edge_to_faces.iter().any(|(_, c)| *c != 2) { + errors.push(Self::NotWatertight.into()) + } + } +} + +#[cfg(test)] +mod tests { + use crate::assert_contains_err; + use crate::insert::Insert; + + use crate::validate::shell::ShellValidationError; + use crate::{ + builder::{CycleBuilder, FaceBuilder}, + objects::Shell, + services::Services, + validate::{Validate, ValidationError}, + }; + + #[test] + fn coincident_not_identical() -> anyhow::Result<()> { + let mut services = Services::new(); + let invalid = { + let face1 = FaceBuilder::new(services.objects.surfaces.xy_plane()) + .with_exterior(CycleBuilder::polygon([ + [0., 0.], + [0., 1.], + [1., 1.], + [1., 0.], + ])) + .build(&mut services.objects) + .insert(&mut services.objects); + + let face2 = FaceBuilder::new(services.objects.surfaces.xz_plane()) + .with_exterior(CycleBuilder::polygon([ + [0., 0.], + [0., 1.], + [1., 1.], + [1., 0.], + ])) + .build(&mut services.objects) + .insert(&mut services.objects); + + Shell::new([face1, face2]) + }; + + assert_contains_err!( + invalid, + ValidationError::Shell( + ShellValidationError::CoincidentEdgesNotIdentical(..) + ) + ); + + Ok(()) + } + #[test] + fn shell_not_watertight() -> anyhow::Result<()> { + let mut services = Services::new(); + + let invalid = { + // Shell with single face is not watertight + let face = FaceBuilder::new(services.objects.surfaces.xy_plane()) + .with_exterior(CycleBuilder::polygon([ + [0., 0.], + [0., 1.], + [1., 1.], + [1., 0.], + ])) + .build(&mut services.objects) + .insert(&mut services.objects); + Shell::new([face]) + }; + + assert_contains_err!( + invalid, + ValidationError::Shell(ShellValidationError::NotWatertight) + ); + + Ok(()) } } diff --git a/crates/fj-kernel/src/validate/solid.rs b/crates/fj-kernel/src/validate/solid.rs index e2982bd6a..b5fe04cc2 100644 --- a/crates/fj-kernel/src/validate/solid.rs +++ b/crates/fj-kernel/src/validate/solid.rs @@ -1,12 +1,99 @@ -use crate::objects::Solid; +use std::iter::repeat; + +use crate::{ + objects::{Solid, Vertex}, + storage::Handle, +}; +use fj_math::Point; use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Solid { fn validate_with_config( &self, - _: &ValidationConfig, - _: &mut Vec, + config: &ValidationConfig, + errors: &mut Vec, ) { + SolidValidationError::check_vertices(self, config, errors) + } +} + +/// [`Solid`] validation failed +#[derive(Clone, Debug, thiserror::Error)] +pub enum SolidValidationError { + /// [`Solid`] contains vertices that are coincident, but not identical + #[error( + "Solid contains Vertices that are coinciendent but not identical\n + Vertex 1: {:#?} {:#?} + Vertex 2: {:#?} {:#?} + ", .0[0].0, .0[0].1,.0[1].0,.0[1].1 + )] + DistinctVertsCoincide([(Handle, Point<3>); 2]), + + /// [`Solid`] contains vertices that are identical, but do not coincide + #[error( + "Solid contains Vertices that are identical but do not coincide\n + Vertex 1: {:#?} {:#?} + Vertex 2: {:#?} {:#?} + ", .0[0].0, .0[0].1,.0[1].0,.0[1].1 + )] + IdenticalVertsNotCoincident([(Handle, Point<3>); 2]), +} + +impl SolidValidationError { + fn check_vertices( + solid: &Solid, + config: &ValidationConfig, + errors: &mut Vec, + ) { + let vertices: Vec<(Point<3>, Handle)> = solid + .shells() + .flat_map(|s| s.faces()) + .flat_map(|face| { + face.all_cycles() + .flat_map(|cycle| cycle.half_edges().cloned()) + .zip(repeat(face.surface().geometry())) + }) + .map(|(h, s)| { + ( + s.point_from_surface_coords(h.start_position()), + h.start_vertex().clone(), + ) + }) + .collect(); + + // This is O(N^2) which isn't great, but we can't use a HashMap since we + // need to deal with float inaccuracies. Maybe we could use some smarter + // data-structure like an octree. + for a in &vertices { + for b in &vertices { + match a.1.id() == b.1.id() { + true => { + if a.0.distance_to(&b.0) > config.identical_max_distance + { + errors.push( + Self::IdenticalVertsNotCoincident([ + (a.1.clone(), a.0), + (b.1.clone(), b.0), + ]) + .into(), + ) + } + } + false => { + if a.0.distance_to(&b.0) < config.distinct_min_distance + { + errors.push( + Self::DistinctVertsCoincide([ + (a.1.clone(), a.0), + (b.1.clone(), b.0), + ]) + .into(), + ) + } + } + } + } + } } }