Merge pull request #1814 from hannobraun/operations

Use richer face representation in `Tetrahedron`

crates/fj-kernel/src/algorithms/transform/mod.rs

@@ -69,7 +69,7 @@ pub trait TransformObject: Sized {
 
 impl<T> TransformObject for Handle<T>
 where
-    T: Clone + Insert + TransformObject + 'static,
+    T: Clone + Insert<Inserted = Handle<T>> + TransformObject + 'static,
 {
     fn transform_with_cache(
         self,

crates/fj-kernel/src/operations/build/face.rs

@@ -3,7 +3,7 @@ use fj_math::Point;
 
 use crate::{
     objects::{Cycle, Face, HalfEdge, Surface, Vertex},
-    operations::Insert,
+    operations::{Insert, IsInserted, IsInsertedNo},
     services::Services,
     storage::Handle,
 };
@@ -62,9 +62,9 @@ impl BuildFace for Face {}
 /// Currently code that deals with `Polygon` might assume that the polygon has
 /// no holes. Unless you create a `Polygon` yourself, or modify a `Polygon`'s
 /// `face` field to have interior cycles, this should not affect you.
-pub struct Polygon<const D: usize> {
+pub struct Polygon<const D: usize, I: IsInserted = IsInsertedNo> {
     /// The face that forms the polygon
-    pub face: Face,
+    pub face: I::T<Face>,
 
     /// The edges of the polygon
     pub edges: [Handle<HalfEdge>; D],

crates/fj-kernel/src/operations/build/shell.rs

@@ -2,12 +2,11 @@ use fj_math::Point;
 
 use crate::{
     objects::{Face, Shell},
-    operations::{Insert, JoinCycle, UpdateFace},
+    operations::{Insert, IsInsertedYes, JoinCycle, UpdateFace},
     services::Services,
-    storage::Handle,
 };
 
-use super::BuildFace;
+use super::{BuildFace, Polygon};
 
 /// Build a [`Shell`]
 pub trait BuildShell {
@@ -35,39 +34,35 @@ pub trait BuildShell {
     ) -> Tetrahedron {
         let [a, b, c, d] = points.map(Into::into);
 
-        let abc = Face::triangle([a, b, c], services).face;
+        let abc = Face::triangle([a, b, c], services);
         let bad =
-            Face::triangle([b, a, d], services)
+            Face::triangle([b, a, d], services).update_exterior(|cycle| {
-                .face
-                .update_exterior(|cycle| {
                 cycle
-                        .join_to(abc.exterior(), 0..=0, 0..=0, services)
+                    .join_to(abc.face.exterior(), 0..=0, 0..=0, services)
                     .insert(services)
             });
         let dac =
-            Face::triangle([d, a, c], services)
+            Face::triangle([d, a, c], services).update_exterior(|cycle| {
-                .face
-                .update_exterior(|cycle| {
                 cycle
-                        .join_to(abc.exterior(), 1..=1, 2..=2, services)
+                    .join_to(abc.face.exterior(), 1..=1, 2..=2, services)
-                        .join_to(bad.exterior(), 0..=0, 1..=1, services)
+                    .join_to(bad.face.exterior(), 0..=0, 1..=1, services)
                     .insert(services)
             });
         let cbd =
-            Face::triangle([c, b, d], services)
+            Face::triangle([c, b, d], services).update_exterior(|cycle| {
-                .face
-                .update_exterior(|cycle| {
                 cycle
-                        .join_to(abc.exterior(), 0..=0, 1..=1, services)
+                    .join_to(abc.face.exterior(), 0..=0, 1..=1, services)
-                        .join_to(bad.exterior(), 1..=1, 2..=2, services)
+                    .join_to(bad.face.exterior(), 1..=1, 2..=2, services)
-                        .join_to(dac.exterior(), 2..=2, 2..=2, services)
+                    .join_to(dac.face.exterior(), 2..=2, 2..=2, services)
                     .insert(services)
             });
 
-        let faces = [abc, bad, dac, cbd].map(|face| face.insert(services));
+        let triangles =
-        let shell = Shell::new(faces.clone());
+            [abc, bad, dac, cbd].map(|triangle| triangle.insert(services));
+        let shell =
+            Shell::new(triangles.iter().map(|triangle| triangle.face.clone()));
 
-        let [abc, bad, dac, cbd] = faces;
+        let [abc, bad, dac, cbd] = triangles;
 
         Tetrahedron {
             shell,
@@ -93,14 +88,14 @@ pub struct Tetrahedron {
     pub shell: Shell,
 
     /// The face formed by the points `a`, `b`, and `c`.
-    pub abc: Handle<Face>,
+    pub abc: Polygon<3, IsInsertedYes>,
 
     /// The face formed by the points `b`, `a`, and `d`.
-    pub bad: Handle<Face>,
+    pub bad: Polygon<3, IsInsertedYes>,
 
     /// The face formed by the points `d`, `a`, and `c`.
-    pub dac: Handle<Face>,
+    pub dac: Polygon<3, IsInsertedYes>,
 
     /// The face formed by the points `c`, `b`, and `d`.
-    pub cbd: Handle<Face>,
+    pub cbd: Polygon<3, IsInsertedYes>,
 }

crates/fj-kernel/src/operations/insert.rs

@@ -7,20 +7,30 @@ use crate::{
     storage::Handle,
 };
 
+use super::Polygon;
+
 /// Insert an object into its respective store
 ///
 /// This is the only primitive operation that is directly understood by
 /// `Service<Objects>`. All other operations are built on top of it.
 pub trait Insert: Sized {
+    /// The type of `Self`, once it has been inserted
+    ///
+    /// Usually this is just `Handle<Self>`, but there are some more complex
+    /// cases where this type needs to be customized.
+    type Inserted;
+
     /// Insert the object into its respective store
-    fn insert(self, services: &mut Services) -> Handle<Self>;
+    fn insert(self, services: &mut Services) -> Self::Inserted;
 }
 
 macro_rules! impl_insert {
     ($($ty:ty, $store:ident;)*) => {
         $(
             impl Insert for $ty {
-                fn insert(self, services: &mut Services) -> Handle<Self> {
+                type Inserted = Handle<Self>;
+
+                fn insert(self, services: &mut Services) -> Self::Inserted {
                     let handle = services.objects.$store.reserve();
                     let object = (handle.clone(), self).into();
                     services.insert_object(object);
@@ -42,3 +52,42 @@ impl_insert!(
     Surface, surfaces;
     Vertex, vertices;
 );
+
+/// Indicate whether an object has been inserted
+///
+/// Intended to be used as a type parameter bound for structs that need to track
+/// whether their contents have been inserted or not.
+pub trait IsInserted {
+    /// The type of the object for which the insertion status is tracked
+    type T<T>;
+}
+
+/// Indicate that an object has been inserted
+///
+/// See [`IsInserted`].
+pub struct IsInsertedYes;
+
+impl IsInserted for IsInsertedYes {
+    type T<T> = Handle<T>;
+}
+
+/// Indicate that an object has not been inserted
+///
+/// See [`IsInserted`].
+pub struct IsInsertedNo;
+
+impl IsInserted for IsInsertedNo {
+    type T<T> = T;
+}
+
+impl<const D: usize> Insert for Polygon<D, IsInsertedNo> {
+    type Inserted = Polygon<D, IsInsertedYes>;
+
+    fn insert(self, services: &mut Services) -> Self::Inserted {
+        Polygon {
+            face: self.face.insert(services),
+            edges: self.edges,
+            vertices: self.vertices,
+        }
+    }
+}

crates/fj-kernel/src/operations/mod.rs

@@ -10,7 +10,7 @@ pub use self::{
         BuildCycle, BuildFace, BuildHalfEdge, BuildShell, BuildSurface,
         Polygon, Tetrahedron,
     },
-    insert::Insert,
+    insert::{Insert, IsInserted, IsInsertedNo, IsInsertedYes},
     join::JoinCycle,
     update::{UpdateCycle, UpdateFace, UpdateHalfEdge, UpdateShell},
 };

crates/fj-kernel/src/operations/update/face.rs

@@ -1,5 +1,8 @@
+use std::array;
+
 use crate::{
     objects::{Cycle, Face},
+    operations::Polygon,
     storage::Handle,
 };
 
@@ -47,3 +50,41 @@ impl UpdateFace for Face {
         )
     }
 }
+
+impl<const D: usize> UpdateFace for Polygon<D> {
+    fn update_exterior(
+        &self,
+        f: impl FnOnce(&Handle<Cycle>) -> Handle<Cycle>,
+    ) -> Self {
+        let face = self.face.update_exterior(f);
+        let edges = array::from_fn(|i| {
+            face.exterior()
+                .nth_half_edge(i)
+                .expect("Operation should not have changed length of cycle")
+                .clone()
+        });
+        let vertices = array::from_fn(|i| {
+            // The duplicated code here is unfortunate, but unless we get a
+            // stable `array::each_ref` and something like `array::unzip`, I'm
+            // not sure how to avoid it.
+            face.exterior()
+                .nth_half_edge(i)
+                .expect("Operation should not have changed length of cycle")
+                .start_vertex()
+                .clone()
+        });
+
+        Polygon {
+            face,
+            edges,
+            vertices,
+        }
+    }
+
+    fn add_interiors(
+        &self,
+        _: impl IntoIterator<Item = Handle<Cycle>>,
+    ) -> Self {
+        panic!("Adding interiors to `Polygon` is not supported.")
+    }
+}

crates/fj-kernel/src/validate/shell.rs

@@ -210,7 +210,7 @@ mod tests {
             [[0., 0., 0.], [0., 1., 0.], [1., 0., 0.], [0., 0., 1.]],
             &mut services,
         );
-        let invalid = valid.shell.update_face(&valid.abc, |face| {
+        let invalid = valid.shell.update_face(&valid.abc.face, |face| {
             face.update_exterior(|cycle| {
                 cycle
                     .update_nth_half_edge(0, |half_edge| {
@@ -243,7 +243,7 @@ mod tests {
             [[0., 0., 0.], [0., 1., 0.], [1., 0., 0.], [0., 0., 1.]],
             &mut services,
         );
-        let invalid = valid.shell.remove_face(&valid.abc);
+        let invalid = valid.shell.remove_face(&valid.abc.face);
 
         valid.shell.validate_and_return_first_error()?;
         assert_contains_err!(