Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up infrastructure for merging partial objects #1337

Merged
merged 20 commits into from
Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 18 additions & 21 deletions crates/fj-kernel/src/partial/maybe_partial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
106 changes: 106 additions & 0 deletions crates/fj-kernel/src/partial/merge.rs
Original file line number Diff line number Diff line change
@@ -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")
}
}
3 changes: 2 additions & 1 deletion crates/fj-kernel/src/partial/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
47 changes: 21 additions & 26 deletions crates/fj-kernel/src/partial/objects/curve.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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");
Expand All @@ -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 {
Expand All @@ -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
Expand Down
32 changes: 12 additions & 20 deletions crates/fj-kernel/src/partial/objects/cycle.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -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);
}

Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
Loading