diff --git a/src/kernel/shape/handle.rs b/src/kernel/shape/handle.rs index b3bfa5d6a..ba2b4fa21 100644 --- a/src/kernel/shape/handle.rs +++ b/src/kernel/shape/handle.rs @@ -1,13 +1,54 @@ use std::{hash::Hash, ops::Deref, rc::Rc}; +/// A handle to an object stored within [`Shape`] +/// +/// If an object of type `T` (this could be `Curve`, `Vertex`, etc.) is added to +/// `Shape`, a `Handle` is returned. This handle is then used in topological +/// types to refer to the object, instead of having those types own an instance +/// of the object. +/// +/// This approach has two advantages: +/// +/// 1. The object can't be mutated through the handle. Since an object can be +/// referred to by multiple other objects, mutating it locally would have no +/// effect on those other references. `Handle` preventing that removes this +/// source of errors. +/// 2. The object is guaranteed to be in `Shape`, as `Handle`s can't be created +/// any other way. This means that if the `Shape` needs to be modified, any +/// objects can be updated once, without requiring an update of all the other +/// objects that reference it. +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct Handle(Storage); + +impl Handle { + /// Access the object that the handle references + /// + /// `Handle` also implements `Deref`, but as that can be inconvenient to use + /// in some cases, this method is an inherent proxy for that. + pub fn get(&self) -> &T { + self.0.deref() + } +} + +impl Deref for Handle { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.0.deref() + } +} + +/// Internal type used in collections within [`Shape`] #[derive(Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Storage(Rc); +pub(super) struct Storage(Rc); impl Storage { + /// Create a [`Storage`] instance that wraps the provided object pub(super) fn new(value: T) -> Self { Self(Rc::new(value)) } + /// Create a handle that refers to this [`Storage`] instance pub(super) fn handle(&self) -> Handle { Handle(self.clone()) } @@ -21,19 +62,11 @@ impl Deref for Storage { } } +// Deriving `Clone` would only derive `Clone` where `T: Clone`. This +// implementation doesn't have that limitation, providing `Clone` for all +// `Handle`s instead. impl Clone for Storage { fn clone(&self) -> Self { Self(self.0.clone()) } } - -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Handle(Storage); - -impl Deref for Handle { - type Target = T; - - fn deref(&self) -> &Self::Target { - self.0.deref() - } -} diff --git a/src/kernel/shapes/difference_2d.rs b/src/kernel/shapes/difference_2d.rs index c6ea0cd69..84ba64cb7 100644 --- a/src/kernel/shapes/difference_2d.rs +++ b/src/kernel/shapes/difference_2d.rs @@ -53,7 +53,7 @@ impl ToShape for fj::Difference2d { [&mut a, &mut b].map(|shape| shape.faces().all().next().unwrap()); let (cycles_a, cycles_b, surface_a, surface_b) = - match ((*face_a).clone(), (*face_b).clone()) { + match (face_a.get().clone(), face_b.get().clone()) { ( Face::Face { cycles: a, diff --git a/src/kernel/shapes/union.rs b/src/kernel/shapes/union.rs index 8147510fc..a843cca7e 100644 --- a/src/kernel/shapes/union.rs +++ b/src/kernel/shapes/union.rs @@ -19,10 +19,10 @@ impl ToShape for fj::Union { // See issue: // https://github.com/hannobraun/Fornjot/issues/42 for face in a.faces().all() { - shape.faces().add((*face).clone()); + shape.faces().add(face.get().clone()); } for face in b.faces().all() { - shape.faces().add((*face).clone()); + shape.faces().add(face.get().clone()); } shape