From 1b85db44614bc61d2cecf93c669ec7a35f40012d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 14:32:27 +0100 Subject: [PATCH 01/15] Avoid use of `PartialCycle`'s `surface` field --- crates/fj-kernel/src/builder/face.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 34bde9efc..cd76bf8b0 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -42,7 +42,7 @@ impl FaceBuilder for PartialFace { } fn update_surface_as_plane(&mut self) -> Partial { - let mut exterior = self.exterior.write(); + let exterior = self.exterior.write(); let mut vertices = exterior .half_edges .iter() @@ -69,7 +69,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 +92,7 @@ impl FaceBuilder for PartialFace { surface_vertex.write().position = Some(point); } - exterior.surface.clone() + self.surface.clone() } fn infer_curves(&mut self) { From bf2b0ba70a9497ad202a327a3ae1d2fb56de43a4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 14:33:25 +0100 Subject: [PATCH 02/15] Lower access to required level --- crates/fj-kernel/src/builder/face.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index cd76bf8b0..173a45986 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -42,7 +42,7 @@ impl FaceBuilder for PartialFace { } fn update_surface_as_plane(&mut self) -> Partial { - let exterior = self.exterior.write(); + let exterior = self.exterior.read(); let mut vertices = exterior .half_edges .iter() From 640078caed69949c1f8a710691688093a99bfcb8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 14:38:22 +0100 Subject: [PATCH 03/15] Add new `CycleBuilder` method --- crates/fj-kernel/src/builder/cycle.rs | 17 +++++++++++++++++ crates/fj-kernel/src/partial/objects/cycle.rs | 17 ++++++++--------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index c20ed6256..8f8e5473b 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -74,6 +74,9 @@ pub trait CycleBuilder { ) -> O::SameSize> where O: ObjectArgument>; + + /// Infer the positions of all vertices, if necessary + fn infer_vertex_positions_if_necessary(&mut self); } impl CycleBuilder for PartialCycle { @@ -187,4 +190,18 @@ impl CycleBuilder for PartialCycle { this }) } + + fn infer_vertex_positions_if_necessary(&mut self) { + let surface = self + .surface + .read() + .geometry + .expect("Need surface geometry to infer vertex positions"); + + for half_edge in &mut self.half_edges { + half_edge + .write() + .infer_vertex_positions_if_necessary(&surface); + } + } } diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index 386fce9e1..1d6077f4a 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -1,5 +1,5 @@ use crate::{ - builder::HalfEdgeBuilder, + builder::CycleBuilder, objects::{Cycle, HalfEdge, Objects, Surface}, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, @@ -29,15 +29,14 @@ impl PartialObject for PartialCycle { } } - fn build(self, objects: &mut Service) -> Self::Full { + fn build(mut self, objects: &mut Service) -> Self::Full { + self.infer_vertex_positions_if_necessary(); + 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) } From aeaf8ee3e1a3ded9c4697b724b3bf1c1651886c0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 14:50:15 +0100 Subject: [PATCH 04/15] Don't infer vertex positions in cycle implicitly --- crates/fj-kernel/src/partial/objects/cycle.rs | 5 +---- crates/fj-kernel/src/partial/objects/face.rs | 8 +++++++- crates/fj-kernel/src/validate/cycle.rs | 2 ++ crates/fj-kernel/src/validate/face.rs | 1 + 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index 1d6077f4a..b7ee51cf6 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -1,5 +1,4 @@ use crate::{ - builder::CycleBuilder, objects::{Cycle, HalfEdge, Objects, Surface}, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, @@ -29,9 +28,7 @@ impl PartialObject for PartialCycle { } } - fn build(mut self, objects: &mut Service) -> Self::Full { - self.infer_vertex_positions_if_necessary(); - + fn build(self, objects: &mut Service) -> Self::Full { let surface = self.surface.build(objects); let half_edges = self .half_edges diff --git a/crates/fj-kernel/src/partial/objects/face.rs b/crates/fj-kernel/src/partial/objects/face.rs index 2d4d5d0c4..1675a6ef2 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,7 +40,12 @@ impl PartialObject for PartialFace { } } - fn build(self, objects: &mut Service) -> Self::Full { + fn build(mut self, objects: &mut Service) -> Self::Full { + self.exterior.write().infer_vertex_positions_if_necessary(); + for interior in &mut self.interiors { + interior.write().infer_vertex_positions_if_necessary(); + } + let surface = self.surface.build(objects); let exterior = self.exterior.build(objects); let interiors = diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 264c4acf6..200dbd39a 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -216,6 +216,7 @@ mod tests { ..Default::default() }; cycle.update_as_polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]); + cycle.infer_vertex_positions_if_necessary(); cycle.build(&mut services.objects) }; let invalid = { @@ -257,6 +258,7 @@ mod tests { ..Default::default() }; cycle.update_as_polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]); + cycle.infer_vertex_positions_if_necessary(); cycle.build(&mut services.objects) }; let invalid = { diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index ff500cffa..c94ac375b 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -139,6 +139,7 @@ mod tests { ..Default::default() }; cycle.update_as_polygon_from_points([[1., 1.], [1., 2.], [2., 1.]]); + cycle.infer_vertex_positions_if_necessary(); let cycle = cycle .build(&mut services.objects) .insert(&mut services.objects); From 7ee02121ac9a2cff15b2b512fd9bea691e6fd656 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 14:53:58 +0100 Subject: [PATCH 05/15] Prepare code for follow-on change --- crates/fj-kernel/src/validate/cycle.rs | 8 ++++++-- crates/fj-kernel/src/validate/face.rs | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 200dbd39a..044c047be 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -211,8 +211,10 @@ mod tests { let mut services = Services::new(); let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut cycle = PartialCycle { - surface: Partial::from(services.objects.surfaces.xy_plane()), + surface: Partial::from(surface), ..Default::default() }; cycle.update_as_polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]); @@ -253,8 +255,10 @@ mod tests { let mut services = Services::new(); let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut cycle = PartialCycle { - surface: Partial::from(services.objects.surfaces.xy_plane()), + surface: Partial::from(surface), ..Default::default() }; cycle.update_as_polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]); diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index c94ac375b..412f5151c 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -134,8 +134,10 @@ mod tests { face.build(&mut services.objects) }; let invalid = { + let surface = services.objects.surfaces.xz_plane(); + let mut cycle = PartialCycle { - surface: Partial::from(services.objects.surfaces.xz_plane()), + surface: Partial::from(surface), ..Default::default() }; cycle.update_as_polygon_from_points([[1., 1.], [1., 2.], [2., 1.]]); From 81e07f23f34b75c28e69e9ba434e95da3c73a535 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 14:56:22 +0100 Subject: [PATCH 06/15] Avoid use of `PartialCycle`'s `surface` field --- crates/fj-kernel/src/builder/cycle.rs | 18 +++++++++--------- crates/fj-kernel/src/partial/objects/face.rs | 11 ++++++++--- crates/fj-kernel/src/validate/cycle.rs | 8 ++++---- crates/fj-kernel/src/validate/face.rs | 4 ++-- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 8f8e5473b..8348167ab 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -76,7 +76,10 @@ pub trait CycleBuilder { O: ObjectArgument>; /// Infer the positions of all vertices, if necessary - fn infer_vertex_positions_if_necessary(&mut self); + fn infer_vertex_positions_if_necessary( + &mut self, + surface: &SurfaceGeometry, + ); } impl CycleBuilder for PartialCycle { @@ -191,17 +194,14 @@ impl CycleBuilder for PartialCycle { }) } - fn infer_vertex_positions_if_necessary(&mut self) { - let surface = self - .surface - .read() - .geometry - .expect("Need surface geometry to infer vertex positions"); - + 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); + .infer_vertex_positions_if_necessary(surface); } } } diff --git a/crates/fj-kernel/src/partial/objects/face.rs b/crates/fj-kernel/src/partial/objects/face.rs index 1675a6ef2..86b03bf0f 100644 --- a/crates/fj-kernel/src/partial/objects/face.rs +++ b/crates/fj-kernel/src/partial/objects/face.rs @@ -41,12 +41,17 @@ impl PartialObject for PartialFace { } fn build(mut self, objects: &mut Service) -> Self::Full { - self.exterior.write().infer_vertex_positions_if_necessary(); + 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(); + interior + .write() + .infer_vertex_positions_if_necessary(&surface.geometry()); } - let surface = self.surface.build(objects); 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 044c047be..0593e53b8 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -214,11 +214,11 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); let mut cycle = PartialCycle { - surface: Partial::from(surface), + surface: Partial::from(surface.clone()), ..Default::default() }; cycle.update_as_polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]); - cycle.infer_vertex_positions_if_necessary(); + cycle.infer_vertex_positions_if_necessary(&surface.geometry()); cycle.build(&mut services.objects) }; let invalid = { @@ -258,11 +258,11 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); let mut cycle = PartialCycle { - surface: Partial::from(surface), + surface: Partial::from(surface.clone()), ..Default::default() }; cycle.update_as_polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]); - cycle.infer_vertex_positions_if_necessary(); + cycle.infer_vertex_positions_if_necessary(&surface.geometry()); cycle.build(&mut services.objects) }; let invalid = { diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 412f5151c..c18b06144 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -137,11 +137,11 @@ mod tests { let surface = services.objects.surfaces.xz_plane(); let mut cycle = PartialCycle { - surface: Partial::from(surface), + surface: Partial::from(surface.clone()), ..Default::default() }; cycle.update_as_polygon_from_points([[1., 1.], [1., 2.], [2., 1.]]); - cycle.infer_vertex_positions_if_necessary(); + cycle.infer_vertex_positions_if_necessary(&surface.geometry()); let cycle = cycle .build(&mut services.objects) .insert(&mut services.objects); From e571adc1fd1b70df0909f766d7f7439074a33543 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 14:59:34 +0100 Subject: [PATCH 07/15] Remove outdated comment --- crates/fj-kernel/src/validate/cycle.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 0593e53b8..8331a2f48 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -18,10 +18,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. } } From 0b8590ae711d5c0f389d832488ab292736e74a45 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 15:02:23 +0100 Subject: [PATCH 08/15] Reduce variable scopes --- crates/fj-kernel/src/validate/face.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index c18b06144..2074e4e08 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -112,9 +112,9 @@ mod tests { fn face_surface_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); - let surface = services.objects.surfaces.xy_plane(); - let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut face = PartialFace { surface: Partial::from(surface.clone()), ..Default::default() @@ -165,9 +165,9 @@ mod tests { fn face_invalid_interior_winding() -> anyhow::Result<()> { let mut services = Services::new(); - let surface = services.objects.surfaces.xy_plane(); - let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut face = PartialFace { surface: Partial::from(surface.clone()), ..Default::default() From dc78d4ae46f8d2b6353d5ea85c24cf2d7eae592e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 15:08:08 +0100 Subject: [PATCH 09/15] Move validation check from cycle to face level This validation check requires access to the surface, which I am in the process of removing from `Cycle`. --- crates/fj-kernel/src/validate/cycle.rs | 130 +--------------------- crates/fj-kernel/src/validate/face.rs | 147 ++++++++++++++++++++++++- 2 files changed, 145 insertions(+), 132 deletions(-) diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 8331a2f48..48b36bb37 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -17,7 +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); } } @@ -67,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 { @@ -154,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, @@ -284,66 +222,4 @@ 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 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]) - }; - - valid.validate_and_return_first_error()?; - assert!(invalid.validate_and_return_first_error().is_err()); - - Ok(()) - } } diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 2074e4e08..a16a01c62 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,12 @@ 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,6 +57,33 @@ 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:#?}" + )] + 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 FaceValidationError { @@ -94,15 +122,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 = 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( + Box::new(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::{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, @@ -205,4 +271,75 @@ 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() + }; + face.exterior.write().surface = Partial::from(surface.clone()); + + 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(valid.surface().clone(), [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(()) + } } From e328f44c609900dbcacff62dcdebc02bd18ff8da Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 15:09:23 +0100 Subject: [PATCH 10/15] Simplify name of enum variant --- crates/fj-kernel/src/validate/face.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index a16a01c62..c7133264c 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -68,7 +68,7 @@ pub enum FaceValidationError { - Distance between the positions: {distance}\n\ - `SurfaceVertex`: {surface_vertex:#?}" )] - VertexSurfacePositionMismatch { + VertexPositionMismatch { /// The position of the surface vertex surface_position: Point<2>, @@ -143,7 +143,7 @@ impl FaceValidationError { if distance > config.identical_max_distance { errors.push( - Box::new(Self::VertexSurfacePositionMismatch { + Box::new(Self::VertexPositionMismatch { surface_position: surface_vertex.position(), surface_position_as_global, global_position, From 3c85fb911cdea8456c9e34bd9741dac20951cacf Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 15:16:02 +0100 Subject: [PATCH 11/15] Avoid use of `Cycle::surface` --- crates/fj-kernel/src/validate/face.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index c7133264c..24f193338 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -131,7 +131,7 @@ impl FaceValidationError { 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 = cycle + let surface_position_as_global = face .surface() .geometry() .point_from_surface_coords(surface_vertex.position()); From 0bec935c5432a7a88361f06034539987debf07af Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 15:27:15 +0100 Subject: [PATCH 12/15] Remove `Surface` ref from `Cycle`/`PartialCycle` --- .../src/algorithms/intersect/curve_face.rs | 3 +- .../src/algorithms/intersect/face_face.rs | 6 ++-- .../src/algorithms/intersect/face_point.rs | 24 +++++---------- .../src/algorithms/intersect/ray_face.rs | 21 +++++-------- .../fj-kernel/src/algorithms/reverse/cycle.rs | 2 +- crates/fj-kernel/src/algorithms/sweep/edge.rs | 4 +-- crates/fj-kernel/src/algorithms/sweep/face.rs | 14 ++------- .../src/algorithms/transform/cycle.rs | 6 +--- .../src/algorithms/triangulate/mod.rs | 5 +--- crates/fj-kernel/src/builder/face.rs | 1 - crates/fj-kernel/src/objects/full/cycle.rs | 22 ++------------ crates/fj-kernel/src/partial/objects/cycle.rs | 9 ++---- crates/fj-kernel/src/validate/cycle.rs | 6 ++-- crates/fj-kernel/src/validate/face.rs | 30 +++---------------- crates/fj-operations/src/sketch.rs | 2 -- 15 files changed, 35 insertions(+), 120 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_face.rs b/crates/fj-kernel/src/algorithms/intersect/curve_face.rs index de42b5e60..92677c73e 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_face.rs @@ -184,10 +184,9 @@ mod tests { let face = { let mut face = PartialFace { - surface: surface.clone(), + surface, ..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..f5c94620e 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_point.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_point.rs @@ -151,10 +151,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [1., 1.], @@ -176,10 +175,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -204,10 +202,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [4., 2.], [0., 4.], @@ -232,10 +229,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -261,10 +257,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -290,10 +285,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -320,10 +314,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 0.], @@ -357,10 +350,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..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..2cfc93d03 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -166,10 +166,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.yz_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -193,10 +192,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.yz_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -223,10 +221,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.yz_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -250,10 +247,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.yz_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -288,10 +284,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.yz_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -326,10 +321,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -355,10 +349,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..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..a82c1a4f3 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. @@ -270,7 +269,6 @@ mod tests { }; let mut cycle = PartialCycle { - surface: Partial::from(surface.clone()), ..Default::default() }; cycle.half_edges.extend( 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..41b17b606 100644 --- a/crates/fj-kernel/src/algorithms/triangulate/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulate/mod.rs @@ -99,10 +99,9 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); let mut face = PartialFace { - surface: surface.clone(), + surface, ..Default::default() }; - face.exterior.write().surface = surface; face.exterior .write() .update_as_polygon_from_points([a, b, c, d]); @@ -144,7 +143,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 +209,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/face.rs b/crates/fj-kernel/src/builder/face.rs index 173a45986..9514b6984 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -34,7 +34,6 @@ 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() }); self.interiors.push(cycle.clone()); 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 b7ee51cf6..7718f52ad 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -1,5 +1,5 @@ use crate::{ - objects::{Cycle, HalfEdge, Objects, Surface}, + objects::{Cycle, HalfEdge, Objects}, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, }; @@ -7,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>, } @@ -19,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() @@ -29,12 +25,11 @@ impl PartialObject for PartialCycle { } fn build(self, objects: &mut Service) -> Self::Full { - let surface = self.surface.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/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 48b36bb37..5d7c3c76d 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -148,7 +148,6 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); let mut cycle = PartialCycle { - surface: Partial::from(surface.clone()), ..Default::default() }; cycle.update_as_polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]); @@ -175,7 +174,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()?; @@ -192,7 +191,6 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); let mut cycle = PartialCycle { - surface: Partial::from(surface.clone()), ..Default::default() }; cycle.update_as_polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]); @@ -214,7 +212,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()?; diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 24f193338..883d9d6c5 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -13,7 +13,6 @@ impl Validate for Face { config: &ValidationConfig, errors: &mut Vec, ) { - FaceValidationError::check_surface_identity(self, errors); FaceValidationError::check_interior_winding(self, errors); FaceValidationError::check_vertex_positions(self, config, errors); } @@ -87,23 +86,6 @@ pub enum FaceValidationError { } 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(); @@ -182,10 +164,9 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); 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([ [0., 0.], [3., 0.], @@ -203,7 +184,6 @@ mod tests { let surface = services.objects.surfaces.xz_plane(); let mut cycle = PartialCycle { - surface: Partial::from(surface.clone()), ..Default::default() }; cycle.update_as_polygon_from_points([[1., 1.], [1., 2.], [2., 1.]]); @@ -235,10 +215,9 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); 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([ [0., 0.], [3., 0.], @@ -283,7 +262,6 @@ mod tests { surface: Partial::from(surface.clone()), ..Default::default() }; - face.exterior.write().surface = Partial::from(surface.clone()); let mut half_edge = face.exterior.write().add_half_edge(); half_edge.write().update_as_circle_from_radius(1.); @@ -326,8 +304,8 @@ mod tests { .insert(&mut services.objects) }; - let exterior = Cycle::new(valid.surface().clone(), [half_edge]) - .insert(&mut services.objects); + let exterior = + Cycle::new([half_edge]).insert(&mut services.objects); Face::new( valid.surface().clone(), diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 83dec0e28..1e223a917 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -37,7 +37,6 @@ impl Shape for fj::Sketch { }; let exterior = { let mut cycle = PartialCycle { - surface: surface.clone(), ..Default::default() }; cycle.half_edges.push(half_edge); @@ -60,7 +59,6 @@ impl Shape for fj::Sketch { let exterior = { let mut cycle = PartialCycle { - surface: Partial::from(surface.clone()), ..Default::default() }; let mut line_segments = vec![]; From bba87e7ccf8a881fa3ff5ee9fe8a1a310ccd9197 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 15:28:14 +0100 Subject: [PATCH 13/15] Inline variables --- .../src/algorithms/intersect/curve_face.rs | 4 +-- .../src/algorithms/intersect/face_point.rs | 32 +++++-------------- .../src/algorithms/intersect/ray_face.rs | 28 ++++------------ .../src/algorithms/triangulate/mod.rs | 4 +-- crates/fj-kernel/src/validate/face.rs | 8 ++--- 5 files changed, 19 insertions(+), 57 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_face.rs b/crates/fj-kernel/src/algorithms/intersect/curve_face.rs index 92677c73e..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,7 +182,7 @@ mod tests { let face = { let mut face = PartialFace { - surface, + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior diff --git a/crates/fj-kernel/src/algorithms/intersect/face_point.rs b/crates/fj-kernel/src/algorithms/intersect/face_point.rs index f5c94620e..1f13c0a8e 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_point.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_point.rs @@ -148,10 +148,8 @@ 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: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -172,10 +170,8 @@ 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: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -199,10 +195,8 @@ 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: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -226,10 +220,8 @@ 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: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -254,10 +246,8 @@ 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: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -282,10 +272,8 @@ 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: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -311,10 +299,8 @@ 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: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -347,10 +333,8 @@ 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: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs index 2cfc93d03..4d90f15c3 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -163,10 +163,8 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace { - surface, + surface: Partial::from(services.objects.surfaces.yz_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -189,10 +187,8 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace { - surface, + surface: Partial::from(services.objects.surfaces.yz_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -218,10 +214,8 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace { - surface, + surface: Partial::from(services.objects.surfaces.yz_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -244,10 +238,8 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace { - surface, + surface: Partial::from(services.objects.surfaces.yz_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -281,10 +273,8 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace { - surface, + surface: Partial::from(services.objects.surfaces.yz_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -318,10 +308,8 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface, + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -346,10 +334,8 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace { - surface, + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ diff --git a/crates/fj-kernel/src/algorithms/triangulate/mod.rs b/crates/fj-kernel/src/algorithms/triangulate/mod.rs index 41b17b606..d2a8ddc97 100644 --- a/crates/fj-kernel/src/algorithms/triangulate/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulate/mod.rs @@ -96,10 +96,8 @@ 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: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 883d9d6c5..3bbd360e5 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -161,10 +161,8 @@ mod tests { let mut services = Services::new(); let valid = { - let surface = services.objects.surfaces.xy_plane(); - let mut face = PartialFace { - surface: Partial::from(surface), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ @@ -212,10 +210,8 @@ mod tests { let mut services = Services::new(); let valid = { - let surface = services.objects.surfaces.xy_plane(); - let mut face = PartialFace { - surface: Partial::from(surface), + surface: Partial::from(services.objects.surfaces.xy_plane()), ..Default::default() }; face.exterior.write().update_as_polygon_from_points([ From 1d64b5afbcf5dd7006b5ca07561cf502dea6a900 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 15:32:16 +0100 Subject: [PATCH 14/15] Simplify `PartialCycle` construction --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 4 +--- crates/fj-kernel/src/validate/cycle.rs | 8 ++------ crates/fj-kernel/src/validate/face.rs | 4 +--- crates/fj-operations/src/sketch.rs | 8 ++------ 4 files changed, 6 insertions(+), 18 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index a82c1a4f3..4f539b11b 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -268,9 +268,7 @@ mod tests { .clone() }; - let mut cycle = PartialCycle { - ..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/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 5d7c3c76d..d8898820d 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -147,9 +147,7 @@ mod tests { let valid = { let surface = services.objects.surfaces.xy_plane(); - let mut cycle = PartialCycle { - ..Default::default() - }; + 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) @@ -190,9 +188,7 @@ mod tests { let valid = { let surface = services.objects.surfaces.xy_plane(); - let mut cycle = PartialCycle { - ..Default::default() - }; + 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) diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 3bbd360e5..6fde9dfb9 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -181,9 +181,7 @@ mod tests { let invalid = { let surface = services.objects.surfaces.xz_plane(); - let mut cycle = PartialCycle { - ..Default::default() - }; + 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 diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 1e223a917..f6a463e6b 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -36,9 +36,7 @@ impl Shape for fj::Sketch { Partial::from_partial(half_edge) }; let exterior = { - let mut cycle = PartialCycle { - ..Default::default() - }; + let mut cycle = PartialCycle::default(); cycle.half_edges.push(half_edge); Partial::from_partial(cycle) }; @@ -58,9 +56,7 @@ impl Shape for fj::Sketch { ); let exterior = { - let mut cycle = PartialCycle { - ..Default::default() - }; + let mut cycle = PartialCycle::default(); let mut line_segments = vec![]; let mut arcs = vec![]; poly_chain.to_segments().into_iter().for_each( From 04bbd4d9075abc1b2752f72e3bcf8b881910f760 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 20 Feb 2023 15:34:01 +0100 Subject: [PATCH 15/15] Simplify `Partial` construction --- crates/fj-kernel/src/builder/face.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 9514b6984..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,9 +33,7 @@ pub trait FaceBuilder { impl FaceBuilder for PartialFace { fn add_interior(&mut self) -> Partial { - let cycle = Partial::from_partial(PartialCycle { - ..Default::default() - }); + let cycle = Partial::new(); self.interiors.push(cycle.clone()); cycle }