From d142184cbafc649a3e451ce5b99ef0bcc4f2951a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 14:17:04 +0100 Subject: [PATCH 01/24] Implement merge infrastructure for partial objects --- crates/fj-kernel/src/partial/maybe_partial.rs | 14 ++++++++ crates/fj-kernel/src/partial/mod.rs | 1 + crates/fj-kernel/src/partial/objects/curve.rs | 27 +++++++++++++++- crates/fj-kernel/src/partial/objects/cycle.rs | 21 +++++++++++- crates/fj-kernel/src/partial/objects/edge.rs | 32 ++++++++++++++++++- crates/fj-kernel/src/partial/objects/mod.rs | 4 +++ .../fj-kernel/src/partial/objects/vertex.rs | 27 +++++++++++++++- crates/fj-kernel/src/partial/traits.rs | 3 ++ crates/fj-kernel/src/partial/util.rs | 26 +++++++++++++++ 9 files changed, 151 insertions(+), 4 deletions(-) create mode 100644 crates/fj-kernel/src/partial/util.rs diff --git a/crates/fj-kernel/src/partial/maybe_partial.rs b/crates/fj-kernel/src/partial/maybe_partial.rs index 7431df451..1e617db9c 100644 --- a/crates/fj-kernel/src/partial/maybe_partial.rs +++ b/crates/fj-kernel/src/partial/maybe_partial.rs @@ -64,6 +64,20 @@ impl MaybePartial { } } + /// Merge this `MaybePartial` with another of the same type + pub fn merge_with(self, other: Self) -> Self { + match (self, other) { + (Self::Full(_), Self::Full(_)) => { + panic!("Can't merge two full objects") + } + (Self::Full(full), Self::Partial(_)) + | (Self::Partial(_), Self::Full(full)) => Self::Full(full), + (Self::Partial(a), Self::Partial(b)) => { + Self::Partial(a.merge_with(b)) + } + } + } + /// Return or build a full object /// /// If this already is a full object, it is returned. If this is a partial diff --git a/crates/fj-kernel/src/partial/mod.rs b/crates/fj-kernel/src/partial/mod.rs index 87ac53f2d..172c05cef 100644 --- a/crates/fj-kernel/src/partial/mod.rs +++ b/crates/fj-kernel/src/partial/mod.rs @@ -37,6 +37,7 @@ mod maybe_partial; mod objects; mod traits; +mod util; pub use self::{ maybe_partial::MaybePartial, diff --git a/crates/fj-kernel/src/partial/objects/curve.rs b/crates/fj-kernel/src/partial/objects/curve.rs index f55b97924..f8840e1a0 100644 --- a/crates/fj-kernel/src/partial/objects/curve.rs +++ b/crates/fj-kernel/src/partial/objects/curve.rs @@ -1,6 +1,6 @@ use crate::{ objects::{Curve, GlobalCurve, Objects, Surface}, - partial::MaybePartial, + partial::{util::merge_options, MaybePartial}, path::SurfacePath, storage::Handle, validate::ValidationError, @@ -59,6 +59,26 @@ impl PartialCurve { self } + /// Merge this partial object with another + pub fn merge_with(self, other: Self) -> Self { + // This is harder than it should be, as `global_form` uses the redundant + // `Option>` representation. There's some code relying + // on that though, so we have to live with it for now. + let global_form = match (self.global_form, other.global_form) { + (Some(a), Some(b)) => Some(a.merge_with(b)), + (Some(global_form), None) | (None, Some(global_form)) => { + Some(global_form) + } + (None, None) => None, + }; + + Self { + path: merge_options(self.path, other.path), + surface: merge_options(self.surface, other.surface), + global_form, + } + } + /// Build a full [`Curve`] from the partial curve pub fn build( self, @@ -102,6 +122,11 @@ impl From<&Curve> for PartialCurve { pub struct PartialGlobalCurve; impl PartialGlobalCurve { + /// Merge this partial object with another + pub fn merge_with(self, _: Self) -> Self { + Self + } + /// Build a full [`GlobalCurve`] from the partial global curve pub fn build( self, diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index fe2163112..f35d10b79 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -1,6 +1,6 @@ use crate::{ objects::{Cycle, HalfEdge, Objects, Surface}, - partial::MaybePartial, + partial::{util::merge_options, MaybePartial}, storage::Handle, validate::ValidationError, }; @@ -43,6 +43,25 @@ impl PartialCycle { self } + /// Merge this partial object with another + pub fn merge_with(self, other: Self) -> Self { + let a_is_empty = self.half_edges.is_empty(); + let b_is_empty = other.half_edges.is_empty(); + let half_edges = match (a_is_empty, b_is_empty) { + (true, true) => { + panic!("Can't merge `PartialHalfEdge`, if both have half-edges") + } + (true, false) => self.half_edges, + (false, true) => other.half_edges, + (false, false) => self.half_edges, // doesn't matter which we use + }; + + Self { + surface: merge_options(self.surface, other.surface), + half_edges, + } + } + /// Build a full [`Cycle`] from the partial cycle pub fn build( mut self, diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 4265d158c..0df8dbaca 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -6,7 +6,10 @@ use crate::{ Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, Surface, Vertex, }, - partial::MaybePartial, + partial::{ + util::{merge_arrays, merge_options}, + MaybePartial, + }, storage::Handle, validate::ValidationError, }; @@ -125,6 +128,16 @@ impl PartialHalfEdge { self } + /// Merge this partial object with another + pub fn merge_with(self, other: Self) -> Self { + Self { + surface: merge_options(self.surface, other.surface), + curve: self.curve.merge_with(other.curve), + vertices: merge_arrays(self.vertices, other.vertices), + global_form: self.global_form.merge_with(other.global_form), + } + } + /// Build a full [`HalfEdge`] from the partial half-edge pub fn build( self, @@ -210,6 +223,23 @@ impl PartialGlobalEdge { self } + /// Merge this partial object with another + pub fn merge_with(self, other: Self) -> Self { + // This is harder than it needs to be, because `vertices` uses the + // redundant combination of `Option` and `MaybePartial`. There's some + // code relying on that, however, so we have to live with it for now. + let vertices = match (self.vertices, other.vertices) { + (Some(a), Some(b)) => Some(merge_arrays(a, b)), + (Some(vertices), None) | (None, Some(vertices)) => Some(vertices), + (None, None) => None, + }; + + Self { + curve: self.curve.merge_with(other.curve), + vertices, + } + } + /// Build a full [`GlobalEdge`] from the partial global edge pub fn build( self, diff --git a/crates/fj-kernel/src/partial/objects/mod.rs b/crates/fj-kernel/src/partial/objects/mod.rs index 4032519a7..751063c23 100644 --- a/crates/fj-kernel/src/partial/objects/mod.rs +++ b/crates/fj-kernel/src/partial/objects/mod.rs @@ -27,6 +27,10 @@ macro_rules! impl_traits { impl Partial for $partial { type Full = $full; + fn merge_with(self, other: Self) -> Self { + self.merge_with(other) + } + fn build(self, objects: &Objects) -> Result< Handle, diff --git a/crates/fj-kernel/src/partial/objects/vertex.rs b/crates/fj-kernel/src/partial/objects/vertex.rs index 778d9b068..b04a67b02 100644 --- a/crates/fj-kernel/src/partial/objects/vertex.rs +++ b/crates/fj-kernel/src/partial/objects/vertex.rs @@ -3,7 +3,7 @@ use fj_math::Point; use crate::{ builder::GlobalVertexBuilder, objects::{Curve, GlobalVertex, Objects, Surface, SurfaceVertex, Vertex}, - partial::MaybePartial, + partial::{util::merge_options, MaybePartial}, storage::Handle, validate::ValidationError, }; @@ -67,6 +67,15 @@ impl PartialVertex { self } + /// Merge this partial object with another + pub fn merge_with(self, other: Self) -> Self { + Self { + position: merge_options(self.position, other.position), + curve: self.curve.merge_with(other.curve), + surface_form: self.surface_form.merge_with(other.surface_form), + } + } + /// Build a full [`Vertex`] from the partial vertex /// /// # Panics @@ -170,6 +179,15 @@ impl PartialSurfaceVertex { self } + /// Merge this partial object with another + pub fn merge_with(self, other: Self) -> Self { + Self { + position: merge_options(self.position, other.position), + surface: merge_options(self.surface, other.surface), + global_form: self.global_form.merge_with(other.global_form), + } + } + /// Build a full [`SurfaceVertex`] from the partial surface vertex pub fn build( self, @@ -232,6 +250,13 @@ impl PartialGlobalVertex { self } + /// Merge this partial object with another + pub fn merge_with(self, other: Self) -> Self { + Self { + position: merge_options(self.position, other.position), + } + } + /// Build a full [`GlobalVertex`] from the partial global vertex pub fn build( self, diff --git a/crates/fj-kernel/src/partial/traits.rs b/crates/fj-kernel/src/partial/traits.rs index 6a15f8b48..e18bf1c4a 100644 --- a/crates/fj-kernel/src/partial/traits.rs +++ b/crates/fj-kernel/src/partial/traits.rs @@ -68,6 +68,9 @@ pub trait Partial: Default + for<'a> From<&'a Self::Full> { /// The type representing the full variant of this object type Full; + /// Merge another partial object of the same type into this one + fn merge_with(self, other: Self) -> Self; + /// Build a full object from this partial one /// /// Implementations of this method will typically try to infer any missing diff --git a/crates/fj-kernel/src/partial/util.rs b/crates/fj-kernel/src/partial/util.rs new file mode 100644 index 000000000..e0605ee1e --- /dev/null +++ b/crates/fj-kernel/src/partial/util.rs @@ -0,0 +1,26 @@ +use fj_interop::ext::ArrayExt; + +use super::{HasPartial, MaybePartial}; + +pub fn merge_options(a: Option, b: Option) -> Option +where + T: Eq, +{ + if a == b { + return a; + } + + // We know that `a != b`, or we wouldn't have made it here. + if a.is_some() && b.is_some() { + panic!("Can't merge `Option`s if both are defined"); + } + + a.xor(b) +} + +pub fn merge_arrays( + a: [MaybePartial; 2], + b: [MaybePartial; 2], +) -> [MaybePartial; 2] { + a.zip_ext(b).map(|(a, b)| a.merge_with(b)) +} From 0192f5d718ca2a8017311b0f101cae74af4634d9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 12:27:32 +0100 Subject: [PATCH 02/24] Inline method --- crates/fj-kernel/src/builder/edge.rs | 6 ++++-- crates/fj-kernel/src/partial/objects/edge.rs | 7 ------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index e23263f9c..c280a6ba7 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -55,7 +55,8 @@ impl HalfEdgeBuilder for PartialHalfEdge { [Scalar::ZERO, Scalar::TAU].map(|coord| Point::from([coord])); let global_vertex = self - .extract_global_vertices() + .global_form() + .vertices() .map(|[global_form, _]| global_form) .unwrap_or_else(|| { GlobalVertex::partial() @@ -147,7 +148,8 @@ impl HalfEdgeBuilder for PartialHalfEdge { must_switch_order }; - self.extract_global_vertices() + self.global_form() + .vertices() .map( |[a, b]| { if must_switch_order { diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 0df8dbaca..9ba74de41 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -55,13 +55,6 @@ impl PartialHalfEdge { .unwrap_or_else(|| self.global_form.curve()) } - /// Access the vertices of the global form, if available - pub fn extract_global_vertices( - &self, - ) -> Option<[MaybePartial; 2]> { - self.global_form.vertices() - } - /// Update the partial half-edge with the given surface pub fn with_surface(mut self, surface: Option>) -> Self { if let Some(surface) = surface { From 880c3f24c4c59da57039eaaa1e3a2967e9a21165 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 12:33:06 +0100 Subject: [PATCH 03/24] Inline variable --- crates/fj-kernel/src/partial/objects/edge.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 9ba74de41..fc0987335 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -136,10 +136,9 @@ impl PartialHalfEdge { self, objects: &Objects, ) -> Result, ValidationError> { - let surface = self.surface; let curve = self .curve - .update_partial(|curve| curve.with_surface(surface)) + .update_partial(|curve| curve.with_surface(self.surface)) .into_full(objects)?; let vertices = self.vertices.try_map_ext(|vertex| { vertex From 6417176bee33abdbca487e4891265023fb259a74 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 12:35:45 +0100 Subject: [PATCH 04/24] Inline variable --- crates/fj-kernel/src/builder/edge.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index c280a6ba7..f0d037fde 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -86,10 +86,9 @@ impl HalfEdgeBuilder for PartialHalfEdge { self, points: [impl Into>; 2], ) -> Self { - let surface = self.surface(); let vertices = points.map(|point| { let surface_form = SurfaceVertex::partial() - .with_surface(surface.clone()) + .with_surface(self.surface()) .with_position(Some(point)); Vertex::partial().with_surface_form(Some(surface_form)) From 4a5c6191248e169188016b3dde26918cc60eba39 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 12:41:19 +0100 Subject: [PATCH 05/24] Remove redundant field --- crates/fj-kernel/src/partial/objects/edge.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index fc0987335..45b8df442 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -6,10 +6,7 @@ use crate::{ Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, Surface, Vertex, }, - partial::{ - util::{merge_arrays, merge_options}, - MaybePartial, - }, + partial::{util::merge_arrays, MaybePartial}, storage::Handle, validate::ValidationError, }; @@ -19,7 +16,6 @@ use crate::{ /// See [`crate::partial`] for more information. #[derive(Clone, Debug, Default)] pub struct PartialHalfEdge { - surface: Option>, curve: MaybePartial, vertices: [MaybePartial; 2], global_form: MaybePartial, @@ -28,7 +24,7 @@ pub struct PartialHalfEdge { impl PartialHalfEdge { /// Access the surface that the [`HalfEdge`]'s [`Curve`] is defined in pub fn surface(&self) -> Option> { - self.surface.clone() + self.curve.surface() } /// Access the curve that the [`HalfEdge`] is defined in @@ -58,7 +54,9 @@ impl PartialHalfEdge { /// Update the partial half-edge with the given surface pub fn with_surface(mut self, surface: Option>) -> Self { if let Some(surface) = surface { - self.surface = Some(surface); + self.curve = self + .curve + .update_partial(|curve| curve.with_surface(Some(surface))); } self } @@ -124,7 +122,6 @@ impl PartialHalfEdge { /// Merge this partial object with another pub fn merge_with(self, other: Self) -> Self { Self { - surface: merge_options(self.surface, other.surface), curve: self.curve.merge_with(other.curve), vertices: merge_arrays(self.vertices, other.vertices), global_form: self.global_form.merge_with(other.global_form), @@ -136,10 +133,7 @@ impl PartialHalfEdge { self, objects: &Objects, ) -> Result, ValidationError> { - let curve = self - .curve - .update_partial(|curve| curve.with_surface(self.surface)) - .into_full(objects)?; + let curve = self.curve.into_full(objects)?; let vertices = self.vertices.try_map_ext(|vertex| { vertex .update_partial(|vertex| vertex.with_curve(Some(curve.clone()))) @@ -165,7 +159,6 @@ impl From<&HalfEdge> for PartialHalfEdge { half_edge.vertices().clone().map(Into::into); Self { - surface: Some(half_edge.curve().surface().clone()), curve: half_edge.curve().clone().into(), vertices: [back_vertex, front_vertex], global_form: half_edge.global_form().clone().into(), From a3cd134d602da29646f0bf39092e50ca6a3b2794 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 13:05:40 +0100 Subject: [PATCH 06/24] Remove redundant code --- crates/fj-kernel/src/algorithms/transform/edge.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index 3e7dac5ae..71c72a8d1 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -15,15 +15,10 @@ impl TransformObject for PartialHalfEdge { transform: &Transform, objects: &Objects, ) -> Result { - let surface = self - .surface() - .map(|surface| surface.transform(transform, objects)) - .transpose()?; let curve: MaybePartial<_> = self .curve() .into_partial() .transform(transform, objects)? - .with_surface(surface.clone()) .into(); let vertices = self.vertices().try_map_ext( |vertex| -> Result<_, ValidationError> { @@ -41,7 +36,6 @@ impl TransformObject for PartialHalfEdge { .into(); Ok(Self::default() - .with_surface(surface) .with_curve(Some(curve)) .with_vertices(Some(vertices)) .with_global_form(global_form)) From 5d50fe53c2bd8d0f3a8902512573066431bfe2b7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 13:26:38 +0100 Subject: [PATCH 07/24] Fix doc comments --- crates/fj-kernel/src/partial/objects/edge.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 45b8df442..46670045f 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -72,7 +72,7 @@ impl PartialHalfEdge { self } - /// Update the partial half-edge with the given from vertex + /// Update the partial half-edge with the given back vertex pub fn with_back_vertex( mut self, vertex: Option>>, @@ -84,7 +84,7 @@ impl PartialHalfEdge { self } - /// Update the partial half-edge with the given from vertex + /// Update the partial half-edge with the given front vertex pub fn with_front_vertex( mut self, vertex: Option>>, From be48ee73bb5bc061f9f82ee99374b31885d8dea5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 13:33:09 +0100 Subject: [PATCH 08/24] Refactor --- crates/fj-kernel/src/builder/edge.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index f0d037fde..d7393e9cb 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -44,9 +44,10 @@ impl HalfEdgeBuilder for PartialHalfEdge { radius: impl Into, objects: &Objects, ) -> Result { - let curve = Curve::partial() + let curve = self + .curve() + .into_partial() .with_global_form(Some(self.extract_global_curve())) - .with_surface(self.surface()) .update_as_circle_from_radius(radius); let path = curve.path().expect("Expected path that was just created"); From 09be728293cb09fe15594ea78c80f49e0c9d1620 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 13:37:34 +0100 Subject: [PATCH 09/24] Refactor --- crates/fj-kernel/src/builder/edge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index d7393e9cb..04e8be27d 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -67,7 +67,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { let surface_vertex = SurfaceVertex::partial() .with_position(Some(path.point_from_path_coords(a_curve))) - .with_surface(self.surface()) + .with_surface(curve.surface()) .with_global_form(Some(global_vertex)) .build(objects)?; From c01a41e1e93116e03216c13bbab69013a70ef641 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 13:37:42 +0100 Subject: [PATCH 10/24] Inline redundant method --- crates/fj-kernel/src/builder/edge.rs | 3 ++- crates/fj-kernel/src/partial/objects/edge.rs | 5 ----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 04e8be27d..b85eea3c9 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -89,7 +89,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { ) -> Self { let vertices = points.map(|point| { let surface_form = SurfaceVertex::partial() - .with_surface(self.surface()) + .with_surface(self.curve().surface()) .with_position(Some(point)); Vertex::partial().with_surface_form(Some(surface_form)) @@ -104,6 +104,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { [&from, &to].map(|vertex| vertex.surface_form()); let surface = self + .curve() .surface() .or_else(|| from_surface.surface()) .or_else(|| to_surface.surface()) diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 46670045f..e5e466db7 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -22,11 +22,6 @@ pub struct PartialHalfEdge { } impl PartialHalfEdge { - /// Access the surface that the [`HalfEdge`]'s [`Curve`] is defined in - pub fn surface(&self) -> Option> { - self.curve.surface() - } - /// Access the curve that the [`HalfEdge`] is defined in pub fn curve(&self) -> MaybePartial { self.curve.clone() From 9741e40395694926313a83a63bf94e4c71b7ee25 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 14:26:23 +0100 Subject: [PATCH 11/24] Provide surface directly where it's needed --- .../src/algorithms/intersect/curve_edge.rs | 18 +++++++------- crates/fj-kernel/src/algorithms/sweep/edge.rs | 12 ++++++---- crates/fj-kernel/src/algorithms/sweep/face.rs | 12 ++++++---- .../fj-kernel/src/algorithms/sweep/vertex.rs | 3 +-- crates/fj-kernel/src/builder/cycle.rs | 3 +-- crates/fj-kernel/src/builder/edge.rs | 10 +++++--- crates/fj-kernel/src/builder/shell.rs | 9 ++++--- crates/fj-kernel/src/iter.rs | 6 +++-- crates/fj-kernel/src/objects/edge.rs | 6 ++--- crates/fj-kernel/src/validate/edge.rs | 24 ++++++++++++------- 10 files changed, 61 insertions(+), 42 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs index 065e7f44c..4b6376820 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs @@ -92,8 +92,7 @@ mod tests { .update_as_u_axis() .build(&objects)?; let half_edge = HalfEdge::partial() - .with_surface(Some(surface)) - .update_as_line_segment_from_points([[1., -1.], [1., 1.]]) + .update_as_line_segment_from_points(surface, [[1., -1.], [1., 1.]]) .build(&objects)?; let intersection = CurveEdgeIntersection::compute(&curve, &half_edge); @@ -117,8 +116,10 @@ mod tests { .update_as_u_axis() .build(&objects)?; let half_edge = HalfEdge::partial() - .with_surface(Some(surface)) - .update_as_line_segment_from_points([[-1., -1.], [-1., 1.]]) + .update_as_line_segment_from_points( + surface, + [[-1., -1.], [-1., 1.]], + ) .build(&objects)?; let intersection = CurveEdgeIntersection::compute(&curve, &half_edge); @@ -142,8 +143,10 @@ mod tests { .update_as_u_axis() .build(&objects)?; let half_edge = HalfEdge::partial() - .with_surface(Some(surface)) - .update_as_line_segment_from_points([[-1., -1.], [1., -1.]]) + .update_as_line_segment_from_points( + surface, + [[-1., -1.], [1., -1.]], + ) .build(&objects)?; let intersection = CurveEdgeIntersection::compute(&curve, &half_edge); @@ -162,8 +165,7 @@ mod tests { .update_as_u_axis() .build(&objects)?; let half_edge = HalfEdge::partial() - .with_surface(Some(surface)) - .update_as_line_segment_from_points([[-1., 0.], [1., 0.]]) + .update_as_line_segment_from_points(surface, [[-1., 0.], [1., 0.]]) .build(&objects)?; let intersection = CurveEdgeIntersection::compute(&curve, &half_edge); diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 40b44d62d..2f3cce225 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -199,8 +199,10 @@ mod tests { let objects = Objects::new(); let half_edge = HalfEdge::partial() - .with_surface(Some(objects.surfaces.xy_plane())) - .update_as_line_segment_from_points([[0., 0.], [1., 0.]]) + .update_as_line_segment_from_points( + objects.surfaces.xy_plane(), + [[0., 0.], [1., 0.]], + ) .build(&objects)?; let face = @@ -210,8 +212,10 @@ mod tests { let surface = objects.surfaces.xz_plane(); let bottom = HalfEdge::partial() - .with_surface(Some(surface.clone())) - .update_as_line_segment_from_points([[0., 0.], [1., 0.]]) + .update_as_line_segment_from_points( + surface.clone(), + [[0., 0.], [1., 0.]], + ) .build(&objects)?; let side_up = HalfEdge::partial() .with_surface(Some(surface.clone())) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 2a95a7438..eb44fc87c 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -125,8 +125,10 @@ mod tests { .array_windows_ext() .map(|&[a, b]| { let half_edge = HalfEdge::partial() - .with_surface(Some(objects.surfaces.xy_plane())) - .update_as_line_segment_from_points([a, b]) + .update_as_line_segment_from_points( + objects.surfaces.xy_plane(), + [a, b], + ) .build(&objects)?; (half_edge, Color::default()).sweep(UP, &objects) }) @@ -167,8 +169,10 @@ mod tests { .array_windows_ext() .map(|&[a, b]| { let half_edge = HalfEdge::partial() - .with_surface(Some(objects.surfaces.xy_plane())) - .update_as_line_segment_from_points([a, b]) + .update_as_line_segment_from_points( + objects.surfaces.xy_plane(), + [a, b], + ) .build(&objects)? .reverse(&objects)?; (half_edge, Color::default()).sweep(DOWN, &objects) diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index 9946dc987..f348b561d 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -191,8 +191,7 @@ mod tests { (vertex, surface.clone()).sweep([0., 0., 1.], &objects)?; let expected_half_edge = HalfEdge::partial() - .with_surface(Some(surface)) - .update_as_line_segment_from_points([[0., 0.], [0., 1.]]) + .update_as_line_segment_from_points(surface, [[0., 0.], [0., 1.]]) .build(&objects)?; assert_eq!(half_edge, expected_half_edge); Ok(()) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 04251a2fc..4f8196ee6 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -127,8 +127,7 @@ impl CycleBuilder for PartialCycle { self.with_half_edges(Some( HalfEdge::partial() - .with_surface(Some(surface)) - .update_as_line_segment_from_points(vertices), + .update_as_line_segment_from_points(surface, vertices), )) } } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index b85eea3c9..65c20f4a0 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -3,7 +3,7 @@ use fj_math::{Point, Scalar}; use crate::{ objects::{ - Curve, GlobalVertex, Objects, SurfaceVertex, Vertex, + Curve, GlobalVertex, Objects, Surface, SurfaceVertex, Vertex, VerticesInNormalizedOrder, }, partial::{HasPartial, PartialGlobalEdge, PartialHalfEdge}, @@ -31,6 +31,7 @@ pub trait HalfEdgeBuilder: Sized { /// Update partial half-edge as a line segment, from the given points fn update_as_line_segment_from_points( self, + surface: Handle, points: [impl Into>; 2], ) -> Self; @@ -85,17 +86,20 @@ impl HalfEdgeBuilder for PartialHalfEdge { fn update_as_line_segment_from_points( self, + surface: Handle, points: [impl Into>; 2], ) -> Self { let vertices = points.map(|point| { let surface_form = SurfaceVertex::partial() - .with_surface(self.curve().surface()) + .with_surface(Some(surface.clone())) .with_position(Some(point)); Vertex::partial().with_surface_form(Some(surface_form)) }); - self.with_vertices(Some(vertices)).update_as_line_segment() + self.with_surface(Some(surface)) + .with_vertices(Some(vertices)) + .update_as_line_segment() } fn update_as_line_segment(self) -> Self { diff --git a/crates/fj-kernel/src/builder/shell.rs b/crates/fj-kernel/src/builder/shell.rs index 83c73cf29..d9d5e2705 100644 --- a/crates/fj-kernel/src/builder/shell.rs +++ b/crates/fj-kernel/src/builder/shell.rs @@ -89,12 +89,11 @@ impl<'a> ShellBuilder<'a> { .zip(&surfaces) .map(|(half_edge, surface)| { HalfEdge::partial() - .with_surface(Some(surface.clone())) .with_global_form(Some(half_edge.global_form().clone())) - .update_as_line_segment_from_points([ - [Z, Z], - [edge_length, Z], - ]) + .update_as_line_segment_from_points( + surface.clone(), + [[Z, Z], [edge_length, Z]], + ) .build(self.objects) .unwrap() }) diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 718f7c5f0..280969f42 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -485,8 +485,10 @@ mod tests { let objects = Objects::new(); let object = HalfEdge::partial() - .with_surface(Some(objects.surfaces.xy_plane())) - .update_as_line_segment_from_points([[0., 0.], [1., 0.]]) + .update_as_line_segment_from_points( + objects.surfaces.xy_plane(), + [[0., 0.], [1., 0.]], + ) .build(&objects); assert_eq!(1, object.curve_iter().count()); diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 5e5dc45fb..ea20d4e7d 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -165,12 +165,10 @@ mod tests { let b = [1., 0.]; let a_to_b = HalfEdge::partial() - .with_surface(Some(surface.clone())) - .update_as_line_segment_from_points([a, b]) + .update_as_line_segment_from_points(surface.clone(), [a, b]) .build(&objects)?; let b_to_a = HalfEdge::partial() - .with_surface(Some(surface)) - .update_as_line_segment_from_points([b, a]) + .update_as_line_segment_from_points(surface, [b, a]) .build(&objects)?; assert_eq!(a_to_b.global_form(), b_to_a.global_form()); diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 3f2f2e3ad..ffc50c29f 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -207,8 +207,10 @@ mod tests { let objects = Objects::new(); let valid = HalfEdge::partial() - .with_surface(Some(objects.surfaces.xy_plane())) - .update_as_line_segment_from_points([[0., 0.], [1., 0.]]) + .update_as_line_segment_from_points( + objects.surfaces.xy_plane(), + [[0., 0.], [1., 0.]], + ) .build(&objects)?; let invalid = { let mut vertices = valid.vertices().clone(); @@ -232,8 +234,10 @@ mod tests { let objects = Objects::new(); let valid = HalfEdge::partial() - .with_surface(Some(objects.surfaces.xy_plane())) - .update_as_line_segment_from_points([[0., 0.], [1., 0.]]) + .update_as_line_segment_from_points( + objects.surfaces.xy_plane(), + [[0., 0.], [1., 0.]], + ) .build(&objects)?; let invalid = HalfEdge::new( valid.vertices().clone(), @@ -255,8 +259,10 @@ mod tests { let objects = Objects::new(); let valid = HalfEdge::partial() - .with_surface(Some(objects.surfaces.xy_plane())) - .update_as_line_segment_from_points([[0., 0.], [1., 0.]]) + .update_as_line_segment_from_points( + objects.surfaces.xy_plane(), + [[0., 0.], [1., 0.]], + ) .build(&objects)?; let invalid = HalfEdge::new( valid.vertices().clone(), @@ -285,8 +291,10 @@ mod tests { let objects = Objects::new(); let valid = HalfEdge::partial() - .with_surface(Some(objects.surfaces.xy_plane())) - .update_as_line_segment_from_points([[0., 0.], [1., 0.]]) + .update_as_line_segment_from_points( + objects.surfaces.xy_plane(), + [[0., 0.], [1., 0.]], + ) .build(&objects)?; let invalid = HalfEdge::new( valid.vertices().clone().try_map_ext(|vertex| { From 5125b6698361a15b1b8fac2b9c6df75ea4351482 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 14:38:54 +0100 Subject: [PATCH 12/24] Refactor --- crates/fj-kernel/src/partial/objects/cycle.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index f35d10b79..5fa2f2391 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -28,7 +28,13 @@ impl PartialCycle { /// Update the partial cycle with the given surface pub fn with_surface(mut self, surface: Option>) -> Self { if let Some(surface) = surface { - self.surface = Some(surface); + self.surface = Some(surface.clone()); + + for half_edge in &mut self.half_edges { + *half_edge = half_edge.clone().update_partial(|half_edge| { + half_edge.with_surface(Some(surface.clone())) + }); + } } self } @@ -68,7 +74,6 @@ impl PartialCycle { objects: &Objects, ) -> Result, ValidationError> { let surface = self.surface.expect("Need surface to build `Cycle`"); - let surface_for_edges = surface.clone(); let half_edges = { let last_vertex = self .half_edges @@ -117,9 +122,7 @@ impl PartialCycle { partial.with_surface_form(previous_vertex) }); - half_edge - .with_surface(Some(surface_for_edges.clone())) - .with_back_vertex(Some(back)) + half_edge.with_back_vertex(Some(back)) }) .into_full(objects)?; From 67857902b6748fcaa047930e20da3a2b89bf50d4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 14:44:09 +0100 Subject: [PATCH 13/24] Refactor --- crates/fj-kernel/src/partial/objects/cycle.rs | 7 +------ crates/fj-kernel/src/partial/objects/edge.rs | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index 5fa2f2391..865175a5a 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -73,7 +73,6 @@ impl PartialCycle { mut self, objects: &Objects, ) -> Result, ValidationError> { - let surface = self.surface.expect("Need surface to build `Cycle`"); let half_edges = { let last_vertex = self .half_edges @@ -89,11 +88,7 @@ impl PartialCycle { .map(|(half_edge, vertex, surface_vertex)| -> Result<_, ValidationError> { - let surface_vertex = surface_vertex - .update_partial(|surface_vertex| { - surface_vertex.with_surface(Some(surface.clone())) - }) - .into_full(objects)?; + let surface_vertex = surface_vertex.into_full(objects)?; *half_edge = half_edge.clone().update_partial(|half_edge| { diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index e5e466db7..2aadf3bb4 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -49,9 +49,21 @@ impl PartialHalfEdge { /// Update the partial half-edge with the given surface pub fn with_surface(mut self, surface: Option>) -> Self { if let Some(surface) = surface { - self.curve = self - .curve - .update_partial(|curve| curve.with_surface(Some(surface))); + self.curve = self.curve.update_partial(|curve| { + curve.with_surface(Some(surface.clone())) + }); + + self.vertices = self.vertices.map(|vertex| { + vertex.update_partial(|vertex| { + let surface_form = vertex.surface_form().update_partial( + |surface_vertex| { + surface_vertex.with_surface(Some(surface.clone())) + }, + ); + + vertex.with_surface_form(Some(surface_form)) + }) + }); } self } From 308a2fd80fb390ef8c99ad8ca9b934cd119c00cc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:06:39 +0100 Subject: [PATCH 14/24] Add `MaybePartial::curve` --- crates/fj-kernel/src/partial/maybe_partial.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/fj-kernel/src/partial/maybe_partial.rs b/crates/fj-kernel/src/partial/maybe_partial.rs index 1e617db9c..5e7e7c1da 100644 --- a/crates/fj-kernel/src/partial/maybe_partial.rs +++ b/crates/fj-kernel/src/partial/maybe_partial.rs @@ -174,6 +174,14 @@ impl MaybePartial { } impl MaybePartial { + /// Access the curve + pub fn curve(&self) -> MaybePartial { + match self { + Self::Full(full) => full.curve().clone().into(), + Self::Partial(partial) => partial.curve(), + } + } + /// Access the front vertex pub fn front(&self) -> MaybePartial { match self { From 774d7846860904e76107f8260a221a90d8a6ed4e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:12:27 +0100 Subject: [PATCH 15/24] Refactor --- crates/fj-kernel/src/builder/face.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index bb34c3768..56588d045 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -50,9 +50,14 @@ impl<'a> FaceBuilder<'a> { mut self, points: impl IntoIterator>>, ) -> Self { + let surface = self + .surface + .as_ref() + .expect("Need surface to create polygon"); + self.exterior = Some( Cycle::partial() - .with_surface(self.surface.clone()) + .with_surface(Some(surface.clone())) .with_poly_chain_from_points(points) .close_with_line_segment() .build(self.objects) @@ -75,9 +80,14 @@ impl<'a> FaceBuilder<'a> { mut self, points: impl IntoIterator>>, ) -> Self { + let surface = self + .surface + .as_ref() + .expect("Need surface to build polygon."); + self.interiors.push( Cycle::partial() - .with_surface(self.surface.clone()) + .with_surface(Some(surface.clone())) .with_poly_chain_from_points(points) .close_with_line_segment() .build(self.objects) From 5c9b6e0f07422c8e670373791eaa0fe0feec3551 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:14:06 +0100 Subject: [PATCH 16/24] Provide surface right where it is needed --- crates/fj-kernel/src/builder/cycle.rs | 9 +++++++-- crates/fj-kernel/src/builder/face.rs | 4 ++-- crates/fj-kernel/src/iter.rs | 7 +++++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 4f8196ee6..e6107c3d6 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -1,8 +1,9 @@ use fj_math::Point; use crate::{ - objects::{Curve, HalfEdge, SurfaceVertex, Vertex}, + objects::{Curve, HalfEdge, Surface, SurfaceVertex, Vertex}, partial::{HasPartial, MaybePartial, PartialCycle}, + storage::Handle, }; use super::{CurveBuilder, HalfEdgeBuilder}; @@ -18,6 +19,7 @@ pub trait CycleBuilder { /// Update the partial cycle with a polygonal chain from the provided points fn with_poly_chain_from_points( self, + surface: Handle, points: impl IntoIterator>>, ) -> Self; @@ -99,10 +101,13 @@ impl CycleBuilder for PartialCycle { fn with_poly_chain_from_points( self, + surface: Handle, points: impl IntoIterator>>, ) -> Self { self.with_poly_chain(points.into_iter().map(|position| { - SurfaceVertex::partial().with_position(Some(position)) + SurfaceVertex::partial() + .with_surface(Some(surface.clone())) + .with_position(Some(position)) })) } diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 56588d045..37b574548 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -58,7 +58,7 @@ impl<'a> FaceBuilder<'a> { self.exterior = Some( Cycle::partial() .with_surface(Some(surface.clone())) - .with_poly_chain_from_points(points) + .with_poly_chain_from_points(surface.clone(), points) .close_with_line_segment() .build(self.objects) .unwrap(), @@ -88,7 +88,7 @@ impl<'a> FaceBuilder<'a> { self.interiors.push( Cycle::partial() .with_surface(Some(surface.clone())) - .with_poly_chain_from_points(points) + .with_poly_chain_from_points(surface.clone(), points) .close_with_line_segment() .build(self.objects) .unwrap(), diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 280969f42..7fc8f606f 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -399,8 +399,11 @@ mod tests { let surface = objects.surfaces.xy_plane(); let object = Cycle::partial() - .with_surface(Some(surface)) - .with_poly_chain_from_points([[0., 0.], [1., 0.], [0., 1.]]) + .with_surface(Some(surface.clone())) + .with_poly_chain_from_points( + surface, + [[0., 0.], [1., 0.], [0., 1.]], + ) .close_with_line_segment() .build(&objects); From bb242f108f3781223d61270796e2622d6a475c76 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:15:36 +0100 Subject: [PATCH 17/24] Refactor --- crates/fj-kernel/src/builder/cycle.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index e6107c3d6..13c1d264c 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -51,9 +51,8 @@ impl CycleBuilder for PartialCycle { let mut half_edges = Vec::new(); for vertex_next in iter { if let Some(vertex_prev) = previous { - let surface = self + let surface = vertex_prev .surface() - .clone() .expect("Need surface to extend cycle with poly-chain"); let position_prev = vertex_prev From 9604c8340541e437c3c80f1bdd73466645db1135 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:17:19 +0100 Subject: [PATCH 18/24] Remove redundant struct field --- crates/fj-kernel/src/partial/objects/cycle.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index 865175a5a..4709d6337 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -1,6 +1,6 @@ use crate::{ objects::{Cycle, HalfEdge, Objects, Surface}, - partial::{util::merge_options, MaybePartial}, + partial::MaybePartial, storage::Handle, validate::ValidationError, }; @@ -10,14 +10,15 @@ use crate::{ /// See [`crate::partial`] for more information. #[derive(Clone, Debug, Default)] pub struct PartialCycle { - surface: Option>, half_edges: Vec>, } impl PartialCycle { /// Access the surface that the [`Cycle`] is defined in pub fn surface(&self) -> Option> { - self.surface.clone() + self.half_edges + .first() + .and_then(|half_edge| half_edge.curve().surface()) } /// Access the half-edges that make up the [`Cycle`] @@ -28,8 +29,6 @@ impl PartialCycle { /// Update the partial cycle with the given surface pub fn with_surface(mut self, surface: Option>) -> Self { if let Some(surface) = surface { - self.surface = Some(surface.clone()); - for half_edge in &mut self.half_edges { *half_edge = half_edge.clone().update_partial(|half_edge| { half_edge.with_surface(Some(surface.clone())) @@ -62,10 +61,7 @@ impl PartialCycle { (false, false) => self.half_edges, // doesn't matter which we use }; - Self { - surface: merge_options(self.surface, other.surface), - half_edges, - } + Self { half_edges } } /// Build a full [`Cycle`] from the partial cycle @@ -138,7 +134,6 @@ impl PartialCycle { impl From<&Cycle> for PartialCycle { fn from(cycle: &Cycle) -> Self { Self { - surface: Some(cycle.surface().clone()), half_edges: cycle.half_edges().cloned().map(Into::into).collect(), } } From 19da56d345f6a3992fc27f7c12db1f23a29920a5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:17:41 +0100 Subject: [PATCH 19/24] Remove redundant code --- crates/fj-kernel/src/builder/face.rs | 2 -- crates/fj-kernel/src/iter.rs | 1 - 2 files changed, 3 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 37b574548..0f217d409 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -57,7 +57,6 @@ impl<'a> FaceBuilder<'a> { self.exterior = Some( Cycle::partial() - .with_surface(Some(surface.clone())) .with_poly_chain_from_points(surface.clone(), points) .close_with_line_segment() .build(self.objects) @@ -87,7 +86,6 @@ impl<'a> FaceBuilder<'a> { self.interiors.push( Cycle::partial() - .with_surface(Some(surface.clone())) .with_poly_chain_from_points(surface.clone(), points) .close_with_line_segment() .build(self.objects) diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 7fc8f606f..c27d3fbaf 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -399,7 +399,6 @@ mod tests { let surface = objects.surfaces.xy_plane(); let object = Cycle::partial() - .with_surface(Some(surface.clone())) .with_poly_chain_from_points( surface, [[0., 0.], [1., 0.], [0., 1.]], From a988da3e5a9303923c80622386ed89a4b0f87c2a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:21:11 +0100 Subject: [PATCH 20/24] Remove redundant code --- crates/fj-kernel/src/builder/shell.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/builder/shell.rs b/crates/fj-kernel/src/builder/shell.rs index d9d5e2705..1785af97b 100644 --- a/crates/fj-kernel/src/builder/shell.rs +++ b/crates/fj-kernel/src/builder/shell.rs @@ -187,10 +187,8 @@ impl<'a> ShellBuilder<'a> { .zip(sides_up) .zip(tops.clone()) .zip(sides_down) - .zip(surfaces) - .map(|((((bottom, side_up), top), side_down), surface)| { + .map(|(((bottom, side_up), top), side_down)| { let cycle = Cycle::partial() - .with_surface(Some(surface)) .with_half_edges([bottom, side_up, top, side_down]) .build(self.objects) .unwrap(); From a79bd1a7c223b1b68fb3d78572deefa511167a8b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:33:22 +0100 Subject: [PATCH 21/24] Refactor --- .../fj-kernel/src/algorithms/transform/cycle.rs | 15 ++------------- crates/fj-kernel/src/partial/objects/cycle.rs | 14 ++++++++++---- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/transform/cycle.rs b/crates/fj-kernel/src/algorithms/transform/cycle.rs index 1cbf50694..5f222af69 100644 --- a/crates/fj-kernel/src/algorithms/transform/cycle.rs +++ b/crates/fj-kernel/src/algorithms/transform/cycle.rs @@ -12,22 +12,11 @@ impl TransformObject for PartialCycle { transform: &Transform, objects: &Objects, ) -> Result { - let surface = self - .surface() - .map(|surface| surface.transform(transform, objects)) - .transpose()?; let half_edges = self .half_edges() - .map(|edge| { - Ok(edge - .into_partial() - .transform(transform, objects)? - .with_surface(surface.clone())) - }) + .map(|edge| edge.into_partial().transform(transform, objects)) .collect::, ValidationError>>()?; - Ok(Self::default() - .with_surface(surface) - .with_half_edges(half_edges)) + Ok(Self::default().with_half_edges(half_edges)) } } diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index 4709d6337..b4e55ef07 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -1,6 +1,6 @@ use crate::{ objects::{Cycle, HalfEdge, Objects, Surface}, - partial::MaybePartial, + partial::{util::merge_options, MaybePartial}, storage::Handle, validate::ValidationError, }; @@ -43,9 +43,15 @@ impl PartialCycle { mut self, half_edges: impl IntoIterator>>, ) -> Self { - self.half_edges - .extend(half_edges.into_iter().map(Into::into)); - self + let half_edges = half_edges.into_iter().map(Into::into); + + let mut surface = self.surface(); + for half_edge in half_edges { + surface = merge_options(surface, half_edge.curve().surface()); + self.half_edges.push(half_edge); + } + + self.with_surface(surface) } /// Merge this partial object with another From 23fe9a5156aaeca8c8545360e5893b2f845583df Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:33:58 +0100 Subject: [PATCH 22/24] Update order of code --- crates/fj-kernel/src/partial/objects/cycle.rs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index b4e55ef07..0c6a8fb16 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -14,28 +14,16 @@ pub struct PartialCycle { } impl PartialCycle { - /// Access the surface that the [`Cycle`] is defined in - pub fn surface(&self) -> Option> { - self.half_edges - .first() - .and_then(|half_edge| half_edge.curve().surface()) - } - /// Access the half-edges that make up the [`Cycle`] pub fn half_edges(&self) -> impl Iterator> { self.half_edges.clone().into_iter() } - /// Update the partial cycle with the given surface - pub fn with_surface(mut self, surface: Option>) -> Self { - if let Some(surface) = surface { - for half_edge in &mut self.half_edges { - *half_edge = half_edge.clone().update_partial(|half_edge| { - half_edge.with_surface(Some(surface.clone())) - }); - } - } - self + /// Access the surface that the [`Cycle`] is defined in + pub fn surface(&self) -> Option> { + self.half_edges + .first() + .and_then(|half_edge| half_edge.curve().surface()) } /// Update the partial cycle with the given half-edges @@ -54,6 +42,18 @@ impl PartialCycle { self.with_surface(surface) } + /// Update the partial cycle with the given surface + pub fn with_surface(mut self, surface: Option>) -> Self { + if let Some(surface) = surface { + for half_edge in &mut self.half_edges { + *half_edge = half_edge.clone().update_partial(|half_edge| { + half_edge.with_surface(Some(surface.clone())) + }); + } + } + self + } + /// Merge this partial object with another pub fn merge_with(self, other: Self) -> Self { let a_is_empty = self.half_edges.is_empty(); From 142bbf8c7736aa3b18ec85d85c42e99851503f00 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:37:51 +0100 Subject: [PATCH 23/24] Update doc comments --- crates/fj-kernel/src/partial/objects/cycle.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index 0c6a8fb16..7d2c146a3 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -19,14 +19,21 @@ impl PartialCycle { self.half_edges.clone().into_iter() } - /// Access the surface that the [`Cycle`] is defined in + /// Access the surface that the [`Cycle`]'s [`HalfEdge`]s are defined in pub fn surface(&self) -> Option> { self.half_edges .first() .and_then(|half_edge| half_edge.curve().surface()) } - /// Update the partial cycle with the given half-edges + /// Add the provided half-edges to the partial cycle + /// + /// This will merge all the surfaces of the added half-edges. All added + /// half-edges will end up with the same merged surface. + /// + /// # Panics + /// + /// Panics, if the surfaces can't be merged. pub fn with_half_edges( mut self, half_edges: impl IntoIterator>>, @@ -42,7 +49,9 @@ impl PartialCycle { self.with_surface(surface) } - /// Update the partial cycle with the given surface + /// Update the partial cycle with the provided surface + /// + /// All [`HalfEdge`]s will be updated with this surface. pub fn with_surface(mut self, surface: Option>) -> Self { if let Some(surface) = surface { for half_edge in &mut self.half_edges { From cb81f3b3e571e993c83269ac27cb2851881f1cb4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 4 Nov 2022 15:39:42 +0100 Subject: [PATCH 24/24] Remove redundant code This code is no longer required, as `PartialCycle` makes sure that all surfaces are properly propagated. --- crates/fj-kernel/src/builder/cycle.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 13c1d264c..86103d627 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -62,12 +62,8 @@ impl CycleBuilder for PartialCycle { .position() .expect("Need surface position to extend cycle"); - let from = vertex_prev.update_partial(|partial| { - partial.with_surface(Some(surface.clone())) - }); - let to = vertex_next.update_partial(|partial| { - partial.with_surface(Some(surface.clone())) - }); + let from = vertex_prev; + let to = vertex_next; previous = Some(to.clone());