diff --git a/crates/fj-kernel/src/algorithms/intersect/face_point.rs b/crates/fj-kernel/src/algorithms/intersect/face_point.rs index 698999e6c..b32f10ae4 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_point.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_point.rs @@ -125,7 +125,7 @@ pub enum FacePointIntersection { PointIsOnEdge(Handle), /// The point is coincident with a vertex - PointIsOnVertex(Handle), + PointIsOnVertex(Vertex), } #[cfg(test)] @@ -346,10 +346,11 @@ mod tests { .find(|vertex| { vertex.surface_form().position() == Point::from([1., 0.]) }) - .unwrap(); + .unwrap() + .clone(); assert_eq!( intersection, - Some(FacePointIntersection::PointIsOnVertex(vertex.clone())) + Some(FacePointIntersection::PointIsOnVertex(vertex)) ); } } diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs index 7565a50ef..08d460448 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -136,7 +136,7 @@ pub enum RayFaceIntersection { RayHitsEdge(Handle), /// The ray hits a vertex - RayHitsVertex(Handle), + RayHitsVertex(Vertex), } #[cfg(test)] @@ -291,10 +291,11 @@ mod tests { .find(|vertex| { vertex.surface_form().position() == Point::from([-1., -1.]) }) - .unwrap(); + .unwrap() + .clone(); assert_eq!( (&ray, &face).intersect(), - Some(RayFaceIntersection::RayHitsVertex(vertex.clone())) + Some(RayFaceIntersection::RayHitsVertex(vertex)) ); } diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 3045830f0..1641c683f 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -81,7 +81,6 @@ impl Sweep for (Handle, Color) { curve.clone(), surface_vertex, ) - .insert(objects) }) }; @@ -138,7 +137,6 @@ impl Sweep for (Handle, Color) { .collect::<[_; 2]>() .map(|(vertex, surface_form)| { Vertex::new(vertex.position(), curve.clone(), surface_form) - .insert(objects) }); HalfEdge::new(vertices, global).insert(objects) @@ -236,10 +234,8 @@ mod tests { { let [back, front] = &mut side_up.vertices; - back.write().surface_form = - bottom.vertices[1].read().surface_form.clone(); + back.surface_form = bottom.vertices[1].surface_form.clone(); - let mut front = front.write(); let mut front = front.surface_form.write(); front.position = Some([1., 1.].into()); front.surface = surface.clone(); @@ -257,13 +253,12 @@ mod tests { { let [back, front] = &mut top.vertices; - let mut back = back.write(); let mut back = back.surface_form.write(); back.position = Some([0., 1.].into()); back.surface = surface.clone(); - front.write().surface_form = - side_up.vertices[1].read().surface_form.clone(); + front.surface_form = + side_up.vertices[1].surface_form.clone(); } top.infer_global_form(); @@ -283,10 +278,8 @@ mod tests { let [back, front] = &mut side_down.vertices; - back.write().surface_form = - bottom.vertices[0].read().surface_form.clone(); - front.write().surface_form = - top.vertices[1].read().surface_form.clone(); + back.surface_form = bottom.vertices[0].surface_form.clone(); + front.surface_form = top.vertices[1].surface_form.clone(); side_down.infer_global_form(); side_down.update_as_line_segment(); diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index 551e64717..dee108bfb 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -13,7 +13,7 @@ use crate::{ use super::{Sweep, SweepCache}; -impl Sweep for (Handle, Handle) { +impl Sweep for (Vertex, Handle) { type Swept = Handle; fn sweep_with_cache( @@ -112,7 +112,6 @@ impl Sweep for (Handle, Handle) { curve.clone(), surface_form, ) - .insert(objects) }); // And finally, creating the output `Edge` is just a matter of @@ -188,8 +187,7 @@ mod tests { ..Default::default() }), } - .build(&mut services.objects) - .insert(&mut services.objects); + .build(&mut services.objects); let half_edge = (vertex, surface.clone()) .sweep([0., 0., 1.], &mut services.objects); diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 10ce138b2..a6dc84c92 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -99,23 +99,21 @@ impl CycleBuilder for PartialCycle { { let shared_surface_vertex = - new_half_edge.read().back().read().surface_form.clone(); + new_half_edge.read().back().surface_form.clone(); let mut last_half_edge = last_half_edge.write(); - last_half_edge.front_mut().write().surface_form = - shared_surface_vertex; + last_half_edge.front_mut().surface_form = shared_surface_vertex; last_half_edge.infer_global_form(); } { let shared_surface_vertex = - first_half_edge.read().back().read().surface_form.clone(); + first_half_edge.read().back().surface_form.clone(); let mut new_half_edge = new_half_edge.write(); - new_half_edge.front_mut().write().surface_form = - shared_surface_vertex; + new_half_edge.front_mut().surface_form = shared_surface_vertex; new_half_edge.replace_surface(self.surface.clone()); new_half_edge.infer_global_form(); } @@ -130,13 +128,8 @@ impl CycleBuilder for PartialCycle { ) -> Partial { let mut half_edge = self.add_half_edge(); - half_edge - .write() - .back_mut() - .write() - .surface_form - .write() - .position = Some(point.into()); + half_edge.write().back_mut().surface_form.write().position = + Some(point.into()); half_edge } @@ -150,7 +143,6 @@ impl CycleBuilder for PartialCycle { half_edge .write() .back_mut() - .write() .surface_form .write() .global_form @@ -198,7 +190,6 @@ impl CycleBuilder for PartialCycle { half_edge .write() .back_mut() - .write() .surface_form .write() .global_form diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 80ee44a89..c2ed2c1ef 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -59,7 +59,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { let surface = surface.into(); for vertex in &mut self.vertices { - vertex.write().replace_surface(surface.clone()); + vertex.replace_surface(surface.clone()); } } @@ -72,7 +72,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { let mut surface_vertex = { let [vertex, _] = &mut self.vertices; - vertex.write().surface_form.clone() + vertex.surface_form.clone() }; surface_vertex.write().position = Some(path.point_from_path_coords(a_curve)); @@ -80,7 +80,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { for (vertex, point_curve) in self.vertices.each_mut_ext().zip_ext([a_curve, b_curve]) { - let mut vertex = vertex.write(); + let mut vertex = vertex; vertex.position = Some(point_curve); vertex.surface_form = surface_vertex.clone(); } @@ -95,7 +95,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { } let points_surface = self.vertices.each_ref_ext().map(|vertex| { vertex - .read() .surface_form .read() .position @@ -123,7 +122,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { for (vertex, point_curve) in self.vertices.each_mut_ext().zip_ext([a_curve, b_curve]) { - let mut vertex = vertex.write(); vertex.position = Some(point_curve); vertex.surface_form.write().position = Some(path.point_from_path_coords(point_curve)); @@ -140,7 +138,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { let surface = surface.into(); for (vertex, point) in self.vertices.each_mut_ext().zip_ext(points) { - let mut vertex = vertex.write(); vertex.curve.write().surface = surface.clone(); let mut surface_form = vertex.surface_form.write(); @@ -154,7 +151,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { fn update_as_line_segment(&mut self) { let points_surface = self.vertices.each_ref_ext().map(|vertex| { vertex - .read() .surface_form .read() .position @@ -167,7 +163,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { for (vertex, position) in self.vertices.each_mut_ext().zip_ext([0., 1.]) { - vertex.write().position = Some([position].into()); + vertex.position = Some([position].into()); } self.infer_global_form(); @@ -176,10 +172,10 @@ impl HalfEdgeBuilder for PartialHalfEdge { fn infer_global_form(&mut self) -> Partial { self.global_form.write().curve = self.curve().read().global_form.clone(); - self.global_form.write().vertices = - self.vertices.each_ref_ext().map(|vertex| { - vertex.read().surface_form.read().global_form.clone() - }); + self.global_form.write().vertices = self + .vertices + .each_ref_ext() + .map(|vertex| vertex.surface_form.read().global_form.clone()); self.global_form.clone() } @@ -202,13 +198,8 @@ impl HalfEdgeBuilder for PartialHalfEdge { .iter_mut() .zip(other.read().vertices.iter().rev()) { - this.write() - .surface_form - .write() - .global_form - .write() - .position = - other.read().surface_form.read().global_form.read().position; + this.surface_form.write().global_form.write().position = + other.surface_form.read().global_form.read().position; } } } diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index d3ebf4432..358ffd625 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -109,9 +109,10 @@ impl FaceBuilder for PartialFace { fn update_surface_as_plane(&mut self) -> Partial { let mut exterior = self.exterior.write(); - let mut vertices = exterior.half_edges.iter().map(|half_edge| { - half_edge.read().back().read().surface_form.clone() - }); + let mut vertices = exterior + .half_edges + .iter() + .map(|half_edge| half_edge.read().back().surface_form.clone()); let vertices = { let array = [ @@ -164,12 +165,7 @@ impl FaceBuilder for PartialFace { MaybeSurfacePath::UndefinedLine => { let points_surface = half_edge.vertices.each_ref_ext().map(|vertex| { - vertex - .read() - .surface_form - .read() - .position - .expect( + vertex.surface_form.read().position.expect( "Can't infer curve without surface points", ) }); @@ -182,7 +178,7 @@ impl FaceBuilder for PartialFace { .each_mut_ext() .zip_ext(points_curve) { - vertex.write().position = Some(point); + vertex.position = Some(point); } } } diff --git a/crates/fj-kernel/src/insert.rs b/crates/fj-kernel/src/insert.rs index f4038e61c..98831ad34 100644 --- a/crates/fj-kernel/src/insert.rs +++ b/crates/fj-kernel/src/insert.rs @@ -5,7 +5,7 @@ use crate::{ objects::{ Curve, Cycle, Face, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, - Objects, Shell, Sketch, Solid, Surface, SurfaceVertex, Vertex, + Objects, Shell, Sketch, Solid, Surface, SurfaceVertex, }, services::{Service, ServiceObjectsExt}, storage::Handle, @@ -46,5 +46,4 @@ impl_insert!( Solid, solids; SurfaceVertex, surface_vertices; Surface, surfaces; - Vertex, vertices; ); diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 1cb588d35..782d72bd6 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -8,16 +8,13 @@ use crate::{ /// A half-edge #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdge { - vertices: [Handle; 2], + vertices: [Vertex; 2], global_form: Handle, } impl HalfEdge { /// Create an instance of `HalfEdge` - pub fn new( - vertices: [Handle; 2], - global_form: Handle, - ) -> Self { + pub fn new(vertices: [Vertex; 2], global_form: Handle) -> Self { Self { vertices, global_form, @@ -31,18 +28,18 @@ impl HalfEdge { } /// Access the vertices that bound the half-edge on the curve - pub fn vertices(&self) -> &[Handle; 2] { + pub fn vertices(&self) -> &[Vertex; 2] { &self.vertices } /// Access the vertex at the back of the half-edge - pub fn back(&self) -> &Handle { + pub fn back(&self) -> &Vertex { let [back, _] = self.vertices(); back } /// Access the vertex at the front of the half-edge - pub fn front(&self) -> &Handle { + pub fn front(&self) -> &Vertex { let [_, front] = self.vertices(); front } diff --git a/crates/fj-kernel/src/objects/object.rs b/crates/fj-kernel/src/objects/object.rs index 0c0e98133..2bf627d87 100644 --- a/crates/fj-kernel/src/objects/object.rs +++ b/crates/fj-kernel/src/objects/object.rs @@ -3,7 +3,7 @@ use std::any::Any; use crate::{ objects::{ Curve, Cycle, Face, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, - Objects, Shell, Sketch, Solid, Surface, SurfaceVertex, Vertex, + Objects, Shell, Sketch, Solid, Surface, SurfaceVertex, }, storage::{Handle, ObjectId}, validate::{Validate, ValidationError}, @@ -120,7 +120,6 @@ object!( Solid, "solid", solids; Surface, "surface", surfaces; SurfaceVertex, "surface vertex", surface_vertices; - Vertex, "vertex", vertices; ); /// The form that an object can take diff --git a/crates/fj-kernel/src/objects/stores.rs b/crates/fj-kernel/src/objects/stores.rs index 1f5adf652..3cda7ba09 100644 --- a/crates/fj-kernel/src/objects/stores.rs +++ b/crates/fj-kernel/src/objects/stores.rs @@ -7,7 +7,7 @@ use crate::{ use super::{ Curve, Cycle, Face, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Shell, - Sketch, Solid, Surface, SurfaceVertex, Vertex, + Sketch, Solid, Surface, SurfaceVertex, }; /// The available object stores @@ -55,9 +55,6 @@ pub struct Objects { /// Store for [`Surface`]s pub surfaces: Surfaces, - - /// Store for [`Vertex`] objects - pub vertices: Store, } impl Objects { diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 7aaeacf5b..5b8b0ef14 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -4,7 +4,7 @@ use fj_interop::ext::ArrayExt; use crate::{ objects::{ - Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, Vertex, + Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, }, partial::{FullToPartialCache, Partial, PartialObject, PartialVertex}, services::Service, @@ -14,7 +14,7 @@ use crate::{ #[derive(Clone, Debug)] pub struct PartialHalfEdge { /// The vertices that bound the half-edge on the curve - pub vertices: [Partial; 2], + pub vertices: [PartialVertex; 2], /// The global form of the half-edge pub global_form: Partial, @@ -24,29 +24,29 @@ impl PartialHalfEdge { /// Access the curve the partial edge is defined on pub fn curve(&self) -> Partial { let [vertex, _] = &self.vertices; - vertex.read().curve.clone() + vertex.curve.clone() } /// Access a reference to the half-edge's back vertex - pub fn back(&self) -> &Partial { + pub fn back(&self) -> &PartialVertex { let [back, _] = &self.vertices; back } /// Access a reference to the half-edge's front vertex - pub fn front(&self) -> &Partial { + pub fn front(&self) -> &PartialVertex { let [_, front] = &self.vertices; front } /// Access a mutable reference to the half-edge's back vertex - pub fn back_mut(&mut self) -> &mut Partial { + pub fn back_mut(&mut self) -> &mut PartialVertex { let [back, _] = &mut self.vertices; back } /// Access a mutable reference to the half-edge's front vertex - pub fn front_mut(&mut self) -> &mut Partial { + pub fn front_mut(&mut self) -> &mut PartialVertex { let [_, front] = &mut self.vertices; front } @@ -63,7 +63,7 @@ impl PartialObject for PartialHalfEdge { vertices: half_edge .vertices() .clone() - .map(|vertex| Partial::from_full(vertex, cache)), + .map(|vertex| PartialVertex::from_full(&vertex, cache)), global_form: Partial::from_full( half_edge.global_form().clone(), cache, @@ -82,17 +82,15 @@ impl PartialObject for PartialHalfEdge { impl Default for PartialHalfEdge { fn default() -> Self { let curve = Partial::::new(); - let vertices = array::from_fn(|_| { - Partial::from_partial(PartialVertex { - curve: curve.clone(), - ..Default::default() - }) + let vertices = array::from_fn(|_| PartialVertex { + curve: curve.clone(), + ..Default::default() }); let global_curve = curve.read().global_form.clone(); let global_vertices = - vertices.each_ref_ext().map(|vertex: &Partial| { - let surface_vertex = vertex.read().surface_form.clone(); + vertices.each_ref_ext().map(|vertex: &PartialVertex| { + let surface_vertex = vertex.surface_form.clone(); let global_vertex = surface_vertex.read().global_form.clone(); global_vertex }); diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 89a9b030c..41b3e2373 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -98,9 +98,9 @@ mod tests { let first_half_edge = half_edges.first_mut().unwrap(); let [first_vertex, _] = &mut first_half_edge.write().vertices; let surface_vertex = Partial::from_partial( - first_vertex.read().surface_form.read().clone(), + first_vertex.surface_form.read().clone(), ); - first_vertex.write().surface_form = surface_vertex; + first_vertex.surface_form = surface_vertex; } let half_edges = half_edges diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index fc6b18045..1997c14d7 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -3,8 +3,8 @@ use fj_math::{Point, Scalar}; use crate::{ objects::{ - Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, - VerticesInNormalizedOrder, + Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface, + Vertex, VerticesInNormalizedOrder, }, storage::Handle, }; @@ -20,7 +20,9 @@ impl Validate for HalfEdge { HalfEdgeValidationError::check_curve_identity(self, errors); 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_position(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, @@ -98,6 +100,20 @@ pub enum HalfEdgeValidationError { half_edge: HalfEdge, }, + /// Mismatch between the surface's of the curve and surface form + #[error( + "Surface form of vertex must be defined on same surface as curve\n\ + - `Surface` of curve: {curve_surface:#?}\n\ + - `Surface` of surface form: {surface_form_surface:#?}" + )] + SurfaceMismatch { + /// The surface of the vertex' curve + curve_surface: Handle, + + /// The surface of the vertex' surface form + surface_form_surface: Handle, + }, + /// [`HalfEdge`]'s vertices are coincident #[error( "Vertices of `HalfEdge` on curve are coincident\n\ @@ -118,6 +134,24 @@ pub enum HalfEdgeValidationError { /// The half-edge half_edge: HalfEdge, }, + + /// Mismatch between position of the vertex and position of its surface form + #[error( + "`Vertex` position doesn't match position of its surface form\n\ + - `Vertex`: {vertex:#?}\n\ + - `Vertex` position on surface: {curve_position_on_surface:?}\n\ + - Distance between the positions: {distance}" + )] + VertexPositionMismatch { + /// The vertex + vertex: Vertex, + + /// The curve position converted into a surface position + curve_position_on_surface: Point<2>, + + /// The distance between the positions + distance: Scalar, + }, } impl HalfEdgeValidationError { @@ -198,6 +232,27 @@ impl HalfEdgeValidationError { } } + fn check_surface_identity( + half_edge: &HalfEdge, + errors: &mut Vec, + ) { + let curve_surface = half_edge.curve().surface(); + + for vertex in half_edge.vertices() { + let surface_form_surface = vertex.surface_form().surface(); + + if curve_surface.id() != surface_form_surface.id() { + errors.push( + Box::new(Self::SurfaceMismatch { + curve_surface: curve_surface.clone(), + surface_form_surface: surface_form_surface.clone(), + }) + .into(), + ); + } + } + } + fn check_vertex_positions( half_edge: &HalfEdge, config: &ValidationConfig, @@ -220,17 +275,47 @@ impl HalfEdgeValidationError { ); } } + + fn check_position( + half_edge: &HalfEdge, + config: &ValidationConfig, + errors: &mut Vec, + ) { + for vertex in half_edge.vertices() { + let curve_position_on_surface = vertex + .curve() + .path() + .point_from_path_coords(vertex.position()); + let surface_position = vertex.surface_form().position(); + + let distance = + curve_position_on_surface.distance_to(&surface_position); + + if distance > config.identical_max_distance { + errors.push( + Box::new(Self::VertexPositionMismatch { + vertex: vertex.clone(), + curve_position_on_surface, + distance, + }) + .into(), + ); + } + } + } } #[cfg(test)] mod tests { + use fj_math::Point; + use crate::{ builder::HalfEdgeBuilder, insert::Insert, objects::{GlobalCurve, HalfEdge}, partial::{ - Partial, PartialHalfEdge, PartialObject, PartialSurfaceVertex, - PartialVertex, + FullToPartialCache, Partial, PartialHalfEdge, PartialObject, + PartialSurfaceVertex, PartialVertex, }, services::Services, validate::Validate, @@ -251,9 +336,12 @@ mod tests { }; let invalid = { let mut vertices = valid.vertices().clone(); - let mut vertex = Partial::from(vertices[1].clone()); + let mut vertex = PartialVertex::from_full( + &vertices[1], + &mut FullToPartialCache::default(), + ); // Arranging for an equal but not identical curve here. - vertex.write().curve = Partial::from_partial( + vertex.curve = Partial::from_partial( Partial::from(valid.curve().clone()).read().clone(), ); vertices[1] = vertex.build(&mut services.objects); @@ -325,6 +413,40 @@ 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.vertices().clone().map(|vertex| { + let mut vertex = PartialVertex::from_full( + &vertex, + &mut FullToPartialCache::default(), + ); + vertex.surface_form.write().surface = + Partial::from(services.objects.surfaces.xz_plane()); + + vertex.build(&mut services.objects) + }); + + HalfEdge::new(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(); @@ -340,7 +462,10 @@ mod tests { }; let invalid = HalfEdge::new( valid.vertices().clone().map(|vertex| { - let vertex = Partial::from(vertex).read().clone(); + let vertex = PartialVertex::from_full( + &vertex, + &mut FullToPartialCache::default(), + ); let surface = vertex.surface_form.read().surface.clone(); PartialVertex { position: Some([0.].into()), @@ -351,7 +476,6 @@ mod tests { ..vertex } .build(&mut services.objects) - .insert(&mut services.objects) }), valid.global_form().clone(), ); @@ -361,4 +485,37 @@ mod tests { Ok(()) } + + #[test] + fn 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 vertices = valid.vertices().clone().map(|vertex| { + let mut vertex = PartialVertex::from_full( + &vertex, + &mut FullToPartialCache::default(), + ); + vertex.position = Some(Point::from([2.])); + + vertex.build(&mut services.objects) + }); + + HalfEdge::new(vertices, 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/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 3e3585307..c37cbad36 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -11,10 +11,8 @@ mod surface; mod vertex; pub use self::{ - cycle::CycleValidationError, - edge::HalfEdgeValidationError, - face::FaceValidationError, - vertex::{SurfaceVertexValidationError, VertexValidationError}, + cycle::CycleValidationError, edge::HalfEdgeValidationError, + face::FaceValidationError, vertex::SurfaceVertexValidationError, }; use std::convert::Infallible; @@ -100,10 +98,6 @@ pub enum ValidationError { /// `SurfaceVertex` validation error #[error(transparent)] SurfaceVertex(#[from] Box), - - /// `Vertex` validation error - #[error(transparent)] - Vertex(#[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 f5195577b..7dbd1f049 100644 --- a/crates/fj-kernel/src/validate/vertex.rs +++ b/crates/fj-kernel/src/validate/vertex.rs @@ -1,20 +1,15 @@ use fj_math::{Point, Scalar}; -use crate::{ - objects::{GlobalVertex, Surface, SurfaceVertex, Vertex}, - storage::Handle, -}; +use crate::objects::{GlobalVertex, SurfaceVertex, Vertex}; use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Vertex { fn validate_with_config( &self, - config: &ValidationConfig, - errors: &mut Vec, + _: &ValidationConfig, + _: &mut Vec, ) { - VertexValidationError::check_surface_identity(self, errors); - VertexValidationError::check_position(self, config, errors); } } @@ -37,87 +32,6 @@ impl Validate for GlobalVertex { } } -/// [`Vertex`] validation failed -#[derive(Clone, Debug, thiserror::Error)] -pub enum VertexValidationError { - /// Mismatch between the surface's of the curve and surface form - #[error( - "Surface form of vertex must be defined on same surface as curve\n\ - - `Surface` of curve: {curve_surface:#?}\n\ - - `Surface` of surface form: {surface_form_surface:#?}" - )] - SurfaceMismatch { - /// The surface of the vertex' curve - curve_surface: Handle, - - /// The surface of the vertex' surface form - surface_form_surface: Handle, - }, - - /// Mismatch between position of the vertex and position of its surface form - #[error( - "`Vertex` position doesn't match position of its surface form\n\ - - `Vertex`: {vertex:#?}\n\ - - `Vertex` position on surface: {curve_position_on_surface:?}\n\ - - Distance between the positions: {distance}" - )] - PositionMismatch { - /// The vertex - vertex: Vertex, - - /// The curve position converted into a surface position - curve_position_on_surface: Point<2>, - - /// The distance between the positions - distance: Scalar, - }, -} - -impl VertexValidationError { - fn check_surface_identity( - vertex: &Vertex, - errors: &mut Vec, - ) { - let curve_surface = vertex.curve().surface(); - let surface_form_surface = vertex.surface_form().surface(); - - if curve_surface.id() != surface_form_surface.id() { - errors.push( - Box::new(Self::SurfaceMismatch { - curve_surface: curve_surface.clone(), - surface_form_surface: surface_form_surface.clone(), - }) - .into(), - ); - } - } - - fn check_position( - vertex: &Vertex, - config: &ValidationConfig, - errors: &mut Vec, - ) { - let curve_position_on_surface = vertex - .curve() - .path() - .point_from_path_coords(vertex.position()); - let surface_position = vertex.surface_form().position(); - - let distance = curve_position_on_surface.distance_to(&surface_position); - - if distance > config.identical_max_distance { - errors.push( - Box::new(Self::PositionMismatch { - vertex: vertex.clone(), - curve_position_on_surface, - distance, - }) - .into(), - ); - } - } -} - /// [`SurfaceVertex`] validation error #[derive(Clone, Debug, thiserror::Error)] pub enum SurfaceVertexValidationError { @@ -175,89 +89,13 @@ impl SurfaceVertexValidationError { #[cfg(test)] mod tests { use crate::{ - builder::CurveBuilder, insert::Insert, - objects::{GlobalVertex, SurfaceVertex, Vertex}, - partial::{ - Partial, PartialCurve, PartialObject, PartialSurfaceVertex, - PartialVertex, - }, + objects::{GlobalVertex, SurfaceVertex}, + partial::{Partial, PartialObject, PartialSurfaceVertex}, services::Services, validate::Validate, }; - #[test] - fn vertex_surface_mismatch() -> anyhow::Result<()> { - let mut services = Services::new(); - - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut curve = PartialCurve { - surface: surface.clone(), - ..Default::default() - }; - curve.update_as_u_axis(); - - let valid = PartialVertex { - position: Some([0.].into()), - curve: Partial::from_partial(curve), - surface_form: Partial::from_partial(PartialSurfaceVertex { - surface, - ..Default::default() - }), - } - .build(&mut services.objects); - let invalid = { - let mut surface_form = Partial::from(valid.surface_form().clone()); - surface_form.write().surface = - Partial::from(services.objects.surfaces.xz_plane()); - let surface_form = surface_form.build(&mut services.objects); - - Vertex::new(valid.position(), valid.curve().clone(), surface_form) - }; - - valid.validate_and_return_first_error()?; - assert!(invalid.validate_and_return_first_error().is_err()); - - Ok(()) - } - - #[test] - fn vertex_position_mismatch() -> anyhow::Result<()> { - let mut services = Services::new(); - - let valid = { - let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut curve = PartialCurve { - surface: surface.clone(), - ..Default::default() - }; - curve.update_as_u_axis(); - - PartialVertex { - position: Some([0.].into()), - curve: Partial::from_partial(curve), - surface_form: Partial::from_partial(PartialSurfaceVertex { - surface, - ..Default::default() - }), - } - .build(&mut services.objects) - }; - let invalid = { - let mut surface_form = Partial::from(valid.surface_form().clone()); - surface_form.write().position = Some([1., 0.].into()); - surface_form.write().global_form = Partial::new(); - let surface_form = surface_form.build(&mut services.objects); - - Vertex::new(valid.position(), valid.curve().clone(), surface_form) - }; - - valid.validate_and_return_first_error()?; - assert!(invalid.validate_and_return_first_error().is_err()); - - Ok(()) - } - #[test] fn surface_vertex_position_mismatch() -> anyhow::Result<()> { let mut services = Services::new(); diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index d82698fa9..a144d2f2c 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -35,8 +35,7 @@ impl Shape for fj::Sketch { half_edge.curve().write().surface = surface.clone(); for vertex in &mut half_edge.vertices { - vertex.write().surface_form.write().surface = - surface.clone(); + vertex.surface_form.write().surface = surface.clone(); } half_edge.update_as_circle_from_radius(circle.radius());