From 008a67887a4b3bcbda91e9d64c0f69d8c78d4887 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 12:10:03 +0200 Subject: [PATCH 01/11] Don't move vertices unnecessarily --- crates/fj-kernel/src/partial/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/partial/edge.rs b/crates/fj-kernel/src/partial/edge.rs index 55639ef52..280adf145 100644 --- a/crates/fj-kernel/src/partial/edge.rs +++ b/crates/fj-kernel/src/partial/edge.rs @@ -124,7 +124,7 @@ impl PartialHalfEdge { .copied() .or_else(|| to.surface().copied()) .expect("Can't infer line segment without a surface"); - let points = [from, to].map(|vertex| { + let points = [&from, &to].map(|vertex| { vertex .position() .expect("Can't infer line segment without surface position") From 82e4da9dfb2a9f2efef794d124059c3db02282b3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 12:11:27 +0200 Subject: [PATCH 02/11] Make variable names more specific --- crates/fj-kernel/src/partial/edge.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/partial/edge.rs b/crates/fj-kernel/src/partial/edge.rs index 280adf145..15d90171b 100644 --- a/crates/fj-kernel/src/partial/edge.rs +++ b/crates/fj-kernel/src/partial/edge.rs @@ -109,7 +109,7 @@ impl PartialHalfEdge { /// Update partial half-edge as a line segment, reusing existing vertices pub fn as_line_segment(mut self) -> Self { - let [from, to] = self + let [from_surface, to_surface] = self .vertices .clone() .expect("Can't infer line segment without vertices") @@ -119,12 +119,12 @@ impl PartialHalfEdge { ) }); - let surface = from + let surface = from_surface .surface() .copied() - .or_else(|| to.surface().copied()) + .or_else(|| to_surface.surface().copied()) .expect("Can't infer line segment without a surface"); - let points = [&from, &to].map(|vertex| { + let points = [&from_surface, &to_surface].map(|vertex| { vertex .position() .expect("Can't infer line segment without surface position") From f094d3d1f177b4490b9ecd9acfec7c2dd4d26222 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 12:12:23 +0200 Subject: [PATCH 03/11] Refactor --- crates/fj-kernel/src/partial/edge.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/partial/edge.rs b/crates/fj-kernel/src/partial/edge.rs index 15d90171b..ccf8c0be8 100644 --- a/crates/fj-kernel/src/partial/edge.rs +++ b/crates/fj-kernel/src/partial/edge.rs @@ -109,15 +109,15 @@ impl PartialHalfEdge { /// Update partial half-edge as a line segment, reusing existing vertices pub fn as_line_segment(mut self) -> Self { - let [from_surface, to_surface] = self + let [from, to] = self .vertices .clone() - .expect("Can't infer line segment without vertices") - .map(|vertex| { - vertex.surface_form().expect( - "Can't infer line segment without two surface vertices", - ) - }); + .expect("Can't infer line segment without vertices"); + let [from_surface, to_surface] = [&from, &to].map(|vertex| { + vertex + .surface_form() + .expect("Can't infer line segment without two surface vertices") + }); let surface = from_surface .surface() From 19bf0fe724d0511ca424b0ae8faac0c37773fb56 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 12:17:38 +0200 Subject: [PATCH 04/11] Store inferred curve in vertices, if necessary --- crates/fj-kernel/src/partial/edge.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/fj-kernel/src/partial/edge.rs b/crates/fj-kernel/src/partial/edge.rs index ccf8c0be8..6639c8c29 100644 --- a/crates/fj-kernel/src/partial/edge.rs +++ b/crates/fj-kernel/src/partial/edge.rs @@ -137,7 +137,16 @@ impl PartialHalfEdge { .with_surface(surface) .as_line_from_points(points); + let vertices = [from, to].map(|vertex| match vertex { + MaybePartial::Partial(vertex) => { + let vertex = vertex.with_curve(curve.clone()); + MaybePartial::from(vertex) + } + _ => vertex, + }); + self.curve = Some(curve.into()); + self.vertices = Some(vertices); self } From 744864efa1201bbc839a9fee8f7e482abb9d767c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 13:50:15 +0200 Subject: [PATCH 05/11] Add `MaybePartial::global_form` --- crates/fj-kernel/src/partial/maybe_partial.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/partial/maybe_partial.rs b/crates/fj-kernel/src/partial/maybe_partial.rs index 277ef9902..01f2e6af6 100644 --- a/crates/fj-kernel/src/partial/maybe_partial.rs +++ b/crates/fj-kernel/src/partial/maybe_partial.rs @@ -1,7 +1,7 @@ use fj_math::Point; use crate::{ - objects::{GlobalCurve, GlobalEdge, Surface, SurfaceVertex, Vertex}, + objects::{Curve, GlobalCurve, GlobalEdge, Surface, SurfaceVertex, Vertex}, stores::{Handle, Stores}, }; @@ -35,6 +35,16 @@ impl MaybePartial { } } +impl MaybePartial { + /// Access the global form + pub fn global_form(&self) -> Option>> { + match self { + Self::Full(full) => Some(full.global_form().clone().into()), + Self::Partial(partial) => partial.global_form.clone(), + } + } +} + impl MaybePartial { /// Access the curve pub fn curve(&self) -> Option<&Handle> { From 121dc0dd97164ddbc22ea3659996a041278a01b2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 13:50:37 +0200 Subject: [PATCH 06/11] Use global form of curve, if available --- crates/fj-kernel/src/partial/edge.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/partial/edge.rs b/crates/fj-kernel/src/partial/edge.rs index 6639c8c29..e84ef80e5 100644 --- a/crates/fj-kernel/src/partial/edge.rs +++ b/crates/fj-kernel/src/partial/edge.rs @@ -177,8 +177,20 @@ impl PartialHalfEdge { fn extract_global_curve( &self, ) -> Option>> { - let global_curve = self.global_form.as_ref()?.curve()?.clone(); - Some(global_curve.into()) + fn extract_global_curve_from_curve( + partial: &PartialHalfEdge, + ) -> Option>> { + partial.curve.as_ref()?.global_form() + } + + fn extract_global_curve_from_global_form( + partial: &PartialHalfEdge, + ) -> Option>> { + Some(partial.global_form.as_ref()?.curve()?.clone().into()) + } + + extract_global_curve_from_curve(self) + .or_else(|| extract_global_curve_from_global_form(self)) } } From 86e59b656b01a311bb15ac8c8c23ec994a938886 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 15:11:40 +0200 Subject: [PATCH 07/11] Infer vertex position in `as_line_segment` --- crates/fj-kernel/src/partial/edge.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/partial/edge.rs b/crates/fj-kernel/src/partial/edge.rs index e84ef80e5..db601289b 100644 --- a/crates/fj-kernel/src/partial/edge.rs +++ b/crates/fj-kernel/src/partial/edge.rs @@ -137,13 +137,16 @@ impl PartialHalfEdge { .with_surface(surface) .as_line_from_points(points); - let vertices = [from, to].map(|vertex| match vertex { - MaybePartial::Partial(vertex) => { - let vertex = vertex.with_curve(curve.clone()); - MaybePartial::from(vertex) - } - _ => vertex, - }); + let vertices = + [(from, 0.), (to, 1.)].map(|(vertex, position)| match vertex { + MaybePartial::Partial(vertex) => { + let vertex = vertex + .with_position([position]) + .with_curve(curve.clone()); + MaybePartial::from(vertex) + } + _ => vertex, + }); self.curve = Some(curve.into()); self.vertices = Some(vertices); From 4e70462bf5b0a8a5addd1a8b0be0da7ea722d942 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 15:43:37 +0200 Subject: [PATCH 08/11] Simplify function implementation --- crates/fj-kernel/src/partial/edge.rs | 30 ++++++++++------------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/crates/fj-kernel/src/partial/edge.rs b/crates/fj-kernel/src/partial/edge.rs index db601289b..deb15c13c 100644 --- a/crates/fj-kernel/src/partial/edge.rs +++ b/crates/fj-kernel/src/partial/edge.rs @@ -2,7 +2,8 @@ use fj_math::{Point, Scalar}; use crate::{ objects::{ - Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface, Vertex, + Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface, + SurfaceVertex, Vertex, }, stores::{Handle, Stores}, }; @@ -84,27 +85,18 @@ impl PartialHalfEdge { /// Update partial half-edge as a line segment, from the given points pub fn as_line_segment_from_points( - mut self, + self, surface: Surface, points: [impl Into>; 2], ) -> Self { - let curve = PartialCurve { - global_form: self.extract_global_curve(), - ..PartialCurve::default() - } - .with_surface(surface) - .as_line_from_points(points); - - let vertices = [0., 1.].map(|position| { - Vertex::partial() - .with_position([position]) - .with_curve(curve.clone()) - }); - - self.curve = Some(curve.into()); - self.vertices = Some(vertices.map(Into::into)); - - self + self.with_vertices(points.map(|point| { + Vertex::partial().with_surface_form( + SurfaceVertex::partial() + .with_surface(surface) + .with_position(point), + ) + })) + .as_line_segment() } /// Update partial half-edge as a line segment, reusing existing vertices From e51eb5125fdf16e824f0907e56524aa926530cf5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 15:46:28 +0200 Subject: [PATCH 09/11] Inline function to its only use site --- crates/fj-kernel/src/partial/edge.rs | 40 ++++++++++++++-------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/crates/fj-kernel/src/partial/edge.rs b/crates/fj-kernel/src/partial/edge.rs index deb15c13c..2e6a4116a 100644 --- a/crates/fj-kernel/src/partial/edge.rs +++ b/crates/fj-kernel/src/partial/edge.rs @@ -101,6 +101,25 @@ impl PartialHalfEdge { /// Update partial half-edge as a line segment, reusing existing vertices pub fn as_line_segment(mut self) -> Self { + fn extract_global_curve( + partial: &PartialHalfEdge, + ) -> Option>> { + fn extract_global_curve_from_curve( + partial: &PartialHalfEdge, + ) -> Option>> { + partial.curve.as_ref()?.global_form() + } + + fn extract_global_curve_from_global_form( + partial: &PartialHalfEdge, + ) -> Option>> { + Some(partial.global_form.as_ref()?.curve()?.clone().into()) + } + + extract_global_curve_from_curve(partial) + .or_else(|| extract_global_curve_from_global_form(partial)) + } + let [from, to] = self .vertices .clone() @@ -123,7 +142,7 @@ impl PartialHalfEdge { }); let curve = PartialCurve { - global_form: self.extract_global_curve(), + global_form: extract_global_curve(&self), ..PartialCurve::default() } .with_surface(surface) @@ -168,25 +187,6 @@ impl PartialHalfEdge { HalfEdge::new(curve, vertices, global_form) } - - fn extract_global_curve( - &self, - ) -> Option>> { - fn extract_global_curve_from_curve( - partial: &PartialHalfEdge, - ) -> Option>> { - partial.curve.as_ref()?.global_form() - } - - fn extract_global_curve_from_global_form( - partial: &PartialHalfEdge, - ) -> Option>> { - Some(partial.global_form.as_ref()?.curve()?.clone().into()) - } - - extract_global_curve_from_curve(self) - .or_else(|| extract_global_curve_from_global_form(self)) - } } impl From for PartialHalfEdge { From 60b734d633e5e388b8fca141ddd0dbcb283fb3d0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 15:51:31 +0200 Subject: [PATCH 10/11] Add `MaybePartial::update_partial` --- crates/fj-kernel/src/partial/edge.rs | 15 +++++---------- crates/fj-kernel/src/partial/maybe_partial.rs | 11 +++++++++++ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/crates/fj-kernel/src/partial/edge.rs b/crates/fj-kernel/src/partial/edge.rs index 2e6a4116a..cc374e7bf 100644 --- a/crates/fj-kernel/src/partial/edge.rs +++ b/crates/fj-kernel/src/partial/edge.rs @@ -148,16 +148,11 @@ impl PartialHalfEdge { .with_surface(surface) .as_line_from_points(points); - let vertices = - [(from, 0.), (to, 1.)].map(|(vertex, position)| match vertex { - MaybePartial::Partial(vertex) => { - let vertex = vertex - .with_position([position]) - .with_curve(curve.clone()); - MaybePartial::from(vertex) - } - _ => vertex, - }); + let vertices = [(from, 0.), (to, 1.)].map(|(vertex, position)| { + vertex.update_partial(|vertex| { + vertex.with_position([position]).with_curve(curve.clone()) + }) + }); self.curve = Some(curve.into()); self.vertices = Some(vertices); diff --git a/crates/fj-kernel/src/partial/maybe_partial.rs b/crates/fj-kernel/src/partial/maybe_partial.rs index 01f2e6af6..4be91ebbc 100644 --- a/crates/fj-kernel/src/partial/maybe_partial.rs +++ b/crates/fj-kernel/src/partial/maybe_partial.rs @@ -18,6 +18,17 @@ pub enum MaybePartial { } impl MaybePartial { + /// If this is a partial object, update it + pub fn update_partial( + self, + f: impl FnOnce(T::PartialForm) -> T::PartialForm, + ) -> Self { + match self { + Self::Partial(partial) => Self::Partial(f(partial)), + _ => self, + } + } + /// Return the full object, either directly or by building it pub fn into_full(self, stores: &Stores) -> T { match self { From 79b89201440943ea3207f3b8486c64dd872e4dda Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 27 Sep 2022 15:53:40 +0200 Subject: [PATCH 11/11] Don't duplicate curves when building `HalfEdge` --- crates/fj-kernel/src/partial/edge.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/partial/edge.rs b/crates/fj-kernel/src/partial/edge.rs index cc374e7bf..f25dee173 100644 --- a/crates/fj-kernel/src/partial/edge.rs +++ b/crates/fj-kernel/src/partial/edge.rs @@ -169,7 +169,11 @@ impl PartialHalfEdge { let vertices = self .vertices .expect("Can't build `HalfEdge` without vertices") - .map(|vertex| vertex.into_full(stores)); + .map(|vertex| { + vertex + .update_partial(|vertex| vertex.with_curve(curve.clone())) + .into_full(stores) + }); let global_form = self .global_form