From 3a57c6a95b2a51b4d0b2bc6fba2f058b6391c247 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 16 Nov 2022 13:40:24 +0100 Subject: [PATCH 1/7] Refactor --- crates/fj-kernel/src/algorithms/transform/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/transform/mod.rs b/crates/fj-kernel/src/algorithms/transform/mod.rs index 5e370e198..c73554add 100644 --- a/crates/fj-kernel/src/algorithms/transform/mod.rs +++ b/crates/fj-kernel/src/algorithms/transform/mod.rs @@ -90,13 +90,13 @@ where transform: &Transform, objects: &Objects, ) -> Result { - match self { - Self::Full(full) => { - Ok(Self::Full(full.transform(transform, objects)?)) - } + let transformed = match self { + Self::Full(full) => Self::Full(full.transform(transform, objects)?), Self::Partial(partial) => { - Ok(Self::Partial(partial.transform(transform, objects)?)) + Self::Partial(partial.transform(transform, objects)?) } - } + }; + + Ok(transformed) } } From 29c547d9821083fbe21b557c6715e1ab39f3579b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 16 Nov 2022 14:03:08 +0100 Subject: [PATCH 2/7] Add `MaybePartial::position` --- crates/fj-kernel/src/partial/maybe_partial.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/fj-kernel/src/partial/maybe_partial.rs b/crates/fj-kernel/src/partial/maybe_partial.rs index 3c1b71c69..f9b56f1b3 100644 --- a/crates/fj-kernel/src/partial/maybe_partial.rs +++ b/crates/fj-kernel/src/partial/maybe_partial.rs @@ -207,6 +207,16 @@ impl MaybePartial { } } +impl MaybePartial { + /// Access the position + pub fn position(&self) -> Option> { + match self { + Self::Full(full) => Some(full.position()), + Self::Partial(partial) => partial.position, + } + } +} + impl MaybePartial { /// Access the curve pub fn curve(&self) -> MaybePartial { From 3bbd1add7d6c425db0e3adf13e8918dd0fe7305f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 16 Nov 2022 14:04:17 +0100 Subject: [PATCH 3/7] Be more careful about overriding global vertex pos The previous approach could lead to conflicts (resulting in a panic) under certain constellations. --- crates/fj-kernel/src/partial/objects/vertex.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/partial/objects/vertex.rs b/crates/fj-kernel/src/partial/objects/vertex.rs index 418917287..8ac5e94f1 100644 --- a/crates/fj-kernel/src/partial/objects/vertex.rs +++ b/crates/fj-kernel/src/partial/objects/vertex.rs @@ -102,7 +102,7 @@ pub struct PartialSurfaceVertex { impl PartialSurfaceVertex { /// Build a full [`SurfaceVertex`] from the partial surface vertex pub fn build( - self, + mut self, objects: &Objects, ) -> Result { let position = self @@ -112,13 +112,15 @@ impl PartialSurfaceVertex { .surface .expect("Can't build `SurfaceVertex` without `Surface`"); - let global_form = self - .global_form - .merge_with(PartialGlobalVertex::from_surface_and_position( - &surface.geometry(), - position, - )) - .into_full(objects)?; + if self.global_form.position().is_none() { + self.global_form = self.global_form.merge_with( + PartialGlobalVertex::from_surface_and_position( + &surface.geometry(), + position, + ), + ) + } + let global_form = self.global_form.into_full(objects)?; Ok(SurfaceVertex::new(position, surface, global_form)) } From 37c6d96b491692ed5826ae585182d2b3e71bf177 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 16 Nov 2022 14:05:58 +0100 Subject: [PATCH 4/7] Make `MaybePartial` transform result in partial This provides the caller with more flexibility. --- crates/fj-kernel/src/algorithms/transform/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/transform/mod.rs b/crates/fj-kernel/src/algorithms/transform/mod.rs index c73554add..f4092b981 100644 --- a/crates/fj-kernel/src/algorithms/transform/mod.rs +++ b/crates/fj-kernel/src/algorithms/transform/mod.rs @@ -91,7 +91,9 @@ where objects: &Objects, ) -> Result { let transformed = match self { - Self::Full(full) => Self::Full(full.transform(transform, objects)?), + Self::Full(full) => { + Self::Partial(full.to_partial().transform(transform, objects)?) + } Self::Partial(partial) => { Self::Partial(partial.transform(transform, objects)?) } From c185a07a3ca98946796448c85bd988ff3eeaef55 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 16 Nov 2022 14:06:55 +0100 Subject: [PATCH 5/7] Refactor --- crates/fj-kernel/src/algorithms/transform/mod.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/transform/mod.rs b/crates/fj-kernel/src/algorithms/transform/mod.rs index f4092b981..73fc3a77d 100644 --- a/crates/fj-kernel/src/algorithms/transform/mod.rs +++ b/crates/fj-kernel/src/algorithms/transform/mod.rs @@ -92,13 +92,11 @@ where ) -> Result { let transformed = match self { Self::Full(full) => { - Self::Partial(full.to_partial().transform(transform, objects)?) - } - Self::Partial(partial) => { - Self::Partial(partial.transform(transform, objects)?) + full.to_partial().transform(transform, objects)? } + Self::Partial(partial) => partial.transform(transform, objects)?, }; - Ok(transformed) + Ok(Self::Partial(transformed)) } } From 6e8524f370c613a62fb287fe893e9d185931db7b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 16 Nov 2022 14:08:25 +0100 Subject: [PATCH 6/7] Add comment --- crates/fj-kernel/src/algorithms/transform/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/fj-kernel/src/algorithms/transform/mod.rs b/crates/fj-kernel/src/algorithms/transform/mod.rs index 73fc3a77d..da9d1f771 100644 --- a/crates/fj-kernel/src/algorithms/transform/mod.rs +++ b/crates/fj-kernel/src/algorithms/transform/mod.rs @@ -97,6 +97,10 @@ where Self::Partial(partial) => partial.transform(transform, objects)?, }; + // Transforming a `MaybePartial` *always* results in a partial object. + // This provides the most flexibility to the caller, who might want to + // use the transformed partial object for merging or whatever else, + // before building it themselves. Ok(Self::Partial(transformed)) } } From e34db7e1cf1baeed55cceef26f457e64f2c0a784 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 16 Nov 2022 14:09:21 +0100 Subject: [PATCH 7/7] Refactor --- crates/fj-kernel/src/algorithms/transform/edge.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index 27d26e8d8..1d960cf20 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -3,7 +3,7 @@ use fj_math::Transform; use crate::{ objects::Objects, - partial::{MaybePartial, PartialGlobalEdge, PartialHalfEdge}, + partial::{PartialGlobalEdge, PartialHalfEdge}, validate::ValidationError, }; @@ -15,11 +15,7 @@ impl TransformObject for PartialHalfEdge { transform: &Transform, objects: &Objects, ) -> Result { - let curve: MaybePartial<_> = self - .curve - .into_partial() - .transform(transform, objects)? - .into(); + let curve = self.curve.transform(transform, objects)?; let vertices = self.vertices.try_map_ext( |vertex| -> Result<_, ValidationError> { let mut vertex =