Merge pull request #1337 from hannobraun/merge

Clean up infrastructure for merging partial objects

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<T: HasPartial> MaybePartial<T> {
         }
     }
 
-    /// Merge this `MaybePartial` with another of the same type
-    pub fn merge_with(self, other: impl Into<Self>) -> 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<T> MergeWith for MaybePartial<T>
+where
+    T: HasPartial,
+    T::Partial: MergeWith,
+{
+    fn merge_with(self, other: impl Into<Self>) -> 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<T> From<Handle<T>> for MaybePartial<T>
 where
     T: HasPartial,

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>) -> 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<T>(pub T);
+
+impl<T, const N: usize> MergeWith for [T; N]
+where
+    T: MergeWith,
+{
+    fn merge_with(self, other: impl Into<Self>) -> Self {
+        self.into_iter_fixed()
+            .zip(other.into())
+            .collect::<[_; N]>()
+            .map(|(a, b)| a.merge_with(b))
+    }
+}
+
+impl<T> MergeWith for Option<T>
+where
+    T: PartialEq,
+{
+    fn merge_with(self, other: impl Into<Self>) -> 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<T> MergeWith for Mergeable<Option<T>>
+where
+    T: MergeWith,
+{
+    fn merge_with(self, other: impl Into<Self>) -> 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<T> MergeWith for Vec<T> {
+    fn merge_with(self, other: impl Into<Self>) -> 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<T> MergeWith for Mergeable<Vec<T>> {
+    fn merge_with(mut self, other: impl Into<Self>) -> Self {
+        self.0.extend(other.into().0);
+        self
+    }
+}
+
+impl<T> MergeWith for Handle<T> {
+    fn merge_with(self, other: impl Into<Self>) -> Self {
+        if self.id() == other.into().id() {
+            return self;
+        }
+
+        panic!("Can't merge two distinct objects")
+    }
+}

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,

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<MaybePartial<_>>` 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<Curve, ValidationError> {
         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>) -> 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<GlobalCurve, ValidationError> {
         Ok(GlobalCurve)
     }
 }
 
+impl MergeWith for PartialGlobalCurve {
+    fn merge_with(self, _: impl Into<Self>) -> Self {
+        Self
+    }
+}
+
 impl From<&GlobalCurve> for PartialGlobalCurve {
     fn from(_: &GlobalCurve) -> Self {
         Self

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>) -> 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 {