diff --git a/crates/fj-kernel/src/partial/maybe_partial.rs b/crates/fj-kernel/src/partial/maybe_partial.rs index 4a232fb71..03caa5e6c 100644 --- a/crates/fj-kernel/src/partial/maybe_partial.rs +++ b/crates/fj-kernel/src/partial/maybe_partial.rs @@ -11,7 +11,7 @@ use crate::{ validate::{Validate, ValidationError}, }; -use super::{HasPartial, Partial}; +use super::{HasPartial, MergeWith, Partial}; /// Can be used everywhere either a partial or full objects are accepted /// @@ -65,26 +65,6 @@ impl MaybePartial { } } - /// Merge this `MaybePartial` with another of the same type - pub fn merge_with(self, other: impl Into) -> Self { - match (self, other.into()) { - (Self::Full(a), Self::Full(b)) => { - if a.id() != b.id() { - panic!("Can't merge two full objects") - } - - // If they're equal, which they are, if we reach this point, - // then merging them is a no-op. - Self::Full(a) - } - (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 @@ -128,6 +108,23 @@ where } } +impl MergeWith for MaybePartial +where + T: HasPartial, + T::Partial: MergeWith, +{ + fn merge_with(self, other: impl Into) -> Self { + match (self, other.into()) { + (Self::Full(a), Self::Full(b)) => Self::Full(a.merge_with(b)), + (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)) + } + } + } +} + impl From> for MaybePartial where T: HasPartial, diff --git a/crates/fj-kernel/src/partial/merge.rs b/crates/fj-kernel/src/partial/merge.rs new file mode 100644 index 000000000..dac76cf55 --- /dev/null +++ b/crates/fj-kernel/src/partial/merge.rs @@ -0,0 +1,106 @@ +use iter_fixed::IntoIteratorFixed; + +use crate::storage::Handle; + +/// Trait for merging partial objects +/// +/// Implemented for all partial objects themselves, and also some related types +/// that partial objects usually contain. +pub trait MergeWith: Sized { + /// Merge this object with another + /// + /// # Panics + /// + /// Merging two objects that cannot be merged is considered a programmer + /// error and will result in a panic. + fn merge_with(self, other: impl Into) -> Self; +} + +/// Wrapper struct that indicates that the contents can be merged +/// +/// Used in connection with [`MergeWith`] to select one implementation over +/// another. +pub struct Mergeable(pub T); + +impl MergeWith for [T; N] +where + T: MergeWith, +{ + fn merge_with(self, other: impl Into) -> Self { + self.into_iter_fixed() + .zip(other.into()) + .collect::<[_; N]>() + .map(|(a, b)| a.merge_with(b)) + } +} + +impl MergeWith for Option +where + T: PartialEq, +{ + fn merge_with(self, other: impl Into) -> Self { + let other = other.into(); + + if self == other { + return self; + } + + // We know that `self != other`, or we wouldn't have made it here. + if self.is_some() && other.is_some() { + panic!("Can't merge two `Option`s that are both `Some`") + } + + self.xor(other) + } +} + +// We wouldn't need to use `Mergeable` here, if we had `specialization`: +// https://doc.rust-lang.org/nightly/unstable-book/language-features/specialization.html +// +// Or maybe `min_specialization`: +// https://doc.rust-lang.org/nightly/unstable-book/language-features/min-specialization.html +impl MergeWith for Mergeable> +where + T: MergeWith, +{ + fn merge_with(self, other: impl Into) -> Self { + let merged = match (self.0, other.into().0) { + (Some(a), Some(b)) => Some(a.merge_with(b)), + (a, b) => a.xor(b), + }; + + Self(merged) + } +} + +impl MergeWith for Vec { + fn merge_with(self, other: impl Into) -> Self { + let other = other.into(); + + match (self.is_empty(), other.is_empty()) { + (true, true) => { + panic!("Can't merge `PartialHalfEdge`, if both have half-edges") + } + (true, false) => other, + (false, true) => self, + (false, false) => self, // doesn't matter which we use + } + } +} + +impl MergeWith for Mergeable> { + fn merge_with(mut self, other: impl Into) -> Self { + self.0.extend(other.into().0); + self + } +} + +impl MergeWith for Handle { + fn merge_with(self, other: impl Into) -> Self { + if self.id() == other.into().id() { + return self; + } + + panic!("Can't merge two distinct objects") + } +} diff --git a/crates/fj-kernel/src/partial/mod.rs b/crates/fj-kernel/src/partial/mod.rs index f7e845d07..2f81c0d25 100644 --- a/crates/fj-kernel/src/partial/mod.rs +++ b/crates/fj-kernel/src/partial/mod.rs @@ -35,12 +35,13 @@ //! [#1147]: https://github.com/hannobraun/Fornjot/issues/1147 mod maybe_partial; +mod merge; mod objects; mod traits; -mod util; pub use self::{ maybe_partial::MaybePartial, + merge::{MergeWith, Mergeable}, objects::{ curve::{PartialCurve, PartialGlobalCurve}, cycle::PartialCycle, diff --git a/crates/fj-kernel/src/partial/objects/curve.rs b/crates/fj-kernel/src/partial/objects/curve.rs index 6c08f77fd..233b6b2c7 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::{util::merge_options, MaybePartial}, + partial::{MaybePartial, MergeWith, Mergeable}, path::SurfacePath, storage::Handle, validate::ValidationError, @@ -59,26 +59,6 @@ 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, objects: &Objects) -> Result { let path = self.path.expect("Can't build `Curve` without path"); @@ -95,6 +75,20 @@ impl PartialCurve { } } +impl MergeWith for PartialCurve { + fn merge_with(self, other: impl Into) -> Self { + let other = other.into(); + + Self { + path: self.path.merge_with(other.path), + surface: self.surface.merge_with(other.surface), + global_form: Mergeable(self.global_form) + .merge_with(Mergeable(other.global_form)) + .0, + } + } +} + impl From<&Curve> for PartialCurve { fn from(curve: &Curve) -> Self { Self { @@ -117,17 +111,18 @@ 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, _: &Objects) -> Result { Ok(GlobalCurve) } } +impl MergeWith for PartialGlobalCurve { + fn merge_with(self, _: impl Into) -> Self { + Self + } +} + impl From<&GlobalCurve> for PartialGlobalCurve { fn from(_: &GlobalCurve) -> Self { Self diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index bbefef967..a802133e3 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -1,9 +1,7 @@ use crate::{ builder::HalfEdgeBuilder, objects::{Cycle, HalfEdge, Objects, Surface}, - partial::{ - util::merge_options, MaybePartial, PartialHalfEdge, PartialVertex, - }, + partial::{MaybePartial, MergeWith, PartialHalfEdge, PartialVertex}, storage::Handle, validate::ValidationError, }; @@ -45,7 +43,7 @@ impl PartialCycle { let mut surface = self.surface(); for half_edge in half_edges { - surface = merge_options(surface, half_edge.curve().surface()); + surface = surface.merge_with(half_edge.curve().surface()); self.half_edges.push(half_edge); } @@ -66,22 +64,6 @@ 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 { half_edges } - } - /// Build a full [`Cycle`] from the partial cycle pub fn build( mut self, @@ -146,6 +128,16 @@ impl PartialCycle { } } +impl MergeWith for PartialCycle { + fn merge_with(self, other: impl Into) -> Self { + let other = other.into(); + + Self { + half_edges: self.half_edges.merge_with(other.half_edges), + } + } +} + impl From<&Cycle> for PartialCycle { fn from(cycle: &Cycle) -> Self { Self { diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index f2d7e3cea..40e32906e 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -6,7 +6,7 @@ use crate::{ Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, Surface, Vertex, }, - partial::{util::merge_arrays, MaybePartial}, + partial::{MaybePartial, MergeWith, Mergeable}, storage::Handle, validate::ValidationError, }; @@ -92,15 +92,6 @@ impl PartialHalfEdge { self } - /// Merge this partial object with another - pub fn merge_with(self, other: Self) -> Self { - Self { - 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, objects: &Objects) -> Result { let curve = self.curve.into_full(objects)?; @@ -121,6 +112,18 @@ impl PartialHalfEdge { } } +impl MergeWith for PartialHalfEdge { + fn merge_with(self, other: impl Into) -> Self { + let other = other.into(); + + Self { + curve: self.curve.merge_with(other.curve), + vertices: self.vertices.merge_with(other.vertices), + global_form: self.global_form.merge_with(other.global_form), + } + } +} + impl From<&HalfEdge> for PartialHalfEdge { fn from(half_edge: &HalfEdge) -> Self { let [back_vertex, front_vertex] = @@ -176,23 +179,6 @@ 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, @@ -208,6 +194,19 @@ impl PartialGlobalEdge { } } +impl MergeWith for PartialGlobalEdge { + fn merge_with(self, other: impl Into) -> Self { + let other = other.into(); + + Self { + curve: self.curve.merge_with(other.curve), + vertices: Mergeable(self.vertices) + .merge_with(Mergeable(other.vertices)) + .0, + } + } +} + impl From<&GlobalEdge> for PartialGlobalEdge { fn from(global_edge: &GlobalEdge) -> Self { Self { diff --git a/crates/fj-kernel/src/partial/objects/face.rs b/crates/fj-kernel/src/partial/objects/face.rs index f2dcc8460..02d837484 100644 --- a/crates/fj-kernel/src/partial/objects/face.rs +++ b/crates/fj-kernel/src/partial/objects/face.rs @@ -2,7 +2,7 @@ use fj_interop::mesh::Color; use crate::{ objects::{Cycle, Face, Objects, Surface}, - partial::{util::merge_options, MaybePartial}, + partial::{MaybePartial, MergeWith, Mergeable}, storage::Handle, validate::ValidationError, }; @@ -70,19 +70,6 @@ impl PartialFace { self } - /// Merge this partial object with another - pub fn merge_with(self, other: Self) -> Self { - let mut interiors = self.interiors; - interiors.extend(other.interiors); - - Self { - surface: merge_options(self.surface, other.surface), - exterior: self.exterior.merge_with(other.exterior), - interiors, - color: merge_options(self.color, other.color), - } - } - /// Construct a polygon from a list of points pub fn build(self, objects: &Objects) -> Result { let exterior = self.exterior.into_full(objects)?; @@ -97,6 +84,21 @@ impl PartialFace { } } +impl MergeWith for PartialFace { + fn merge_with(self, other: impl Into) -> Self { + let other = other.into(); + + Self { + surface: self.surface.merge_with(other.surface), + exterior: self.exterior.merge_with(other.exterior), + interiors: Mergeable(self.interiors) + .merge_with(Mergeable(other.interiors)) + .0, + color: self.color.merge_with(other.color), + } + } +} + impl From<&Face> for PartialFace { fn from(face: &Face) -> Self { Self { diff --git a/crates/fj-kernel/src/partial/objects/mod.rs b/crates/fj-kernel/src/partial/objects/mod.rs index 277752a94..917f53a56 100644 --- a/crates/fj-kernel/src/partial/objects/mod.rs +++ b/crates/fj-kernel/src/partial/objects/mod.rs @@ -25,10 +25,6 @@ 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< Self::Full, diff --git a/crates/fj-kernel/src/partial/objects/vertex.rs b/crates/fj-kernel/src/partial/objects/vertex.rs index e7693ca50..eaa7ac355 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::{util::merge_options, MaybePartial}, + partial::{MaybePartial, MergeWith}, storage::Handle, validate::ValidationError, }; @@ -60,15 +60,6 @@ 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 @@ -99,6 +90,18 @@ impl PartialVertex { } } +impl MergeWith for PartialVertex { + fn merge_with(self, other: impl Into) -> Self { + let other = other.into(); + + Self { + position: self.position.merge_with(other.position), + curve: self.curve.merge_with(other.curve), + surface_form: self.surface_form.merge_with(other.surface_form), + } + } +} + impl From<&Vertex> for PartialVertex { fn from(vertex: &Vertex) -> Self { Self { @@ -165,15 +168,6 @@ 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, @@ -197,6 +191,18 @@ impl PartialSurfaceVertex { } } +impl MergeWith for PartialSurfaceVertex { + fn merge_with(self, other: impl Into) -> Self { + let other = other.into(); + + Self { + position: self.position.merge_with(other.position), + surface: self.surface.merge_with(other.surface), + global_form: self.global_form.merge_with(other.global_form), + } + } +} + impl From<&SurfaceVertex> for PartialSurfaceVertex { fn from(surface_vertex: &SurfaceVertex) -> Self { Self { @@ -232,13 +238,6 @@ 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, _: &Objects) -> Result { let position = self @@ -249,6 +248,16 @@ impl PartialGlobalVertex { } } +impl MergeWith for PartialGlobalVertex { + fn merge_with(self, other: impl Into) -> Self { + let other = other.into(); + + Self { + position: self.position.merge_with(other.position), + } + } +} + impl From<&GlobalVertex> for PartialGlobalVertex { fn from(global_vertex: &GlobalVertex) -> Self { Self { diff --git a/crates/fj-kernel/src/partial/traits.rs b/crates/fj-kernel/src/partial/traits.rs index c0b1a1d54..3b149317b 100644 --- a/crates/fj-kernel/src/partial/traits.rs +++ b/crates/fj-kernel/src/partial/traits.rs @@ -68,9 +68,6 @@ 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 deleted file mode 100644 index a0c11dc7c..000000000 --- a/crates/fj-kernel/src/partial/util.rs +++ /dev/null @@ -1,29 +0,0 @@ -use iter_fixed::IntoIteratorFixed; - -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.into_iter_fixed() - .zip(b) - .collect::<[_; 2]>() - .map(|(a, b)| a.merge_with(b)) -}