From 1bfe7f1db9e2506a70e063d04773e47220580585 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 18 Jan 2023 15:53:35 +0100 Subject: [PATCH 1/9] Add `Handle` to `HalfEdge` This is preparation for completely moving the `Curve` reference from `Vertex` to `HalfEdge`. --- .../fj-kernel/src/algorithms/reverse/edge.rs | 7 ++- crates/fj-kernel/src/algorithms/sweep/edge.rs | 5 +- .../fj-kernel/src/algorithms/sweep/vertex.rs | 2 +- .../src/algorithms/transform/edge.rs | 6 +- crates/fj-kernel/src/objects/full/edge.rs | 8 ++- crates/fj-kernel/src/partial/objects/edge.rs | 3 +- crates/fj-kernel/src/validate/edge.rs | 59 ++++++++++++------- 7 files changed, 62 insertions(+), 28 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse/edge.rs b/crates/fj-kernel/src/algorithms/reverse/edge.rs index 8a4fcdfe6..062835590 100644 --- a/crates/fj-kernel/src/algorithms/reverse/edge.rs +++ b/crates/fj-kernel/src/algorithms/reverse/edge.rs @@ -14,6 +14,11 @@ impl Reverse for Handle { [b, a] }; - HalfEdge::new(vertices, self.global_form().clone()).insert(objects) + HalfEdge::new( + self.curve().clone(), + vertices, + self.global_form().clone(), + ) + .insert(objects) } } diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 1641c683f..6aefa8687 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -84,7 +84,8 @@ impl Sweep for (Handle, Color) { }) }; - HalfEdge::new(vertices, edge.global_form().clone()).insert(objects) + HalfEdge::new(curve, vertices, edge.global_form().clone()) + .insert(objects) }; let side_edges = bottom_edge.vertices().clone().map(|vertex| { @@ -139,7 +140,7 @@ impl Sweep for (Handle, Color) { Vertex::new(vertex.position(), curve.clone(), surface_form) }); - HalfEdge::new(vertices, global).insert(objects) + HalfEdge::new(curve, vertices, global).insert(objects) }; let cycle = { diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index dee108bfb..62e26a6c8 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -116,7 +116,7 @@ impl Sweep for (Vertex, Handle) { // And finally, creating the output `Edge` is just a matter of // assembling the pieces we've already created. - HalfEdge::new(vertices, edge_global).insert(objects) + HalfEdge::new(curve, vertices, edge_global).insert(objects) } } diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index 5c0f2b3c8..7c5829e1c 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -14,6 +14,10 @@ impl TransformObject for HalfEdge { objects: &mut Service, cache: &mut TransformCache, ) -> Self { + let curve = self + .curve() + .clone() + .transform_with_cache(transform, objects, cache); let vertices = self.vertices().clone().map(|vertex| { vertex.transform_with_cache(transform, objects, cache) }); @@ -22,7 +26,7 @@ impl TransformObject for HalfEdge { .clone() .transform_with_cache(transform, objects, cache); - Self::new(vertices, global_form) + Self::new(curve, vertices, global_form) } } diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 782d72bd6..8bce39bce 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -8,14 +8,20 @@ use crate::{ /// A half-edge #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdge { + curve: Handle, vertices: [Vertex; 2], global_form: Handle, } impl HalfEdge { /// Create an instance of `HalfEdge` - pub fn new(vertices: [Vertex; 2], global_form: Handle) -> Self { + pub fn new( + curve: Handle, + vertices: [Vertex; 2], + global_form: Handle, + ) -> Self { Self { + curve, vertices, global_form, } diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 5b8b0ef14..23ae8f00e 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -72,10 +72,11 @@ impl PartialObject for PartialHalfEdge { } fn build(self, objects: &mut Service) -> Self::Full { + let curve = self.curve().build(objects); let vertices = self.vertices.map(|vertex| vertex.build(objects)); let global_form = self.global_form.build(objects); - HalfEdge::new(vertices, global_form) + HalfEdge::new(curve, vertices, global_form) } } diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 1997c14d7..b08ddc717 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -346,7 +346,11 @@ mod tests { ); vertices[1] = vertex.build(&mut services.objects); - HalfEdge::new(vertices, valid.global_form().clone()) + HalfEdge::new( + valid.curve().clone(), + vertices, + valid.global_form().clone(), + ) }; valid.validate_and_return_first_error()?; @@ -368,12 +372,13 @@ mod tests { half_edge.build(&mut services.objects) }; - let invalid = HalfEdge::new(valid.vertices().clone(), { - let mut tmp = Partial::from(valid.global_form().clone()); - tmp.write().curve = - Partial::from(GlobalCurve.insert(&mut services.objects)); - tmp.build(&mut services.objects) - }); + let invalid = + HalfEdge::new(valid.curve().clone(), valid.vertices().clone(), { + let mut tmp = Partial::from(valid.global_form().clone()); + tmp.write().curve = + Partial::from(GlobalCurve.insert(&mut services.objects)); + tmp.build(&mut services.objects) + }); valid.validate_and_return_first_error()?; assert!(invalid.validate_and_return_first_error().is_err()); @@ -394,18 +399,21 @@ mod tests { half_edge.build(&mut services.objects) }; - let invalid = HalfEdge::new(valid.vertices().clone(), { - let mut tmp = Partial::from(valid.global_form().clone()); - tmp.write().vertices = valid - .global_form() - .vertices() - .access_in_normalized_order() - // Creating equal but not identical vertices here. - .map(|vertex| { - Partial::from_partial(Partial::from(vertex).read().clone()) - }); - tmp.build(&mut services.objects) - }); + let invalid = + HalfEdge::new(valid.curve().clone(), valid.vertices().clone(), { + let mut tmp = Partial::from(valid.global_form().clone()); + tmp.write().vertices = valid + .global_form() + .vertices() + .access_in_normalized_order() + // Creating equal but not identical vertices here. + .map(|vertex| { + Partial::from_partial( + Partial::from(vertex).read().clone(), + ) + }); + tmp.build(&mut services.objects) + }); valid.validate_and_return_first_error()?; assert!(invalid.validate_and_return_first_error().is_err()); @@ -438,7 +446,11 @@ mod tests { vertex.build(&mut services.objects) }); - HalfEdge::new(vertices, valid.global_form().clone()) + HalfEdge::new( + valid.curve().clone(), + vertices, + valid.global_form().clone(), + ) }; valid.validate_and_return_first_error()?; @@ -461,6 +473,7 @@ mod tests { half_edge.build(&mut services.objects) }; let invalid = HalfEdge::new( + valid.curve().clone(), valid.vertices().clone().map(|vertex| { let vertex = PartialVertex::from_full( &vertex, @@ -510,7 +523,11 @@ mod tests { vertex.build(&mut services.objects) }); - HalfEdge::new(vertices, valid.global_form().clone()) + HalfEdge::new( + valid.curve().clone(), + vertices, + valid.global_form().clone(), + ) }; valid.validate_and_return_first_error()?; From 1e670054e673b8adc2bc541c75f69f61d4869bed Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 18 Jan 2023 15:56:13 +0100 Subject: [PATCH 2/9] Refactor --- crates/fj-kernel/src/objects/full/edge.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 8bce39bce..93a82f63c 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -29,8 +29,7 @@ impl HalfEdge { /// Access the curve that defines the half-edge's geometry pub fn curve(&self) -> &Handle { - let [vertex, _] = self.vertices(); - vertex.curve() + &self.curve } /// Access the vertices that bound the half-edge on the curve From c425d23ef74011914431414b743147447243e7da Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 18 Jan 2023 15:59:10 +0100 Subject: [PATCH 3/9] Add `Partial` to `PartialHalfEdge` This follows the corresponding change in `HalfEdge`. --- crates/fj-kernel/src/partial/objects/edge.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 23ae8f00e..fd61c524a 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -13,6 +13,9 @@ use crate::{ /// A partial [`HalfEdge`] #[derive(Clone, Debug)] pub struct PartialHalfEdge { + /// The curve that the half-edge is defined in + pub curve: Partial, + /// The vertices that bound the half-edge on the curve pub vertices: [PartialVertex; 2], @@ -60,6 +63,7 @@ impl PartialObject for PartialHalfEdge { cache: &mut FullToPartialCache, ) -> Self { Self { + curve: Partial::from_full(half_edge.curve().clone(), cache), vertices: half_edge .vertices() .clone() @@ -102,6 +106,7 @@ impl Default for PartialHalfEdge { }); Self { + curve, vertices, global_form, } From 0effbdf5431a574fe3be0797a3f2ace6c9472e4d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 18 Jan 2023 16:05:22 +0100 Subject: [PATCH 4/9] Remove redundant method --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 6 +++--- crates/fj-kernel/src/builder/edge.rs | 20 +++++++++---------- crates/fj-kernel/src/builder/face.rs | 2 +- crates/fj-kernel/src/partial/objects/edge.rs | 8 +------- crates/fj-operations/src/sketch.rs | 2 +- 5 files changed, 15 insertions(+), 23 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 6aefa8687..d86800b40 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -230,7 +230,7 @@ mod tests { }; let side_up = { let mut side_up = PartialHalfEdge::default(); - side_up.curve().write().surface = surface.clone(); + side_up.curve.write().surface = surface.clone(); { let [back, front] = &mut side_up.vertices; @@ -249,7 +249,7 @@ mod tests { }; let top = { let mut top = PartialHalfEdge::default(); - top.curve().write().surface = surface.clone(); + top.curve.write().surface = surface.clone(); { let [back, front] = &mut top.vertices; @@ -275,7 +275,7 @@ mod tests { }; let side_down = { let mut side_down = PartialHalfEdge::default(); - side_down.curve().write().surface = surface; + side_down.curve.write().surface = surface; let [back, front] = &mut side_down.vertices; diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index c2ed2c1ef..a6dcaa0d2 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -64,8 +64,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { } fn update_as_circle_from_radius(&mut self, radius: impl Into) { - let mut curve = self.curve(); - let path = curve.write().update_as_circle_from_radius(radius); + let path = self.curve.write().update_as_circle_from_radius(radius); let [a_curve, b_curve] = [Scalar::ZERO, Scalar::TAU].map(|coord| Point::from([coord])); @@ -107,8 +106,8 @@ impl HalfEdgeBuilder for PartialHalfEdge { angle_rad, ); - let mut curve = self.curve(); - let path = curve + let path = self + .curve .write() .update_as_circle_from_center_and_radius(arc.center, arc.radius); @@ -157,7 +156,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { .expect("Can't infer line segment without surface position") }); - self.curve() + self.curve .write() .update_as_line_from_points(points_surface); @@ -170,8 +169,7 @@ 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().curve = self.curve.read().global_form.clone(); self.global_form.write().vertices = self .vertices .each_ref_ext() @@ -181,13 +179,13 @@ impl HalfEdgeBuilder for PartialHalfEdge { } fn update_from_other_edge(&mut self, other: &Partial) { - let global_curve = other.read().curve().read().global_form.clone(); - self.curve().write().global_form = global_curve.clone(); + let global_curve = other.read().curve.read().global_form.clone(); + self.curve.write().global_form = global_curve.clone(); self.global_form.write().curve = global_curve; - self.curve().write().path = other + self.curve.write().path = other .read() - .curve() + .curve .read() .path .as_ref() diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 358ffd625..e89149c03 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -151,7 +151,7 @@ impl FaceBuilder for PartialFace { for half_edge in &mut self.exterior.write().half_edges { let mut half_edge = half_edge.write(); - let mut curve = half_edge.curve(); + let mut curve = half_edge.curve.clone(); let mut curve = curve.write(); if let Some(path) = &mut curve.path { diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index fd61c524a..40436018d 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -24,12 +24,6 @@ pub struct PartialHalfEdge { } impl PartialHalfEdge { - /// Access the curve the partial edge is defined on - pub fn curve(&self) -> Partial { - let [vertex, _] = &self.vertices; - vertex.curve.clone() - } - /// Access a reference to the half-edge's back vertex pub fn back(&self) -> &PartialVertex { let [back, _] = &self.vertices; @@ -76,7 +70,7 @@ impl PartialObject for PartialHalfEdge { } fn build(self, objects: &mut Service) -> Self::Full { - let curve = self.curve().build(objects); + let curve = self.curve.build(objects); let vertices = self.vertices.map(|vertex| vertex.build(objects)); let global_form = self.global_form.build(objects); diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index a144d2f2c..00243f3aa 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -32,7 +32,7 @@ impl Shape for fj::Sketch { let mut half_edge = PartialHalfEdge::default(); - half_edge.curve().write().surface = surface.clone(); + half_edge.curve.write().surface = surface.clone(); for vertex in &mut half_edge.vertices { vertex.surface_form.write().surface = surface.clone(); From 7f42c6c577723e2b2afe8186be1a65f16d80602c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 18 Jan 2023 16:32:32 +0100 Subject: [PATCH 5/9] Simplify test --- crates/fj-kernel/src/validate/edge.rs | 36 +++++++++++++-------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index b08ddc717..939dc43f9 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -307,6 +307,7 @@ impl HalfEdgeValidationError { #[cfg(test)] mod tests { + use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ @@ -315,7 +316,7 @@ mod tests { objects::{GlobalCurve, HalfEdge}, partial::{ FullToPartialCache, Partial, PartialHalfEdge, PartialObject, - PartialSurfaceVertex, PartialVertex, + PartialVertex, }, services::Services, validate::Validate, @@ -472,26 +473,23 @@ mod tests { half_edge.build(&mut services.objects) }; - let invalid = HalfEdge::new( - valid.curve().clone(), - valid.vertices().clone().map(|vertex| { - let vertex = PartialVertex::from_full( - &vertex, + let invalid = { + let vertices = valid.vertices().each_ref_ext().map(|vertex| { + let mut vertex = PartialVertex::from_full( + vertex, &mut FullToPartialCache::default(), ); - let surface = vertex.surface_form.read().surface.clone(); - PartialVertex { - position: Some([0.].into()), - surface_form: Partial::from_partial(PartialSurfaceVertex { - surface, - ..Default::default() - }), - ..vertex - } - .build(&mut services.objects) - }), - valid.global_form().clone(), - ); + vertex.position = Some(Point::from([0.])); + + vertex.build(&mut services.objects) + }); + + HalfEdge::new( + valid.curve().clone(), + vertices, + valid.global_form().clone(), + ) + }; valid.validate_and_return_first_error()?; assert!(invalid.validate_and_return_first_error().is_err()); From 7195fb9f606bb22cf641dd69fda73d2b5caacf1c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 18 Jan 2023 15:53:35 +0100 Subject: [PATCH 6/9] Infer vertex positions in `HalfEdge` build This is preparation for completely moving the `Curve` reference from `Vertex` to `HalfEdge`. --- .../fj-kernel/src/algorithms/sweep/vertex.rs | 10 ++++-- crates/fj-kernel/src/partial/objects/edge.rs | 35 ++++++++++++++++++- .../fj-kernel/src/partial/objects/vertex.rs | 16 ++------- crates/fj-kernel/src/validate/vertex.rs | 10 ++++-- 4 files changed, 51 insertions(+), 20 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index 62e26a6c8..0214ad3da 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -153,6 +153,7 @@ impl Sweep for Handle { #[cfg(test)] mod tests { + use fj_math::Point; use pretty_assertions::assert_eq; use crate::{ @@ -160,8 +161,8 @@ mod tests { builder::{CurveBuilder, HalfEdgeBuilder}, insert::Insert, partial::{ - Partial, PartialCurve, PartialHalfEdge, PartialObject, - PartialSurfaceVertex, PartialVertex, + Partial, PartialCurve, PartialGlobalVertex, PartialHalfEdge, + PartialObject, PartialSurfaceVertex, PartialVertex, }, services::Services, }; @@ -183,8 +184,11 @@ mod tests { position: Some([0.].into()), curve: Partial::from(curve), surface_form: Partial::from_partial(PartialSurfaceVertex { + position: Some(Point::from([0., 0.])), surface: Partial::from(surface.clone()), - ..Default::default() + global_form: Partial::from_partial(PartialGlobalVertex { + position: Some(Point::from([0., 0., 0.])), + }), }), } .build(&mut services.objects); diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 40436018d..acfe2335a 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -71,7 +71,40 @@ impl PartialObject for PartialHalfEdge { fn build(self, objects: &mut Service) -> Self::Full { let curve = self.curve.build(objects); - let vertices = self.vertices.map(|vertex| vertex.build(objects)); + let vertices = self.vertices.map(|mut vertex| { + let position_surface = vertex.surface_form.read().position; + + // Infer surface position, if not available. + let position_surface = match position_surface { + Some(position_surface) => position_surface, + None => { + let position_curve = vertex.position.expect( + "Can't infer surface position without curve position", + ); + let position_surface = + curve.path().point_from_path_coords(position_curve); + + vertex.surface_form.write().position = + Some(position_surface); + + position_surface + } + }; + + // Infer global position, if not available. + let position_global = + vertex.surface_form.read().global_form.read().position; + if position_global.is_none() { + let surface = curve.surface().geometry(); + + let position_global = + surface.point_from_surface_coords(position_surface); + vertex.surface_form.write().global_form.write().position = + Some(position_global); + } + + vertex.build(objects) + }); let global_form = self.global_form.build(objects); HalfEdge::new(curve, vertices, global_form) diff --git a/crates/fj-kernel/src/partial/objects/vertex.rs b/crates/fj-kernel/src/partial/objects/vertex.rs index 171f7aadc..3c46b4878 100644 --- a/crates/fj-kernel/src/partial/objects/vertex.rs +++ b/crates/fj-kernel/src/partial/objects/vertex.rs @@ -1,7 +1,6 @@ use fj_math::Point; use crate::{ - builder::SurfaceVertexBuilder, objects::{Curve, GlobalVertex, Objects, Surface, SurfaceVertex, Vertex}, partial::{FullToPartialCache, Partial, PartialCurve, PartialObject}, services::Service, @@ -34,18 +33,11 @@ impl PartialObject for PartialVertex { } } - fn build(mut self, objects: &mut Service) -> Self::Full { + fn build(self, objects: &mut Service) -> Self::Full { let position = self .position .expect("Can't build `Vertex` without position"); let curve = self.curve.build(objects); - - // Infer surface position, if not available. - if self.surface_form.read().position.is_none() { - self.surface_form.write().position = - Some(curve.path().point_from_path_coords(position)); - } - let surface_form = self.surface_form.build(objects); Vertex::new(position, curve, surface_form) @@ -106,11 +98,7 @@ impl PartialObject for PartialSurfaceVertex { } } - fn build(mut self, objects: &mut Service) -> Self::Full { - if self.global_form.read().position.is_none() { - self.infer_global_position(); - } - + fn build(self, objects: &mut Service) -> Self::Full { let position = self .position .expect("Can't build `SurfaceVertex` without position"); diff --git a/crates/fj-kernel/src/validate/vertex.rs b/crates/fj-kernel/src/validate/vertex.rs index 7dbd1f049..bb75f5e98 100644 --- a/crates/fj-kernel/src/validate/vertex.rs +++ b/crates/fj-kernel/src/validate/vertex.rs @@ -88,10 +88,14 @@ impl SurfaceVertexValidationError { #[cfg(test)] mod tests { + use fj_math::Point; + use crate::{ insert::Insert, objects::{GlobalVertex, SurfaceVertex}, - partial::{Partial, PartialObject, PartialSurfaceVertex}, + partial::{ + Partial, PartialGlobalVertex, PartialObject, PartialSurfaceVertex, + }, services::Services, validate::Validate, }; @@ -103,7 +107,9 @@ mod tests { let valid = PartialSurfaceVertex { position: Some([0., 0.].into()), surface: Partial::from(services.objects.surfaces.xy_plane()), - ..Default::default() + global_form: Partial::from_partial(PartialGlobalVertex { + position: Some(Point::from([0., 0., 0.])), + }), } .build(&mut services.objects); let invalid = SurfaceVertex::new( From a097370fab561d1008620dc46a4021f260ffad11 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 18 Jan 2023 16:40:50 +0100 Subject: [PATCH 7/9] Avoid use of `Vertex::curve` This is preparation for completely moving the `Curve` reference from `Vertex` to `HalfEdge`. --- 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 939dc43f9..cf358676b 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -282,7 +282,7 @@ impl HalfEdgeValidationError { errors: &mut Vec, ) { for vertex in half_edge.vertices() { - let curve_position_on_surface = vertex + let curve_position_on_surface = half_edge .curve() .path() .point_from_path_coords(vertex.position()); From 2521e467c613a21df428dc77e015496f5b6a1876 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 18 Jan 2023 16:00:51 +0100 Subject: [PATCH 8/9] Write to `PartialHalfEdge`'s curve in builder --- crates/fj-kernel/src/builder/edge.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index a6dcaa0d2..cd794fea9 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -58,6 +58,8 @@ impl HalfEdgeBuilder for PartialHalfEdge { fn replace_surface(&mut self, surface: impl Into>) { let surface = surface.into(); + self.curve.write().surface = surface.clone(); + for vertex in &mut self.vertices { vertex.replace_surface(surface.clone()); } @@ -136,6 +138,8 @@ impl HalfEdgeBuilder for PartialHalfEdge { ) { let surface = surface.into(); + self.curve.write().surface = surface.clone(); + for (vertex, point) in self.vertices.each_mut_ext().zip_ext(points) { vertex.curve.write().surface = surface.clone(); From a06b0d5c17097969fb41c9bf1289159bc82a8392 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 18 Jan 2023 16:45:35 +0100 Subject: [PATCH 9/9] Remove ref. to curve from `Vertex`/`PartialVertex` This simplifies the object graph, making it less redundant, even making it possible to remove a now superfluous validation check. --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 8 +- .../fj-kernel/src/algorithms/sweep/vertex.rs | 10 +-- .../src/algorithms/transform/vertex.rs | 6 +- crates/fj-kernel/src/builder/edge.rs | 2 - crates/fj-kernel/src/builder/vertex.rs | 2 - crates/fj-kernel/src/objects/full/vertex.rs | 13 +-- crates/fj-kernel/src/partial/objects/edge.rs | 5 +- .../fj-kernel/src/partial/objects/vertex.rs | 16 +--- crates/fj-kernel/src/validate/edge.rs | 82 +------------------ 9 files changed, 12 insertions(+), 132 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index d86800b40..4f884a24e 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -76,11 +76,7 @@ impl Sweep for (Handle, Color) { ) .insert(objects); - Vertex::new( - vertex.position(), - curve.clone(), - surface_vertex, - ) + Vertex::new(vertex.position(), surface_vertex) }) }; @@ -137,7 +133,7 @@ impl Sweep for (Handle, Color) { .zip(surface_vertices) .collect::<[_; 2]>() .map(|(vertex, surface_form)| { - Vertex::new(vertex.position(), curve.clone(), surface_form) + Vertex::new(vertex.position(), surface_form) }); HalfEdge::new(curve, vertices, global).insert(objects) diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index 0214ad3da..f17a56aec 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -107,11 +107,7 @@ impl Sweep for (Vertex, Handle) { // And now the vertices. Again, nothing wild here. let vertices = vertices_surface.map(|surface_form| { - Vertex::new( - [surface_form.position().v], - curve.clone(), - surface_form, - ) + Vertex::new([surface_form.position().v], surface_form) }); // And finally, creating the output `Edge` is just a matter of @@ -177,12 +173,8 @@ mod tests { ..Default::default() }; curve.update_as_u_axis(); - let curve = curve - .build(&mut services.objects) - .insert(&mut services.objects); let vertex = PartialVertex { position: Some([0.].into()), - curve: Partial::from(curve), surface_form: Partial::from_partial(PartialSurfaceVertex { position: Some(Point::from([0., 0.])), surface: Partial::from(surface.clone()), diff --git a/crates/fj-kernel/src/algorithms/transform/vertex.rs b/crates/fj-kernel/src/algorithms/transform/vertex.rs index 983c53d1f..7b5adf724 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 Vertex { // coordinates and thus transforming the curve takes care of it. let position = self.position(); - let curve = self - .curve() - .clone() - .transform_with_cache(transform, objects, cache); let surface_form = self .surface_form() .clone() .transform_with_cache(transform, objects, cache); - Self::new(position, curve, surface_form) + Self::new(position, surface_form) } } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index cd794fea9..31ba4642f 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -141,8 +141,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { self.curve.write().surface = surface.clone(); for (vertex, point) in self.vertices.each_mut_ext().zip_ext(points) { - vertex.curve.write().surface = surface.clone(); - let mut surface_form = vertex.surface_form.write(); surface_form.position = Some(point.into()); surface_form.surface = surface.clone(); diff --git a/crates/fj-kernel/src/builder/vertex.rs b/crates/fj-kernel/src/builder/vertex.rs index 64f175138..2b3dda16a 100644 --- a/crates/fj-kernel/src/builder/vertex.rs +++ b/crates/fj-kernel/src/builder/vertex.rs @@ -23,8 +23,6 @@ pub trait VertexBuilder { impl VertexBuilder for PartialVertex { fn replace_surface(&mut self, surface: impl Into>) { let surface = surface.into(); - - self.curve.write().surface = surface.clone(); self.surface_form.write().surface = surface; } } diff --git a/crates/fj-kernel/src/objects/full/vertex.rs b/crates/fj-kernel/src/objects/full/vertex.rs index c0bc62293..b969135b7 100644 --- a/crates/fj-kernel/src/objects/full/vertex.rs +++ b/crates/fj-kernel/src/objects/full/vertex.rs @@ -1,9 +1,6 @@ use fj_math::Point; -use crate::{ - objects::{Curve, Surface}, - storage::Handle, -}; +use crate::{objects::Surface, storage::Handle}; /// A vertex /// @@ -15,7 +12,6 @@ use crate::{ #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Vertex { position: Point<1>, - curve: Handle, surface_form: Handle, } @@ -23,14 +19,12 @@ impl Vertex { /// Construct an instance of `Vertex` pub fn new( position: impl Into>, - curve: Handle, surface_form: Handle, ) -> Self { let position = position.into(); Self { position, - curve, surface_form, } } @@ -40,11 +34,6 @@ impl Vertex { self.position } - /// Access the curve that the vertex is defined in - pub fn curve(&self) -> &Handle { - &self.curve - } - /// Access the surface form of this vertex pub fn surface_form(&self) -> &Handle { &self.surface_form diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index acfe2335a..4e251124a 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -114,10 +114,7 @@ impl PartialObject for PartialHalfEdge { impl Default for PartialHalfEdge { fn default() -> Self { let curve = Partial::::new(); - let vertices = array::from_fn(|_| PartialVertex { - curve: curve.clone(), - ..Default::default() - }); + let vertices = array::from_fn(|_| PartialVertex::default()); let global_curve = curve.read().global_form.clone(); let global_vertices = diff --git a/crates/fj-kernel/src/partial/objects/vertex.rs b/crates/fj-kernel/src/partial/objects/vertex.rs index 3c46b4878..9f117cccf 100644 --- a/crates/fj-kernel/src/partial/objects/vertex.rs +++ b/crates/fj-kernel/src/partial/objects/vertex.rs @@ -1,8 +1,8 @@ use fj_math::Point; use crate::{ - objects::{Curve, GlobalVertex, Objects, Surface, SurfaceVertex, Vertex}, - partial::{FullToPartialCache, Partial, PartialCurve, PartialObject}, + objects::{GlobalVertex, Objects, Surface, SurfaceVertex, Vertex}, + partial::{FullToPartialCache, Partial, PartialObject}, services::Service, }; @@ -12,9 +12,6 @@ pub struct PartialVertex { /// The position of the vertex on the curve pub position: Option>, - /// The curve that the vertex is defined in - pub curve: Partial, - /// The surface form of the vertex pub surface_form: Partial, } @@ -25,7 +22,6 @@ impl PartialObject for PartialVertex { fn from_full(vertex: &Self::Full, cache: &mut FullToPartialCache) -> Self { Self { position: Some(vertex.position()), - curve: Partial::from_full(vertex.curve().clone(), cache), surface_form: Partial::from_full( vertex.surface_form().clone(), cache, @@ -37,10 +33,9 @@ impl PartialObject for PartialVertex { let position = self .position .expect("Can't build `Vertex` without position"); - let curve = self.curve.build(objects); let surface_form = self.surface_form.build(objects); - Vertex::new(position, curve, surface_form) + Vertex::new(position, surface_form) } } @@ -48,10 +43,6 @@ impl Default for PartialVertex { fn default() -> Self { let surface = Partial::new(); - let curve = Partial::from_partial(PartialCurve { - surface: surface.clone(), - ..Default::default() - }); let surface_form = Partial::from_partial(PartialSurfaceVertex { surface, ..Default::default() @@ -59,7 +50,6 @@ impl Default for PartialVertex { Self { position: None, - curve, surface_form, } } diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index cf358676b..09a711d04 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, Surface, - Vertex, VerticesInNormalizedOrder, + GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface, Vertex, + VerticesInNormalizedOrder, }, storage::Handle, }; @@ -17,7 +17,6 @@ impl Validate for HalfEdge { config: &ValidationConfig, errors: &mut Vec, ) { - 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); @@ -42,24 +41,6 @@ impl Validate for GlobalEdge { /// [`HalfEdge`] validation failed #[derive(Clone, Debug, thiserror::Error)] pub enum HalfEdgeValidationError { - /// [`HalfEdge`] vertices are not defined on the same `Curve` - #[error( - "`HalfEdge` vertices are not defined on the same `Curve`\n\ - - `Curve` of back vertex: {back_curve:#?}\n\ - - `Curve` of front vertex: {front_curve:#?}\n\ - - `HalfEdge`: {half_edge:#?}" - )] - CurveMismatch { - /// The curve of the [`HalfEdge`]'s back vertex - back_curve: Handle, - - /// The curve of the [`HalfEdge`]'s front vertex - front_curve: Handle, - - /// The half-edge - half_edge: HalfEdge, - }, - /// [`HalfEdge`]'s [`GlobalCurve`]s do not match #[error( "Global form of `HalfEdge`'s `Curve` does not match `GlobalCurve` of \n\ @@ -69,7 +50,7 @@ pub enum HalfEdgeValidationError { - `HalfEdge`: {half_edge:#?}", )] GlobalCurveMismatch { - /// The [`GlobalCurve`] from the [`HalfEdge`]'s [`Curve`] + /// The [`GlobalCurve`] from the [`HalfEdge`]'s `Curve` global_curve_from_curve: Handle, /// The [`GlobalCurve`] from the [`HalfEdge`]'s global form @@ -155,25 +136,6 @@ pub enum HalfEdgeValidationError { } impl HalfEdgeValidationError { - fn check_curve_identity( - half_edge: &HalfEdge, - errors: &mut Vec, - ) { - let back_curve = half_edge.back().curve(); - let front_curve = half_edge.front().curve(); - - if back_curve.id() != front_curve.id() { - errors.push( - Box::new(Self::CurveMismatch { - back_curve: back_curve.clone(), - front_curve: front_curve.clone(), - half_edge: half_edge.clone(), - }) - .into(), - ); - } - } - fn check_global_curve_identity( half_edge: &HalfEdge, errors: &mut Vec, @@ -322,44 +284,6 @@ mod tests { validate::Validate, }; - #[test] - fn half_edge_curve_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 vertices = valid.vertices().clone(); - let mut vertex = PartialVertex::from_full( - &vertices[1], - &mut FullToPartialCache::default(), - ); - // Arranging for an equal but not identical curve here. - vertex.curve = Partial::from_partial( - Partial::from(valid.curve().clone()).read().clone(), - ); - vertices[1] = vertex.build(&mut services.objects); - - HalfEdge::new( - 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_global_curve_mismatch() -> anyhow::Result<()> { let mut services = Services::new();