From f9bffb79110a3bef0d348ff3bf05055eee1b0123 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 12:34:19 +0100 Subject: [PATCH 01/12] Remove unused code --- crates/fj-kernel/src/builder/vertex.rs | 28 +++----------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/crates/fj-kernel/src/builder/vertex.rs b/crates/fj-kernel/src/builder/vertex.rs index 298bd88c4..1ff0fdcb1 100644 --- a/crates/fj-kernel/src/builder/vertex.rs +++ b/crates/fj-kernel/src/builder/vertex.rs @@ -1,33 +1,11 @@ -use fj_math::Point; - use crate::partial::{PartialGlobalVertex, PartialSurfaceVertex}; /// Builder API for [`PartialSurfaceVertex`] -pub trait SurfaceVertexBuilder { - /// Infer the position of the surface vertex' global form - /// - /// Updates the global vertex referenced by this surface vertex with the - /// inferred position, and also returns the position. - fn infer_global_position(&mut self) -> Point<3>; -} +pub trait SurfaceVertexBuilder {} impl SurfaceVertexBuilder for PartialSurfaceVertex { - fn infer_global_position(&mut self) -> Point<3> { - let position_surface = self - .position - .expect("Can't infer global position without surface position"); - let surface = self - .surface - .read() - .geometry - .expect("Can't infer global position without surface geometry"); - - let position_global = - surface.point_from_surface_coords(position_surface); - self.global_form.write().position = Some(position_global); - - position_global - } + // No methods are currently defined. This trait serves as a placeholder, to + // make it clear where to add such methods, once necessary. } /// Builder API for [`PartialGlobalVertex`] From d96f7ea22b4de1549c3c92ef0374d7cd59702862 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 12:38:46 +0100 Subject: [PATCH 02/12] Improve method name --- crates/fj-kernel/src/validate/edge.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index ed434ea1a..bb7754fc5 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -16,7 +16,7 @@ impl Validate for HalfEdge { HalfEdgeValidationError::check_global_curve_identity(self, errors); HalfEdgeValidationError::check_global_vertex_identity(self, errors); HalfEdgeValidationError::check_surface_identity(self, errors); - HalfEdgeValidationError::check_vertex_positions(self, config, errors); + HalfEdgeValidationError::check_vertex_coincidence(self, config, errors); // We don't need to check anything about surfaces here. We already check // curves, which makes sure the vertices are consistent with each other, @@ -179,7 +179,7 @@ impl HalfEdgeValidationError { } } - fn check_vertex_positions( + fn check_vertex_coincidence( half_edge: &HalfEdge, config: &ValidationConfig, errors: &mut Vec, From dd7b1df57615d98874f766795a5930320b84f44a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 12:56:25 +0100 Subject: [PATCH 03/12] Move check from `SurfaceVertex` to `HalfEdge` I'm working on removing the reference to `Surface` from `SurfaceVertex`. Once that is gone, it won't be able to support that validation check any longer. --- crates/fj-kernel/src/validate/edge.rs | 105 +++++++++++++++++++++++- crates/fj-kernel/src/validate/vertex.rs | 104 +---------------------- 2 files changed, 106 insertions(+), 103 deletions(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index bb7754fc5..5308897a6 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -1,7 +1,9 @@ use fj_math::{Point, Scalar}; use crate::{ - objects::{GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface}, + objects::{ + GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface, SurfaceVertex, + }, storage::Handle, }; @@ -17,6 +19,7 @@ impl Validate for HalfEdge { HalfEdgeValidationError::check_global_vertex_identity(self, errors); HalfEdgeValidationError::check_surface_identity(self, errors); HalfEdgeValidationError::check_vertex_coincidence(self, config, errors); + HalfEdgeValidationError::check_vertex_positions(self, config, errors); // We don't need to check anything about surfaces here. We already check // curves, which makes sure the vertices are consistent with each other, @@ -109,6 +112,33 @@ pub enum HalfEdgeValidationError { /// The half-edge half_edge: HalfEdge, }, + + /// Mismatch between position and position of global form + #[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:#?}" + )] + PositionMismatch { + /// 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: SurfaceVertex, + }, } impl HalfEdgeValidationError { @@ -199,6 +229,36 @@ impl HalfEdgeValidationError { ); } } + + fn check_vertex_positions( + half_edge: &HalfEdge, + config: &ValidationConfig, + errors: &mut Vec, + ) { + for surface_vertex in half_edge.surface_vertices() { + let surface_position_as_global = surface_vertex + .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::PositionMismatch { + surface_position: surface_vertex.position(), + surface_position_as_global, + global_position, + distance, + surface_vertex: surface_vertex.clone_object(), + }) + .into(), + ); + } + } + } } #[cfg(test)] @@ -209,7 +269,7 @@ mod tests { use crate::{ builder::HalfEdgeBuilder, insert::Insert, - objects::{GlobalCurve, HalfEdge}, + objects::{GlobalCurve, HalfEdge, SurfaceVertex}, partial::{Partial, PartialHalfEdge, PartialObject}, services::Services, validate::Validate, @@ -372,4 +432,45 @@ mod tests { Ok(()) } + + #[test] + fn surface_vertex_position_mismatch() -> anyhow::Result<()> { + let mut services = Services::new(); + + let valid = { + let mut half_edge = PartialHalfEdge::default(); + half_edge.update_as_line_segment_from_points( + services.objects.surfaces.xy_plane(), + [[0., 0.], [1., 0.]], + ); + + half_edge.build(&mut services.objects) + }; + let invalid = { + let mut surface_vertices = + valid.surface_vertices().map(Clone::clone); + + let [_, surface_vertex] = surface_vertices.each_mut_ext(); + *surface_vertex = SurfaceVertex::new( + [2., 0.], + surface_vertex.surface().clone(), + surface_vertex.global_form().clone(), + ) + .insert(&mut services.objects); + + let boundary = valid.boundary().zip_ext(surface_vertices); + + HalfEdge::new( + valid.surface().clone(), + valid.curve().clone(), + boundary, + valid.global_form().clone(), + ) + }; + + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); + + Ok(()) + } } diff --git a/crates/fj-kernel/src/validate/vertex.rs b/crates/fj-kernel/src/validate/vertex.rs index edd00113e..ea4870b07 100644 --- a/crates/fj-kernel/src/validate/vertex.rs +++ b/crates/fj-kernel/src/validate/vertex.rs @@ -1,5 +1,3 @@ -use fj_math::{Point, Scalar}; - use crate::objects::{GlobalVertex, SurfaceVertex}; use super::{Validate, ValidationConfig, ValidationError}; @@ -7,10 +5,9 @@ use super::{Validate, ValidationConfig, ValidationError}; impl Validate for SurfaceVertex { fn validate_with_config( &self, - config: &ValidationConfig, - errors: &mut Vec, + _: &ValidationConfig, + _: &mut Vec, ) { - SurfaceVertexValidationError::check_position(self, config, errors); } } @@ -25,99 +22,4 @@ impl Validate for GlobalVertex { /// [`SurfaceVertex`] validation error #[derive(Clone, Debug, thiserror::Error)] -pub enum SurfaceVertexValidationError { - /// Mismatch between position and position of global form - #[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:#?}" - )] - PositionMismatch { - /// 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: SurfaceVertex, - }, -} - -impl SurfaceVertexValidationError { - fn check_position( - surface_vertex: &SurfaceVertex, - config: &ValidationConfig, - errors: &mut Vec, - ) { - let surface_position_as_global = surface_vertex - .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::PositionMismatch { - surface_position: surface_vertex.position(), - surface_position_as_global, - global_position, - distance, - surface_vertex: surface_vertex.clone(), - }) - .into(), - ); - } - } -} - -#[cfg(test)] -mod tests { - use fj_math::Point; - - use crate::{ - insert::Insert, - objects::{GlobalVertex, SurfaceVertex}, - partial::{ - Partial, PartialGlobalVertex, PartialObject, PartialSurfaceVertex, - }, - services::Services, - validate::Validate, - }; - - #[test] - fn surface_vertex_position_mismatch() -> anyhow::Result<()> { - let mut services = Services::new(); - - let valid = PartialSurfaceVertex { - position: Some([0., 0.].into()), - surface: Partial::from(services.objects.surfaces.xy_plane()), - global_form: Partial::from_partial(PartialGlobalVertex { - position: Some(Point::from([0., 0., 0.])), - }), - } - .build(&mut services.objects); - let invalid = SurfaceVertex::new( - valid.position(), - valid.surface().clone(), - GlobalVertex::new([1., 0., 0.]).insert(&mut services.objects), - ); - - valid.validate_and_return_first_error()?; - assert!(invalid.validate_and_return_first_error().is_err()); - - Ok(()) - } -} +pub enum SurfaceVertexValidationError {} From fe8c135b34ce14492f9b207b917419fc2bea5522 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 12:57:57 +0100 Subject: [PATCH 04/12] Remove unused code --- crates/fj-kernel/src/validate/mod.rs | 6 +----- crates/fj-kernel/src/validate/vertex.rs | 4 ---- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 2ae48ea9a..2a7dbc6b1 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -12,7 +12,7 @@ mod vertex; pub use self::{ cycle::CycleValidationError, edge::HalfEdgeValidationError, - face::FaceValidationError, vertex::SurfaceVertexValidationError, + face::FaceValidationError, }; use std::convert::Infallible; @@ -94,10 +94,6 @@ pub enum ValidationError { /// `HalfEdge` validation error #[error("`HalfEdge` validation error:\n{0}")] HalfEdge(#[from] Box), - - /// `SurfaceVertex` validation error - #[error("`SurfaceVertex` validation error:\n{0}")] - SurfaceVertex(#[from] Box), } impl From for ValidationError { diff --git a/crates/fj-kernel/src/validate/vertex.rs b/crates/fj-kernel/src/validate/vertex.rs index ea4870b07..bcad84f24 100644 --- a/crates/fj-kernel/src/validate/vertex.rs +++ b/crates/fj-kernel/src/validate/vertex.rs @@ -19,7 +19,3 @@ impl Validate for GlobalVertex { ) { } } - -/// [`SurfaceVertex`] validation error -#[derive(Clone, Debug, thiserror::Error)] -pub enum SurfaceVertexValidationError {} From c3c8c3be325741715927df60e7c4282ad21a62cb Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 12:58:23 +0100 Subject: [PATCH 05/12] Remove outdated comment --- crates/fj-kernel/src/validate/edge.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 5308897a6..1eee5bf6f 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -20,10 +20,6 @@ impl Validate for HalfEdge { HalfEdgeValidationError::check_surface_identity(self, errors); HalfEdgeValidationError::check_vertex_coincidence(self, config, errors); HalfEdgeValidationError::check_vertex_positions(self, config, errors); - - // We don't need to check anything about surfaces here. We already check - // curves, which makes sure the vertices are consistent with each other, - // and the validation of those vertices checks the surfaces. } } From 25fbec95e02ecdc1819ca72722e998de1cc36a41 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 12:58:56 +0100 Subject: [PATCH 06/12] Update name of enum variant --- crates/fj-kernel/src/validate/edge.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 1eee5bf6f..c99edbce8 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -119,7 +119,7 @@ pub enum HalfEdgeValidationError { - Distance between the positions: {distance}\n\ - `SurfaceVertex`: {surface_vertex:#?}" )] - PositionMismatch { + VertexSurfacePositionMismatch { /// The position of the surface vertex surface_position: Point<2>, @@ -243,7 +243,7 @@ impl HalfEdgeValidationError { if distance > config.identical_max_distance { errors.push( - Box::new(Self::PositionMismatch { + Box::new(Self::VertexSurfacePositionMismatch { surface_position: surface_vertex.position(), surface_position_as_global, global_position, From 5c71bc397b3248502489c66fa9cf88725fb8c8aa Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:06:05 +0100 Subject: [PATCH 07/12] Update doc comment --- crates/fj-kernel/src/validate/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index c99edbce8..ed80f68ce 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -109,7 +109,7 @@ pub enum HalfEdgeValidationError { half_edge: HalfEdge, }, - /// Mismatch between position and position of global form + /// Mismatch between [`SurfaceVertex`] and [`GlobalVertex`] positions #[error( "`SurfaceVertex` position doesn't match position of its global form\n\ - Surface position: {surface_position:?}\n\ From cd430cdf1cefb670bc7d0eeb677edbf9d2772945 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:06:55 +0100 Subject: [PATCH 08/12] Add more information to error message --- crates/fj-kernel/src/validate/edge.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index ed80f68ce..f11d9f7b4 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -133,7 +133,7 @@ pub enum HalfEdgeValidationError { distance: Scalar, /// The surface vertex - surface_vertex: SurfaceVertex, + surface_vertex: Handle, }, } @@ -248,7 +248,7 @@ impl HalfEdgeValidationError { surface_position_as_global, global_position, distance, - surface_vertex: surface_vertex.clone_object(), + surface_vertex: surface_vertex.clone(), }) .into(), ); From 924c9c7f00d11c93d326331126235123f3cb29fe Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:08:23 +0100 Subject: [PATCH 09/12] Avoid use of `SurfaceVertex::surface` --- crates/fj-kernel/src/validate/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index f11d9f7b4..e0f7caac3 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -232,7 +232,7 @@ impl HalfEdgeValidationError { errors: &mut Vec, ) { for surface_vertex in half_edge.surface_vertices() { - let surface_position_as_global = surface_vertex + let surface_position_as_global = half_edge .surface() .geometry() .point_from_surface_coords(surface_vertex.position()); From 82431cde1c1c8bef5a706a9dbc5963fde4865560 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:10:01 +0100 Subject: [PATCH 10/12] Remove reference to `Surface` from `SurfaceVertex` It is already available through `HalfEdge`. This reduces the redundancy within the object graph and removes the need for a validation check. --- .../src/algorithms/transform/vertex.rs | 6 +- crates/fj-kernel/src/builder/edge.rs | 6 +- crates/fj-kernel/src/objects/full/vertex.rs | 10 +--- crates/fj-kernel/src/partial/objects/edge.rs | 1 - .../fj-kernel/src/partial/objects/vertex.rs | 12 +--- crates/fj-kernel/src/validate/edge.rs | 60 ------------------- 6 files changed, 5 insertions(+), 90 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/transform/vertex.rs b/crates/fj-kernel/src/algorithms/transform/vertex.rs index 8777cf9f8..669f2f644 100644 --- a/crates/fj-kernel/src/algorithms/transform/vertex.rs +++ b/crates/fj-kernel/src/algorithms/transform/vertex.rs @@ -18,16 +18,12 @@ impl TransformObject for SurfaceVertex { // coordinates and thus transforming the surface takes care of it. let position = self.position(); - let surface = self - .surface() - .clone() - .transform_with_cache(transform, objects, cache); let global_form = self .global_form() .clone() .transform_with_cache(transform, objects, cache); - Self::new(position, surface, global_form) + Self::new(position, global_form) } } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 6a1e920a0..c500ab756 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -64,11 +64,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { fn replace_surface(&mut self, surface: impl Into>) { let surface = surface.into(); - self.surface = surface.clone(); - - for vertex in &mut self.vertices { - vertex.1.write().surface = surface.clone(); - } + self.surface = surface; } fn update_as_circle_from_radius(&mut self, radius: impl Into) { diff --git a/crates/fj-kernel/src/objects/full/vertex.rs b/crates/fj-kernel/src/objects/full/vertex.rs index 1fee8ba05..8c222a1e7 100644 --- a/crates/fj-kernel/src/objects/full/vertex.rs +++ b/crates/fj-kernel/src/objects/full/vertex.rs @@ -1,12 +1,11 @@ use fj_math::Point; -use crate::{objects::Surface, storage::Handle}; +use crate::storage::Handle; /// A vertex, defined in surface (2D) coordinates #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct SurfaceVertex { position: Point<2>, - surface: Handle, global_form: Handle, } @@ -14,13 +13,11 @@ impl SurfaceVertex { /// Construct a new instance of `SurfaceVertex` pub fn new( position: impl Into>, - surface: Handle, global_form: Handle, ) -> Self { let position = position.into(); Self { position, - surface, global_form, } } @@ -30,11 +27,6 @@ impl SurfaceVertex { self.position } - /// Access the surface that the vertex is defined in - pub fn surface(&self) -> &Handle { - &self.surface - } - /// Access the global form of the vertex pub fn global_form(&self) -> &Handle { &self.global_form diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index ff0addbbb..6ee0ad8de 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -107,7 +107,6 @@ impl Default for PartialHalfEdge { let curve = Partial::::new(); let vertices = array::from_fn(|_| { let surface_form = Partial::from_partial(PartialSurfaceVertex { - surface: surface.clone(), ..Default::default() }); diff --git a/crates/fj-kernel/src/partial/objects/vertex.rs b/crates/fj-kernel/src/partial/objects/vertex.rs index 74c0694e4..9e9051ee9 100644 --- a/crates/fj-kernel/src/partial/objects/vertex.rs +++ b/crates/fj-kernel/src/partial/objects/vertex.rs @@ -1,7 +1,7 @@ use fj_math::Point; use crate::{ - objects::{GlobalVertex, Objects, Surface, SurfaceVertex}, + objects::{GlobalVertex, Objects, SurfaceVertex}, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, }; @@ -12,9 +12,6 @@ pub struct PartialSurfaceVertex { /// The position of the vertex on the surface pub position: Option>, - /// The surface that the vertex is defined in - pub surface: Partial, - /// The global form of the vertex pub global_form: Partial, } @@ -28,10 +25,6 @@ impl PartialObject for PartialSurfaceVertex { ) -> Self { Self { position: Some(surface_vertex.position()), - surface: Partial::from_full( - surface_vertex.surface().clone(), - cache, - ), global_form: Partial::from_full( surface_vertex.global_form().clone(), cache, @@ -43,10 +36,9 @@ impl PartialObject for PartialSurfaceVertex { let position = self .position .expect("Can't build `SurfaceVertex` without position"); - let surface = self.surface.build(objects); let global_form = self.global_form.build(objects); - SurfaceVertex::new(position, surface, global_form) + SurfaceVertex::new(position, global_form) } } diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index e0f7caac3..d5658975f 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -17,7 +17,6 @@ impl Validate for HalfEdge { ) { HalfEdgeValidationError::check_global_curve_identity(self, errors); HalfEdgeValidationError::check_global_vertex_identity(self, errors); - HalfEdgeValidationError::check_surface_identity(self, errors); HalfEdgeValidationError::check_vertex_coincidence(self, config, errors); HalfEdgeValidationError::check_vertex_positions(self, config, errors); } @@ -187,24 +186,6 @@ impl HalfEdgeValidationError { } } - fn check_surface_identity( - half_edge: &HalfEdge, - errors: &mut Vec, - ) { - let surface = half_edge.surface(); - let surface_form_surface = half_edge.start_vertex().surface(); - - if surface.id() != surface_form_surface.id() { - errors.push( - Box::new(Self::SurfaceMismatch { - curve_surface: surface.clone(), - surface_form_surface: surface_form_surface.clone(), - }) - .into(), - ); - } - } - fn check_vertex_coincidence( half_edge: &HalfEdge, config: &ValidationConfig, @@ -357,46 +338,6 @@ mod tests { Ok(()) } - #[test] - fn vertex_surface_mismatch() -> anyhow::Result<()> { - let mut services = Services::new(); - - let valid = { - let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), - [[0., 0.], [1., 0.]], - ); - - half_edge.build(&mut services.objects) - }; - let invalid = { - let vertices = valid - .boundary() - .zip_ext(valid.surface_vertices()) - .map(|(point, surface_vertex)| { - let mut surface_vertex = - Partial::from(surface_vertex.clone()); - surface_vertex.write().surface = - Partial::from(services.objects.surfaces.xz_plane()); - - (point, surface_vertex.build(&mut services.objects)) - }); - - HalfEdge::new( - valid.surface().clone(), - valid.curve().clone(), - vertices, - valid.global_form().clone(), - ) - }; - - valid.validate_and_return_first_error()?; - assert!(invalid.validate_and_return_first_error().is_err()); - - Ok(()) - } - #[test] fn half_edge_vertices_are_coincident() -> anyhow::Result<()> { let mut services = Services::new(); @@ -449,7 +390,6 @@ mod tests { let [_, surface_vertex] = surface_vertices.each_mut_ext(); *surface_vertex = SurfaceVertex::new( [2., 0.], - surface_vertex.surface().clone(), surface_vertex.global_form().clone(), ) .insert(&mut services.objects); From d8f6a691da20de433ae41ac22f5c415f78e85b02 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:11:52 +0100 Subject: [PATCH 11/12] Simplify `PartialSurfaceVertex` construction --- crates/fj-kernel/src/partial/objects/edge.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 6ee0ad8de..a0057c85a 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -8,9 +8,7 @@ use crate::{ Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, Surface, SurfaceVertex, }, - partial::{ - FullToPartialCache, Partial, PartialObject, PartialSurfaceVertex, - }, + partial::{FullToPartialCache, Partial, PartialObject}, services::Service, }; @@ -106,10 +104,7 @@ impl Default for PartialHalfEdge { let curve = Partial::::new(); let vertices = array::from_fn(|_| { - let surface_form = Partial::from_partial(PartialSurfaceVertex { - ..Default::default() - }); - + let surface_form = Partial::default(); (None, surface_form) }); From c55abb0f9c8458ebecf805a953e0a5a54d347560 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 13:15:15 +0100 Subject: [PATCH 12/12] Remove `HalfEdgeBuilder::replace_surface` Thanks to the recent simplifications, it was no longer carrying its weight. --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 18 ++++++++++++------ crates/fj-kernel/src/builder/cycle.rs | 2 +- crates/fj-kernel/src/builder/edge.rs | 18 +----------------- crates/fj-operations/src/sketch.rs | 6 ++++-- 4 files changed, 18 insertions(+), 26 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 7086b20f7..488aeb75e 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -196,8 +196,10 @@ mod tests { half_edge }; let side_up = { - let mut side_up = PartialHalfEdge::default(); - side_up.replace_surface(surface.clone()); + let mut side_up = PartialHalfEdge { + surface: surface.clone(), + ..Default::default() + }; { let [back, front] = side_up @@ -217,8 +219,10 @@ mod tests { side_up }; let top = { - let mut top = PartialHalfEdge::default(); - top.replace_surface(surface.clone()); + let mut top = PartialHalfEdge { + surface: surface.clone(), + ..Default::default() + }; { let [(back, back_surface), (front, front_surface)] = @@ -243,8 +247,10 @@ mod tests { .clone() }; let side_down = { - let mut side_down = PartialHalfEdge::default(); - side_down.replace_surface(surface); + let mut side_down = PartialHalfEdge { + surface, + ..Default::default() + }; let [(back, back_surface), (front, front_surface)] = side_down.vertices.each_mut_ext(); diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 158b057bb..3e7202905 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -149,7 +149,7 @@ impl CycleBuilder for PartialCycle { let [_, vertex] = &mut new_half_edge.vertices; vertex.1 = shared_surface_vertex; - new_half_edge.replace_surface(self.surface.clone()); + new_half_edge.surface = self.surface.clone(); new_half_edge.infer_global_form(); } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index c500ab756..78f85e921 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -11,16 +11,6 @@ use super::CurveBuilder; /// Builder API for [`PartialHalfEdge`] pub trait HalfEdgeBuilder { - /// Completely replace the surface in this half-edge's object graph - /// - /// Please note that this operation will write to both vertices that the - /// half-edge references. If any of them were created from full objects, - /// this will break the connection to those, meaning that building the - /// partial objects won't result in those full objects again. This will be - /// the case, even if those full objects already referenced the provided - /// surface. - fn replace_surface(&mut self, surface: impl Into>); - /// Update partial half-edge to be a circle, from the given radius fn update_as_circle_from_radius(&mut self, radius: impl Into); @@ -61,12 +51,6 @@ pub trait HalfEdgeBuilder { } impl HalfEdgeBuilder for PartialHalfEdge { - fn replace_surface(&mut self, surface: impl Into>) { - let surface = surface.into(); - - self.surface = surface; - } - fn update_as_circle_from_radius(&mut self, radius: impl Into) { let path = self.curve.write().update_as_circle_from_radius(radius); @@ -130,7 +114,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { surface: impl Into>, points: [impl Into>; 2], ) { - self.replace_surface(surface.into()); + self.surface = surface.into(); for (vertex, point) in self.vertices.each_mut_ext().zip_ext(points) { let mut surface_form = vertex.1.write(); diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 54c736008..a790d391c 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -30,8 +30,10 @@ impl Shape for fj::Sketch { let half_edge = { let surface = Partial::from(surface); - let mut half_edge = PartialHalfEdge::default(); - half_edge.replace_surface(surface); + let mut half_edge = PartialHalfEdge { + surface, + ..Default::default() + }; half_edge.update_as_circle_from_radius(circle.radius()); Partial::from_partial(half_edge)