From 61f246213fd3df76277601d9e705cfc6a8a2f9bd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 27 Mar 2024 13:30:27 +0100 Subject: [PATCH 1/3] Provide surface to `Cycle::winding` --- crates/fj-core/src/operations/sweep/sketch.rs | 2 +- crates/fj-core/src/topology/objects/cycle.rs | 4 +++- crates/fj-core/src/topology/objects/face.rs | 2 +- crates/fj-core/src/validate/sketch.rs | 6 ++++-- crates/fj-core/src/validation/checks/face_winding.rs | 4 ++-- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/sketch.rs b/crates/fj-core/src/operations/sweep/sketch.rs index 446fa1a90..e2888db5a 100644 --- a/crates/fj-core/src/operations/sweep/sketch.rs +++ b/crates/fj-core/src/operations/sweep/sketch.rs @@ -42,7 +42,7 @@ impl SweepSketch for Sketch { // clockwise. Let's check that real quick. assert!(region .exterior() - .winding(&core.layers.geometry) + .winding(&core.layers.geometry, self.surface()) .is_ccw()); let is_negative_sweep = { diff --git a/crates/fj-core/src/topology/objects/cycle.rs b/crates/fj-core/src/topology/objects/cycle.rs index ea55d8f8f..032a3e4b8 100644 --- a/crates/fj-core/src/topology/objects/cycle.rs +++ b/crates/fj-core/src/topology/objects/cycle.rs @@ -6,6 +6,8 @@ use crate::{ topology::{HalfEdge, ObjectSet}, }; +use super::surface::Surface; + /// A cycle of connected edges #[derive(Clone, Debug)] pub struct Cycle { @@ -29,7 +31,7 @@ impl Cycle { /// Please note that this is not *the* winding of the cycle, only one of the /// two possible windings, depending on the direction you look at the /// surface that the cycle is defined on from. - pub fn winding(&self, geometry: &Geometry) -> Winding { + pub fn winding(&self, geometry: &Geometry, _: &Handle) -> Winding { // The cycle could be made up of one or two circles. If that is the // case, the winding of the cycle is determined by the winding of the // first circle. diff --git a/crates/fj-core/src/topology/objects/face.rs b/crates/fj-core/src/topology/objects/face.rs index 919d635bf..e1e6ab883 100644 --- a/crates/fj-core/src/topology/objects/face.rs +++ b/crates/fj-core/src/topology/objects/face.rs @@ -63,7 +63,7 @@ impl Face { /// back sides. The front side is the side, where the face's exterior cycle /// is wound counter-clockwise. pub fn coord_handedness(&self, geometry: &Geometry) -> Handedness { - match self.region.exterior().winding(geometry) { + match self.region.exterior().winding(geometry, self.surface()) { Winding::Ccw => Handedness::RightHanded, Winding::Cw => Handedness::LeftHanded, } diff --git a/crates/fj-core/src/validate/sketch.rs b/crates/fj-core/src/validate/sketch.rs index 5e536c5f7..150a66833 100644 --- a/crates/fj-core/src/validate/sketch.rs +++ b/crates/fj-core/src/validate/sketch.rs @@ -77,7 +77,7 @@ impl SketchValidationError { ) { sketch.regions().iter().for_each(|region| { let cycle = region.exterior(); - if cycle.winding(geometry) == Winding::Cw { + if cycle.winding(geometry, sketch.surface()) == Winding::Cw { errors.push(ValidationError::Sketch( SketchValidationError::ClockwiseExteriorCycle { cycle: cycle.clone(), @@ -97,7 +97,9 @@ impl SketchValidationError { region .interiors() .iter() - .filter(|interior| interior.winding(geometry) == Winding::Ccw) + .filter(|interior| { + interior.winding(geometry, sketch.surface()) == Winding::Ccw + }) .for_each(|cycle| { errors.push(ValidationError::Sketch( SketchValidationError::CounterClockwiseInteriorCycle { diff --git a/crates/fj-core/src/validation/checks/face_winding.rs b/crates/fj-core/src/validation/checks/face_winding.rs index d60f4cbc6..ee049d1fe 100644 --- a/crates/fj-core/src/validation/checks/face_winding.rs +++ b/crates/fj-core/src/validation/checks/face_winding.rs @@ -53,8 +53,8 @@ impl ValidationCheck for InteriorCycleHasInvalidWinding { return None; } - let exterior_winding = exterior.winding(geometry); - let interior_winding = interior.winding(geometry); + let exterior_winding = exterior.winding(geometry, object.surface()); + let interior_winding = interior.winding(geometry, object.surface()); if exterior_winding == interior_winding { return Some(InteriorCycleHasInvalidWinding { From 30549bc82e74023a31cfab93b7c0d8a8b95b2e74 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Apr 2024 14:06:12 +0200 Subject: [PATCH 2/3] Provide surface to private method This prepares for a follow-on change, as the surface will be needed there shortly. --- .../src/validation/checks/half_edge_connection.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/fj-core/src/validation/checks/half_edge_connection.rs b/crates/fj-core/src/validation/checks/half_edge_connection.rs index f38fbcf37..64166c553 100644 --- a/crates/fj-core/src/validation/checks/half_edge_connection.rs +++ b/crates/fj-core/src/validation/checks/half_edge_connection.rs @@ -3,7 +3,7 @@ use fj_math::{Point, Scalar}; use crate::{ geometry::Geometry, storage::Handle, - topology::{Cycle, Face, HalfEdge, Region, Sketch}, + topology::{Cycle, Face, HalfEdge, Region, Sketch, Surface}, validation::{validation_check::ValidationCheck, ValidationConfig}, }; @@ -63,7 +63,7 @@ impl ValidationCheck for AdjacentHalfEdgesNotConnected { geometry: &'r Geometry, config: &'r ValidationConfig, ) -> impl Iterator + 'r { - check_region(object.region(), geometry, config) + check_region(object.region(), object.surface(), geometry, config) } } @@ -73,26 +73,27 @@ impl ValidationCheck for AdjacentHalfEdgesNotConnected { geometry: &'r Geometry, config: &'r ValidationConfig, ) -> impl Iterator + 'r { - object - .regions() - .iter() - .flat_map(|region| check_region(region, geometry, config)) + object.regions().iter().flat_map(|region| { + check_region(region, object.surface(), geometry, config) + }) } } fn check_region<'r>( region: &'r Region, + surface: &'r Handle, geometry: &'r Geometry, config: &'r ValidationConfig, ) -> impl Iterator + 'r { [region.exterior()] .into_iter() .chain(region.interiors()) - .flat_map(|cycle| check_cycle(cycle, geometry, config)) + .flat_map(|cycle| check_cycle(cycle, surface, geometry, config)) } fn check_cycle<'r>( cycle: &'r Cycle, + _: &'r Handle, geometry: &'r Geometry, config: &'r ValidationConfig, ) -> impl Iterator + 'r { From f7513d0646edf7fdd00eb1fe6e2d28980bfd31c2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Apr 2024 14:07:05 +0200 Subject: [PATCH 3/3] Don't use `self.path` in `HalfEdgeGeom` This prepares for moving the path to `CurveGeom`. --- crates/fj-core/src/algorithms/approx/edge.rs | 9 +++++++- crates/fj-core/src/geometry/half_edge.rs | 4 ++-- crates/fj-core/src/operations/split/face.rs | 22 +++++++++++++++++-- crates/fj-core/src/topology/objects/cycle.rs | 15 +++++++++++-- crates/fj-core/src/validate/solid.rs | 18 +++++++++++---- .../checks/curve_geometry_mismatch.rs | 3 +++ .../validation/checks/half_edge_connection.rs | 16 +++++++++++--- 7 files changed, 73 insertions(+), 14 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 6cff4cecb..19fc1d799 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -30,7 +30,14 @@ impl Approx for (&Handle, &Handle) { let tolerance = tolerance.into(); let start_position_surface = - geometry.of_half_edge(half_edge).start_position(); + geometry.of_half_edge(half_edge).start_position( + &geometry + .of_curve(half_edge.curve()) + .unwrap() + .local_on(surface) + .unwrap() + .path, + ); let start_position = match cache.start_position.get(half_edge.start_vertex()) { Some(position) => position, diff --git a/crates/fj-core/src/geometry/half_edge.rs b/crates/fj-core/src/geometry/half_edge.rs index 0823d6483..0ea12b321 100644 --- a/crates/fj-core/src/geometry/half_edge.rs +++ b/crates/fj-core/src/geometry/half_edge.rs @@ -47,12 +47,12 @@ impl HalfEdgeGeom { } /// Compute the surface position where the half-edge starts - pub fn start_position(&self) -> Point<2> { + pub fn start_position(&self, path: &SurfacePath) -> Point<2> { // Computing the surface position from the curve position is fine. // `HalfEdge` "owns" its start position. There is no competing code that // could compute the surface position from slightly different data. let [start, _] = self.boundary.inner; - self.path.point_from_path_coords(start) + path.point_from_path_coords(start) } } diff --git a/crates/fj-core/src/operations/split/face.rs b/crates/fj-core/src/operations/split/face.rs index 8cfd25d87..b46e241c6 100644 --- a/crates/fj-core/src/operations/split/face.rs +++ b/crates/fj-core/src/operations/split/face.rs @@ -105,8 +105,26 @@ impl SplitFace for Shell { let dividing_half_edge_a_to_d = { let half_edge = HalfEdge::line_segment( [ - core.layers.geometry.of_half_edge(&b).start_position(), - core.layers.geometry.of_half_edge(&d).start_position(), + core.layers.geometry.of_half_edge(&b).start_position( + &core + .layers + .geometry + .of_curve(b.curve()) + .unwrap() + .local_on(face.surface()) + .unwrap() + .path, + ), + core.layers.geometry.of_half_edge(&d).start_position( + &core + .layers + .geometry + .of_curve(d.curve()) + .unwrap() + .local_on(face.surface()) + .unwrap() + .path, + ), ], None, face.surface().clone(), diff --git a/crates/fj-core/src/topology/objects/cycle.rs b/crates/fj-core/src/topology/objects/cycle.rs index 032a3e4b8..019101d37 100644 --- a/crates/fj-core/src/topology/objects/cycle.rs +++ b/crates/fj-core/src/topology/objects/cycle.rs @@ -31,7 +31,11 @@ impl Cycle { /// Please note that this is not *the* winding of the cycle, only one of the /// two possible windings, depending on the direction you look at the /// surface that the cycle is defined on from. - pub fn winding(&self, geometry: &Geometry, _: &Handle) -> Winding { + pub fn winding( + &self, + geometry: &Geometry, + surface: &Handle, + ) -> Winding { // The cycle could be made up of one or two circles. If that is the // case, the winding of the cycle is determined by the winding of the // first circle. @@ -70,7 +74,14 @@ impl Cycle { for (a, b) in self.half_edges().pairs() { let [a, b] = [a, b].map(|half_edge| { - geometry.of_half_edge(half_edge).start_position() + geometry.of_half_edge(half_edge).start_position( + &geometry + .of_curve(half_edge.curve()) + .unwrap() + .local_on(surface) + .unwrap() + .path, + ) }); sum += (b.u - a.u) * (b.v + a.v); diff --git a/crates/fj-core/src/validate/solid.rs b/crates/fj-core/src/validate/solid.rs index 7f392bb6c..f51191aa7 100644 --- a/crates/fj-core/src/validate/solid.rs +++ b/crates/fj-core/src/validate/solid.rs @@ -106,13 +106,23 @@ impl SolidValidationError { .flat_map(|cycle| cycle.half_edges().iter().cloned()) .zip(repeat(face.surface())) }) - .map(|(h, s)| { - ( + .filter_map(|(h, s)| { + let Some(local_curve_geometry) = + geometry.of_curve(h.curve()).unwrap().local_on(s) + else { + // If the curve geometry has no local definition, + // there's nothing we can check. + return None; + }; + + Some(( geometry.of_surface(s).point_from_surface_coords( - geometry.of_half_edge(&h).start_position(), + geometry + .of_half_edge(&h) + .start_position(&local_curve_geometry.path), ), h.start_vertex().clone(), - ) + )) }) .collect(); diff --git a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs index 6b23093fb..b0c96b1ef 100644 --- a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs +++ b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs @@ -201,6 +201,9 @@ mod tests { ) .is_err()); + // Ignore remaining validation errors. + let _ = core.layers.validation.take_errors(); + Ok(()) } } diff --git a/crates/fj-core/src/validation/checks/half_edge_connection.rs b/crates/fj-core/src/validation/checks/half_edge_connection.rs index 64166c553..636ca1b56 100644 --- a/crates/fj-core/src/validation/checks/half_edge_connection.rs +++ b/crates/fj-core/src/validation/checks/half_edge_connection.rs @@ -93,7 +93,7 @@ fn check_region<'r>( fn check_cycle<'r>( cycle: &'r Cycle, - _: &'r Handle, + surface: &'r Handle, geometry: &'r Geometry, config: &'r ValidationConfig, ) -> impl Iterator + 'r { @@ -105,8 +105,18 @@ fn check_cycle<'r>( .path .point_from_path_coords(end) }; - let start_pos_of_second_half_edge = - geometry.of_half_edge(second).start_position(); + + let Some(local_curve_geometry) = + geometry.of_curve(second.curve()).unwrap().local_on(surface) + else { + // If the curve geometry is not defined for our local surface, + // there's nothing we can check. + return None; + }; + + let start_pos_of_second_half_edge = geometry + .of_half_edge(second) + .start_position(&local_curve_geometry.path); let distance_between_positions = (end_pos_of_first_half_edge - start_pos_of_second_half_edge)