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/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/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 6a1e920a0..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,16 +51,6 @@ pub trait HalfEdgeBuilder { } 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(); - } - } - fn update_as_circle_from_radius(&mut self, radius: impl Into) { let path = self.curve.write().update_as_circle_from_radius(radius); @@ -134,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-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`] 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..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,11 +104,7 @@ 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() - }); - + let surface_form = Partial::default(); (None, surface_form) }); 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 ed434ea1a..d5658975f 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, }; @@ -15,12 +17,8 @@ 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); - - // 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. } } @@ -109,6 +107,33 @@ pub enum HalfEdgeValidationError { /// The half-edge half_edge: HalfEdge, }, + + /// 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 HalfEdgeValidationError { @@ -161,25 +186,7 @@ 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_positions( + fn check_vertex_coincidence( half_edge: &HalfEdge, config: &ValidationConfig, errors: &mut Vec, @@ -199,6 +206,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 = half_edge + .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)] @@ -209,7 +246,7 @@ mod tests { use crate::{ builder::HalfEdgeBuilder, insert::Insert, - objects::{GlobalCurve, HalfEdge}, + objects::{GlobalCurve, HalfEdge, SurfaceVertex}, partial::{Partial, PartialHalfEdge, PartialObject}, services::Services, validate::Validate, @@ -302,7 +339,7 @@ mod tests { } #[test] - fn vertex_surface_mismatch() -> anyhow::Result<()> { + fn half_edge_vertices_are_coincident() -> anyhow::Result<()> { let mut services = Services::new(); let valid = { @@ -315,17 +352,9 @@ mod tests { 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)) - }); + let vertices = valid.surface_vertices().map(|surface_vertex| { + (Point::from([0.]), surface_vertex.clone()) + }); HalfEdge::new( valid.surface().clone(), @@ -342,7 +371,7 @@ mod tests { } #[test] - fn half_edge_vertices_are_coincident() -> anyhow::Result<()> { + fn surface_vertex_position_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); let valid = { @@ -355,14 +384,22 @@ mod tests { half_edge.build(&mut services.objects) }; let invalid = { - let vertices = valid.surface_vertices().map(|surface_vertex| { - (Point::from([0.]), surface_vertex.clone()) - }); + 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.global_form().clone(), + ) + .insert(&mut services.objects); + + let boundary = valid.boundary().zip_ext(surface_vertices); HalfEdge::new( valid.surface().clone(), valid.curve().clone(), - vertices, + boundary, valid.global_form().clone(), ) }; 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 edd00113e..bcad84f24 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); } } @@ -22,102 +19,3 @@ 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(()) - } -} 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)