diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_face.rs b/crates/fj-kernel/src/algorithms/intersect/curve_face.rs index de42b5e60..386ec6de7 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_face.rs @@ -161,8 +161,6 @@ mod tests { fn compute() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut curve = PartialCurve::default(); curve.update_as_line_from_points([[-3., 0.], [-2., 0.]]); let curve = curve.build(&mut services.objects); @@ -184,10 +182,9 @@ mod tests { let face = { let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior .write() .update_as_polygon_from_points(exterior); diff --git a/crates/fj-kernel/src/algorithms/intersect/face_face.rs b/crates/fj-kernel/src/algorithms/intersect/face_face.rs index 904f62630..30b9b2460 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_face.rs @@ -94,10 +94,9 @@ mod tests { ] .map(|surface| { let mut face = PartialFace { - surface: Partial::from(surface.clone()), + surface: Partial::from(surface), ..Default::default() }; - face.exterior.write().surface = Partial::from(surface); face.exterior.write().update_as_polygon_from_points(points); face.build(&mut services.objects) @@ -126,10 +125,9 @@ mod tests { ]; let [a, b] = surfaces.clone().map(|surface| { let mut face = PartialFace { - surface: Partial::from(surface.clone()), + surface: Partial::from(surface), ..Default::default() }; - face.exterior.write().surface = Partial::from(surface); face.exterior.write().update_as_polygon_from_points(points); face.build(&mut services.objects) diff --git a/crates/fj-kernel/src/algorithms/intersect/face_point.rs b/crates/fj-kernel/src/algorithms/intersect/face_point.rs index a44ca8c07..1f13c0a8e 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_point.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_point.rs @@ -148,13 +148,10 @@ mod tests { fn point_is_outside_face() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [1., 1.], @@ -173,13 +170,10 @@ mod tests { fn ray_hits_vertex_while_passing_outside() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -201,13 +195,10 @@ mod tests { fn ray_hits_vertex_at_cycle_seam() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [4., 2.], [0., 4.], @@ -229,13 +220,10 @@ mod tests { fn ray_hits_vertex_while_staying_inside() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -258,13 +246,10 @@ mod tests { fn ray_hits_parallel_edge_and_leaves_face_at_vertex() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -287,13 +272,10 @@ mod tests { fn ray_hits_parallel_edge_and_does_not_leave_face_there() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -317,13 +299,10 @@ mod tests { fn point_is_coincident_with_edge() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 0.], @@ -354,13 +333,10 @@ mod tests { fn point_is_coincident_with_vertex() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [1., 0.], diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs index 44269fbdf..4d90f15c3 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -163,13 +163,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.yz_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -190,13 +187,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.yz_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -220,13 +214,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.yz_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -247,13 +238,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.yz_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -285,13 +273,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.yz_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -323,13 +308,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -352,13 +334,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], diff --git a/crates/fj-kernel/src/algorithms/reverse/cycle.rs b/crates/fj-kernel/src/algorithms/reverse/cycle.rs index fd08da75a..23f517430 100644 --- a/crates/fj-kernel/src/algorithms/reverse/cycle.rs +++ b/crates/fj-kernel/src/algorithms/reverse/cycle.rs @@ -17,6 +17,6 @@ impl Reverse for Handle { edges.reverse(); - Cycle::new(self.surface().clone(), edges).insert(objects) + Cycle::new(edges).insert(objects) } } diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index ca25066c7..4f539b11b 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -38,8 +38,7 @@ impl Sweep for (Handle, &Surface, Color) { (edge.curve().clone(), surface) .sweep_with_cache(path, cache, objects), ); - face.surface = surface.clone(); - face.exterior.write().surface = surface; + face.surface = surface; } // Now we're ready to create the edges. @@ -269,10 +268,7 @@ mod tests { .clone() }; - let mut cycle = PartialCycle { - surface: Partial::from(surface.clone()), - ..Default::default() - }; + let mut cycle = PartialCycle::default(); cycle.half_edges.extend( [bottom, side_up, top, side_down].map(Partial::from_partial), ); diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 51731cbc5..d55d0337a 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -83,8 +83,6 @@ impl Sweep for Handle { top_edges.push(Partial::from(top_edge)); } - top_cycle.write().surface = Partial::from(top_surface.clone()); - top_cycle .write() .connect_to_closed_edges(top_edges, &top_surface.geometry()); @@ -165,8 +163,6 @@ mod tests { let mut face = sketch.add_face(); face.write().surface = Partial::from(surface.clone()); - face.write().exterior.write().surface = - Partial::from(surface.clone()); face.write() .exterior .write() @@ -185,7 +181,6 @@ mod tests { ..Default::default() }; - bottom.exterior.write().surface = Partial::from(surface.clone()); bottom .exterior .write() @@ -200,11 +195,10 @@ mod tests { let surface = surface.clone().translate(UP, &mut services.objects); let mut top = PartialFace { - surface: Partial::from(surface.clone()), + surface: Partial::from(surface), ..Default::default() }; - top.exterior.write().surface = Partial::from(surface); top.exterior.write().update_as_polygon_from_points(TRIANGLE); top.build(&mut services.objects) @@ -246,8 +240,6 @@ mod tests { let mut face = sketch.add_face(); face.write().surface = Partial::from(surface.clone()); - face.write().exterior.write().surface = - Partial::from(surface.clone()); face.write() .exterior .write() @@ -265,11 +257,10 @@ mod tests { surface.clone().translate(DOWN, &mut services.objects); let mut bottom = PartialFace { - surface: Partial::from(surface.clone()), + surface: Partial::from(surface), ..Default::default() }; - bottom.exterior.write().surface = Partial::from(surface); bottom .exterior .write() @@ -286,7 +277,6 @@ mod tests { ..Default::default() }; - top.exterior.write().surface = Partial::from(surface.clone()); top.exterior.write().update_as_polygon_from_points(TRIANGLE); top.build(&mut services.objects) diff --git a/crates/fj-kernel/src/algorithms/transform/cycle.rs b/crates/fj-kernel/src/algorithms/transform/cycle.rs index e56793752..5fbc6eb32 100644 --- a/crates/fj-kernel/src/algorithms/transform/cycle.rs +++ b/crates/fj-kernel/src/algorithms/transform/cycle.rs @@ -14,16 +14,12 @@ impl TransformObject for Cycle { objects: &mut Service, cache: &mut TransformCache, ) -> Self { - let surface = self - .surface() - .clone() - .transform_with_cache(transform, objects, cache); let half_edges = self.half_edges().map(|half_edge| { half_edge .clone() .transform_with_cache(transform, objects, cache) }); - Self::new(surface, half_edges) + Self::new(half_edges) } } diff --git a/crates/fj-kernel/src/algorithms/triangulate/mod.rs b/crates/fj-kernel/src/algorithms/triangulate/mod.rs index 87d29de38..d2a8ddc97 100644 --- a/crates/fj-kernel/src/algorithms/triangulate/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulate/mod.rs @@ -96,13 +96,10 @@ mod tests { let c = [2., 2.]; let d = [0., 1.]; - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface: surface.clone(), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = surface; face.exterior .write() .update_as_polygon_from_points([a, b, c, d]); @@ -144,7 +141,6 @@ mod tests { surface: Partial::from(surface.clone()), ..Default::default() }; - face.exterior.write().surface = Partial::from(surface.clone()); face.exterior .write() .update_as_polygon_from_points([a, b, c, d]); @@ -211,7 +207,6 @@ mod tests { surface: Partial::from(surface.clone()), ..Default::default() }; - face.exterior.write().surface = Partial::from(surface.clone()); face.exterior .write() .update_as_polygon_from_points([a, b, c, d, e]); diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index c20ed6256..8348167ab 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -74,6 +74,12 @@ pub trait CycleBuilder { ) -> O::SameSize> where O: ObjectArgument>; + + /// Infer the positions of all vertices, if necessary + fn infer_vertex_positions_if_necessary( + &mut self, + surface: &SurfaceGeometry, + ); } impl CycleBuilder for PartialCycle { @@ -187,4 +193,15 @@ impl CycleBuilder for PartialCycle { this }) } + + fn infer_vertex_positions_if_necessary( + &mut self, + surface: &SurfaceGeometry, + ) { + for half_edge in &mut self.half_edges { + half_edge + .write() + .infer_vertex_positions_if_necessary(surface); + } + } } diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 34bde9efc..277de3be5 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -5,7 +5,7 @@ use fj_interop::ext::ArrayExt; use crate::{ geometry::path::SurfacePath, objects::{Cycle, Surface}, - partial::{MaybeSurfacePath, Partial, PartialCycle, PartialFace}, + partial::{MaybeSurfacePath, Partial, PartialFace}, }; use super::SurfaceBuilder; @@ -33,16 +33,13 @@ pub trait FaceBuilder { impl FaceBuilder for PartialFace { fn add_interior(&mut self) -> Partial { - let cycle = Partial::from_partial(PartialCycle { - surface: self.exterior.read().surface.clone(), - ..Default::default() - }); + let cycle = Partial::new(); self.interiors.push(cycle.clone()); cycle } fn update_surface_as_plane(&mut self) -> Partial { - let mut exterior = self.exterior.write(); + let exterior = self.exterior.read(); let mut vertices = exterior .half_edges .iter() @@ -69,7 +66,7 @@ impl FaceBuilder for PartialFace { let first_three_points_global = first_three_vertices.each_ref_ext().map(|(_, point)| *point); - let (first_three_points_surface, surface) = exterior + let (first_three_points_surface, surface) = self .surface .write() .update_as_plane_from_points(first_three_points_global); @@ -92,7 +89,7 @@ impl FaceBuilder for PartialFace { surface_vertex.write().position = Some(point); } - exterior.surface.clone() + self.surface.clone() } fn infer_curves(&mut self) { diff --git a/crates/fj-kernel/src/objects/full/cycle.rs b/crates/fj-kernel/src/objects/full/cycle.rs index 5fdfb4a4a..a8f429553 100644 --- a/crates/fj-kernel/src/objects/full/cycle.rs +++ b/crates/fj-kernel/src/objects/full/cycle.rs @@ -3,16 +3,11 @@ use std::slice; use fj_interop::ext::SliceExt; use fj_math::{Scalar, Winding}; -use crate::{ - geometry::path::SurfacePath, - objects::{HalfEdge, Surface}, - storage::Handle, -}; +use crate::{geometry::path::SurfacePath, objects::HalfEdge, storage::Handle}; /// A cycle of connected half-edges #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Cycle { - surface: Handle, half_edges: Vec>, } @@ -22,10 +17,7 @@ impl Cycle { /// # Panics /// /// Panics, if `half_edges` does not yield at least one half-edge. - pub fn new( - surface: Handle, - half_edges: impl IntoIterator>, - ) -> Self { + pub fn new(half_edges: impl IntoIterator>) -> Self { let half_edges = half_edges.into_iter().collect::>(); // This is not a validation check, and thus not part of the validation @@ -37,15 +29,7 @@ impl Cycle { "Cycle must contain at least one half-edge" ); - Self { - surface, - half_edges, - } - } - - /// Access the surface that the cycle is in - pub fn surface(&self) -> &Handle { - &self.surface + Self { half_edges } } /// Access the half-edges that make up the cycle diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index 386fce9e1..7718f52ad 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -1,6 +1,5 @@ use crate::{ - builder::HalfEdgeBuilder, - objects::{Cycle, HalfEdge, Objects, Surface}, + objects::{Cycle, HalfEdge, Objects}, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, }; @@ -8,9 +7,6 @@ use crate::{ /// A partial [`Cycle`] #[derive(Clone, Debug, Default)] pub struct PartialCycle { - /// The surface that the cycle is defined in - pub surface: Partial, - /// The half-edges that make up the cycle pub half_edges: Vec>, } @@ -20,7 +16,6 @@ impl PartialObject for PartialCycle { fn from_full(cycle: &Self::Full, cache: &mut FullToPartialCache) -> Self { Self { - surface: Partial::from_full(cycle.surface().clone(), cache), half_edges: cycle .half_edges() .cloned() @@ -30,15 +25,11 @@ impl PartialObject for PartialCycle { } fn build(self, objects: &mut Service) -> Self::Full { - let surface = self.surface.build(objects); - let surface_geometry = surface.geometry(); - let half_edges = self.half_edges.into_iter().map(|mut half_edge| { - half_edge - .write() - .infer_vertex_positions_if_necessary(&surface_geometry); - half_edge.build(objects) - }); + let half_edges = self + .half_edges + .into_iter() + .map(|half_edge| half_edge.build(objects)); - Cycle::new(surface, half_edges) + Cycle::new(half_edges) } } diff --git a/crates/fj-kernel/src/partial/objects/face.rs b/crates/fj-kernel/src/partial/objects/face.rs index 2d4d5d0c4..86b03bf0f 100644 --- a/crates/fj-kernel/src/partial/objects/face.rs +++ b/crates/fj-kernel/src/partial/objects/face.rs @@ -1,6 +1,7 @@ use fj_interop::mesh::Color; use crate::{ + builder::CycleBuilder, objects::{Cycle, Face, Objects, Surface}, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, @@ -39,8 +40,18 @@ impl PartialObject for PartialFace { } } - fn build(self, objects: &mut Service) -> Self::Full { + fn build(mut self, objects: &mut Service) -> Self::Full { let surface = self.surface.build(objects); + + self.exterior + .write() + .infer_vertex_positions_if_necessary(&surface.geometry()); + for interior in &mut self.interiors { + interior + .write() + .infer_vertex_positions_if_necessary(&surface.geometry()); + } + let exterior = self.exterior.build(objects); let interiors = self.interiors.into_iter().map(|cycle| cycle.build(objects)); diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 264c4acf6..d8898820d 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -17,11 +17,6 @@ impl Validate for Cycle { ) { CycleValidationError::check_half_edge_connections(self, errors); CycleValidationError::check_half_edge_boundaries(self, config, errors); - CycleValidationError::check_vertex_positions(self, config, errors); - - // We don't need to check that all half-edges are defined in the same - // surface. We already check that they are connected by identical - // surface vertices, so that would be redundant. } } @@ -71,33 +66,6 @@ pub enum CycleValidationError { /// The half-edge half_edge: Handle, }, - - /// Mismatch between [`SurfaceVertex`] and `GlobalVertex` positions - #[error( - "`SurfaceVertex` position doesn't match position of its global form\n\ - - Surface position: {surface_position:?}\n\ - - Surface position converted to global position: \ - {surface_position_as_global:?}\n\ - - Global position: {global_position:?}\n\ - - Distance between the positions: {distance}\n\ - - `SurfaceVertex`: {surface_vertex:#?}" - )] - VertexSurfacePositionMismatch { - /// The position of the surface vertex - surface_position: Point<2>, - - /// The surface position converted into a global position - surface_position_as_global: Point<3>, - - /// The position of the global vertex - global_position: Point<3>, - - /// The distance between the positions - distance: Scalar, - - /// The surface vertex - surface_vertex: Handle, - }, } impl CycleValidationError { @@ -158,49 +126,15 @@ impl CycleValidationError { } } } - - fn check_vertex_positions( - cycle: &Cycle, - config: &ValidationConfig, - errors: &mut Vec, - ) { - for half_edge in cycle.half_edges() { - for surface_vertex in half_edge.surface_vertices() { - let surface_position_as_global = cycle - .surface() - .geometry() - .point_from_surface_coords(surface_vertex.position()); - let global_position = surface_vertex.global_form().position(); - - let distance = - surface_position_as_global.distance_to(&global_position); - - if distance > config.identical_max_distance { - errors.push( - Self::VertexSurfacePositionMismatch { - surface_position: surface_vertex.position(), - surface_position_as_global, - global_position, - distance, - surface_vertex: surface_vertex.clone(), - } - .into(), - ); - } - } - } - } } #[cfg(test)] mod tests { - use fj_interop::ext::ArrayExt; - use fj_math::{Point, Scalar, Vector}; + use fj_math::Point; use crate::{ - builder::{CycleBuilder, HalfEdgeBuilder}, - insert::Insert, - objects::{Cycle, HalfEdge, SurfaceVertex}, + builder::CycleBuilder, + objects::Cycle, partial::{Partial, PartialCycle, PartialObject}, services::Services, validate::Validate, @@ -211,11 +145,11 @@ mod tests { let mut services = Services::new(); let valid = { - let mut cycle = PartialCycle { - surface: Partial::from(services.objects.surfaces.xy_plane()), - ..Default::default() - }; + let surface = services.objects.surfaces.xy_plane(); + + let mut cycle = PartialCycle::default(); cycle.update_as_polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]); + cycle.infer_vertex_positions_if_necessary(&surface.geometry()); cycle.build(&mut services.objects) }; let invalid = { @@ -238,7 +172,7 @@ mod tests { .into_iter() .map(|half_edge| half_edge.build(&mut services.objects)); - Cycle::new(valid.surface().clone(), half_edges) + Cycle::new(half_edges) }; valid.validate_and_return_first_error()?; @@ -252,11 +186,11 @@ mod tests { let mut services = Services::new(); let valid = { - let mut cycle = PartialCycle { - surface: Partial::from(services.objects.surfaces.xy_plane()), - ..Default::default() - }; + let surface = services.objects.surfaces.xy_plane(); + + let mut cycle = PartialCycle::default(); cycle.update_as_polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]); + cycle.infer_vertex_positions_if_necessary(&surface.geometry()); cycle.build(&mut services.objects) }; let invalid = { @@ -274,69 +208,7 @@ mod tests { .into_iter() .map(|half_edge| half_edge.build(&mut services.objects)); - Cycle::new(valid.surface().clone(), half_edges) - }; - - valid.validate_and_return_first_error()?; - assert!(invalid.validate_and_return_first_error().is_err()); - - Ok(()) - } - - #[test] - fn surface_vertex_position_mismatch() -> anyhow::Result<()> { - let mut services = Services::new(); - - let valid = { - let surface = services.objects.surfaces.xy_plane(); - - let mut cycle = PartialCycle { - surface: Partial::from(surface.clone()), - ..Default::default() - }; - - let mut half_edge = cycle.add_half_edge(); - half_edge.write().update_as_circle_from_radius(1.); - half_edge - .write() - .infer_vertex_positions_if_necessary(&surface.geometry()); - - cycle.build(&mut services.objects) - }; - let invalid = { - let half_edge = { - let half_edge = valid.half_edges().next().unwrap(); - - let boundary = half_edge - .boundary() - .map(|point| point + Vector::from([Scalar::PI / 2.])); - - let mut surface_vertices = - half_edge.surface_vertices().map(Clone::clone); - - let mut invalid = None; - for surface_vertex in surface_vertices.each_mut_ext() { - let invalid = invalid.get_or_insert_with(|| { - SurfaceVertex::new( - [0., 1.], - surface_vertex.global_form().clone(), - ) - .insert(&mut services.objects) - }); - *surface_vertex = invalid.clone(); - } - - let boundary = boundary.zip_ext(surface_vertices); - - HalfEdge::new( - half_edge.curve().clone(), - boundary, - half_edge.global_form().clone(), - ) - .insert(&mut services.objects) - }; - - Cycle::new(valid.surface().clone(), [half_edge]) + Cycle::new(half_edges) }; valid.validate_and_return_first_error()?; diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index ff500cffa..6fde9dfb9 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -1,7 +1,7 @@ -use fj_math::Winding; +use fj_math::{Point, Scalar, Winding}; use crate::{ - objects::{Cycle, Face, Surface}, + objects::{Cycle, Face, Surface, SurfaceVertex}, storage::Handle, }; @@ -10,11 +10,11 @@ use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Face { fn validate_with_config( &self, - _: &ValidationConfig, + config: &ValidationConfig, errors: &mut Vec, ) { - FaceValidationError::check_surface_identity(self, errors); FaceValidationError::check_interior_winding(self, errors); + FaceValidationError::check_vertex_positions(self, config, errors); } } @@ -56,26 +56,36 @@ pub enum FaceValidationError { /// The face face: Face, }, + + /// Mismatch between [`SurfaceVertex`] and `GlobalVertex` positions + #[error( + "`SurfaceVertex` position doesn't match position of its global form\n\ + - Surface position: {surface_position:?}\n\ + - Surface position converted to global position: \ + {surface_position_as_global:?}\n\ + - Global position: {global_position:?}\n\ + - Distance between the positions: {distance}\n\ + - `SurfaceVertex`: {surface_vertex:#?}" + )] + VertexPositionMismatch { + /// The position of the surface vertex + surface_position: Point<2>, + + /// The surface position converted into a global position + surface_position_as_global: Point<3>, + + /// The position of the global vertex + global_position: Point<3>, + + /// The distance between the positions + distance: Scalar, + + /// The surface vertex + surface_vertex: Handle, + }, } impl FaceValidationError { - fn check_surface_identity(face: &Face, errors: &mut Vec) { - let surface = face.surface(); - - for interior in face.interiors() { - if surface.id() != interior.surface().id() { - errors.push( - Box::new(Self::SurfaceMismatch { - surface: surface.clone(), - interior: interior.clone(), - face: face.clone(), - }) - .into(), - ); - } - } - } - fn check_interior_winding(face: &Face, errors: &mut Vec) { let exterior_winding = face.exterior().winding(); @@ -94,15 +104,53 @@ impl FaceValidationError { } } } + + fn check_vertex_positions( + face: &Face, + config: &ValidationConfig, + errors: &mut Vec, + ) { + for cycle in face.all_cycles() { + for half_edge in cycle.half_edges() { + for surface_vertex in half_edge.surface_vertices() { + let surface_position_as_global = face + .surface() + .geometry() + .point_from_surface_coords(surface_vertex.position()); + let global_position = + surface_vertex.global_form().position(); + + let distance = surface_position_as_global + .distance_to(&global_position); + + if distance > config.identical_max_distance { + errors.push( + Box::new(Self::VertexPositionMismatch { + surface_position: surface_vertex.position(), + surface_position_as_global, + global_position, + distance, + surface_vertex: surface_vertex.clone(), + }) + .into(), + ); + } + } + } + } + } } #[cfg(test)] mod tests { + use fj_interop::ext::ArrayExt; + use fj_math::{Scalar, Vector}; + use crate::{ algorithms::reverse::Reverse, - builder::{CycleBuilder, FaceBuilder}, + builder::{CycleBuilder, FaceBuilder, HalfEdgeBuilder}, insert::Insert, - objects::Face, + objects::{Cycle, Face, HalfEdge, SurfaceVertex}, partial::{Partial, PartialCycle, PartialFace, PartialObject}, services::Services, validate::Validate, @@ -112,14 +160,11 @@ mod tests { fn face_surface_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); - let surface = services.objects.surfaces.xy_plane(); - let valid = { let mut face = PartialFace { - surface: Partial::from(surface.clone()), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = Partial::from(surface); face.exterior.write().update_as_polygon_from_points([ [0., 0.], [3., 0.], @@ -134,11 +179,11 @@ mod tests { face.build(&mut services.objects) }; let invalid = { - let mut cycle = PartialCycle { - surface: Partial::from(services.objects.surfaces.xz_plane()), - ..Default::default() - }; + let surface = services.objects.surfaces.xz_plane(); + + let mut cycle = PartialCycle::default(); cycle.update_as_polygon_from_points([[1., 1.], [1., 2.], [2., 1.]]); + cycle.infer_vertex_positions_if_necessary(&surface.geometry()); let cycle = cycle .build(&mut services.objects) .insert(&mut services.objects); @@ -162,14 +207,11 @@ mod tests { fn face_invalid_interior_winding() -> anyhow::Result<()> { let mut services = Services::new(); - let surface = services.objects.surfaces.xy_plane(); - let valid = { let mut face = PartialFace { - surface: Partial::from(surface.clone()), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; - face.exterior.write().surface = Partial::from(surface); face.exterior.write().update_as_polygon_from_points([ [0., 0.], [3., 0.], @@ -202,4 +244,74 @@ mod tests { Ok(()) } + + #[test] + fn surface_vertex_position_mismatch() -> anyhow::Result<()> { + let mut services = Services::new(); + + let valid = { + let surface = services.objects.surfaces.xy_plane(); + + let mut face = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; + + let mut half_edge = face.exterior.write().add_half_edge(); + half_edge.write().update_as_circle_from_radius(1.); + half_edge + .write() + .infer_vertex_positions_if_necessary(&surface.geometry()); + + face.build(&mut services.objects) + }; + let invalid = { + let half_edge = { + let half_edge = valid.exterior().half_edges().next().unwrap(); + + let boundary = half_edge + .boundary() + .map(|point| point + Vector::from([Scalar::PI / 2.])); + + let mut surface_vertices = + half_edge.surface_vertices().map(Clone::clone); + + let mut invalid = None; + for surface_vertex in surface_vertices.each_mut_ext() { + let invalid = invalid.get_or_insert_with(|| { + SurfaceVertex::new( + [0., 1.], + surface_vertex.global_form().clone(), + ) + .insert(&mut services.objects) + }); + *surface_vertex = invalid.clone(); + } + + let boundary = boundary.zip_ext(surface_vertices); + + HalfEdge::new( + half_edge.curve().clone(), + boundary, + half_edge.global_form().clone(), + ) + .insert(&mut services.objects) + }; + + let exterior = + Cycle::new([half_edge]).insert(&mut services.objects); + + Face::new( + valid.surface().clone(), + exterior, + valid.interiors().cloned(), + valid.color(), + ) + }; + + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); + + Ok(()) + } } diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 83dec0e28..f6a463e6b 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -36,10 +36,7 @@ impl Shape for fj::Sketch { Partial::from_partial(half_edge) }; let exterior = { - let mut cycle = PartialCycle { - surface: surface.clone(), - ..Default::default() - }; + let mut cycle = PartialCycle::default(); cycle.half_edges.push(half_edge); Partial::from_partial(cycle) }; @@ -59,10 +56,7 @@ impl Shape for fj::Sketch { ); let exterior = { - let mut cycle = PartialCycle { - surface: Partial::from(surface.clone()), - ..Default::default() - }; + let mut cycle = PartialCycle::default(); let mut line_segments = vec![]; let mut arcs = vec![]; poly_chain.to_segments().into_iter().for_each(