Allow pinwheels and singularities in graphs.

src/graph/mesh.rs

@@ -749,14 +749,11 @@ where
 
 #[cfg(test)]
 mod tests {
-    use nalgebra::{Point2, Point3, Vector3};
+    use nalgebra::{Point3, Vector3};
     use num::Zero;
-    use std::collections::HashSet;
 
     use crate::buffer::U3;
     use crate::geometry::*;
-    use crate::graph::mutation::face::{self, FaceRemoveCache};
-    use crate::graph::mutation::{Mutate, Mutation};
     use crate::graph::*;
     use crate::primitive::decompose::*;
     use crate::primitive::generate::*;
@@ -821,63 +818,6 @@ mod tests {
         assert_eq!(graph.err().unwrap(), GraphError::TopologyConflict);
     }
 
-    #[test]
-    fn error_on_singularity_mesh() {
-        // Construct a mesh with three non-neighboring triangles sharing a
-        // single vertex.
-        let graph = MeshGraph::<Point3<i32>>::from_raw_buffers_with_arity(
-            vec![0u32, 1, 2, 0, 3, 4, 0, 5, 6],
-            vec![
-                (0, 0, 0),
-                (1, -1, 0),
-                (-1, -1, 0),
-                (-3, 1, 0),
-                (-2, 1, 0),
-                (2, 1, 0),
-                (3, 1, 0),
-            ],
-            3,
-        );
-
-        assert_eq!(graph.err().unwrap(), GraphError::TopologyMalformed);
-
-        // Construct a mesh with three triangles forming a rectangle, where one
-        // vertex (at the origin) is shared by all three triangles.
-        let graph = MeshGraph::<Point2<i32>>::from_raw_buffers_with_arity(
-            vec![0u32, 1, 3, 1, 4, 3, 1, 2, 4],
-            vec![(-1, 0), (0, 0), (1, 0), (-1, 1), (1, 1)],
-            3,
-        )
-        .unwrap();
-        // TODO: Create a shared testing geometry that allows topology to be
-        //       marked and more easily located. Finding very specific geometry
-        //       like this is cumbersome.
-        // Find the "center" triangle and use a mutation to remove it. This
-        // creates a singularity, with the two remaining triangles sharing no
-        // edges but having a single common vertex.
-        let geometry = &[(0, 0), (1, 1), (-1, 1)]
-            .iter()
-            .cloned()
-            .collect::<HashSet<_>>();
-        let key = graph
-            .faces()
-            .find(|face| {
-                face.vertices()
-                    .map(|vertex| vertex.geometry.clone())
-                    .map(|position| (position.x, position.y))
-                    .collect::<HashSet<_>>()
-                    == *geometry
-            })
-            .unwrap()
-            .key();
-        let cache = FaceRemoveCache::snapshot(&graph, key).unwrap();
-        let mut mutation = Mutation::mutate(graph);
-        assert_eq!(
-            face::remove_with_cache(mutation, cache).err().unwrap(),
-            GraphError::TopologyConflict,
-        );
-    }
-
     // This test is a sanity check for mesh iterators, topological views, and
     // the unsafe transmutations used to coerce lifetimes.
     #[test]

src/graph/mutation/face.rs

@@ -207,11 +207,11 @@ where
         mutation.commit().and_then(move |core| {
             let (vertices, edges, ..) = core.into_storage();
             {
+                // TODO: Rejection of pinwheel connectivity has been removed.
+                //       Determine if this check is related in a way that is
+                //       inconsistent. If so, this should probably be removed.
                 let core = Core::empty().bind(&vertices).bind(&edges).bind(&faces);
                 for (vertex, faces) in singularities {
-                    // TODO: This will not detect exactly two faces joined by a single
-                    //       vertex. This is technically supported, but perhaps should
-                    //       be detected and rejected.
                     // Determine if any unreachable faces exist in the mesh. This
                     // cannot happen if the mesh is ultimately a manifold and edge
                     // connectivity heals.
@@ -278,8 +278,9 @@ where
         M: Reborrow,
         M::Target: AsStorage<Edge<G>> + AsStorage<Face<G>> + AsStorage<Vertex<G>>,
     {
-        // Verify that the region is not already occupied by a face and collect
-        // the incoming and outgoing edges for each vertex in the region.
+        // Verify that the minimal closed path is not already occupied by a
+        // face and collect the incoming and outgoing edges for each vertex in
+        // the region.
         let region = Region::from_keyed_storage(vertices, storage)?;
         if region.face().is_some() {
             return Err(GraphError::TopologyConflict);
@@ -299,7 +300,6 @@ where
     G: Geometry,
 {
     abc: FaceKey,
-    mutuals: Vec<VertexKey>,
     edges: Vec<EdgeKey>,
     phantom: PhantomData<G>,
 }
@@ -308,6 +308,7 @@ impl<G> FaceRemoveCache<G>
 where
     G: Geometry,
 {
+    // TODO: Should this require consistency?
     pub fn snapshot<M>(storage: M, abc: FaceKey) -> Result<Self, GraphError>
     where
         M: Reborrow,
@@ -318,7 +319,6 @@ where
         let edges = face.interior_edges().map(|edge| edge.key()).collect();
         Ok(FaceRemoveCache {
             abc,
-            mutuals: face.reachable_mutuals().into_iter().collect(),
             edges,
             phantom: PhantomData,
         })
@@ -514,6 +514,7 @@ where
     }
 }
 
+// TODO: Does this require a cache (or consistency)?
 pub fn remove_with_cache<M, N, G>(
     mut mutation: N,
     cache: FaceRemoveCache<G>,
@@ -523,50 +524,10 @@ where
     M: Consistent + From<OwnedCore<G>> + Into<OwnedCore<G>>,
     G: Geometry,
 {
-    let FaceRemoveCache {
-        abc,
-        mutuals,
-        edges,
-        ..
-    } = cache;
-    let mutation = mutation.as_mut();
-    let core = Core::empty()
-        .bind(mutation.as_vertex_storage())
-        .bind(mutation.as_edge_storage())
-        .bind(mutation.as_face_storage());
-    // Iterate over the set of vertices shared between the face and all of
-    // its neighbors. These are potential singularities.
-    for vertex in mutuals {
-        // Circulate (in order) over the neighboring faces of the potential
-        // singularity, ignoring the face to be removed.  Count the number
-        // of gaps, where neighboring faces do not share any edges. Because
-        // a face is being ignored, exactly one gap is expected. If any
-        // additional gaps exist, then removal will create a singularity.
-        let vertex = VertexView::from_keyed_source((vertex, &core))
-            .ok_or_else(|| GraphError::TopologyNotFound)?;
-        let n = vertex
-            .reachable_neighboring_faces()
-            .filter(|face| face.key() != abc)
-            .perimeter()
-            .filter(|&(previous, next)| {
-                let exterior = previous
-                    .reachable_interior_edges()
-                    .flat_map(|edge| edge.into_reachable_opposite_edge())
-                    .map(|edge| edge.key())
-                    .collect::<HashSet<_>>();
-                let interior = next
-                    .reachable_interior_edges()
-                    .map(|edge| edge.key())
-                    .collect::<HashSet<_>>();
-                exterior.intersection(&interior).count() == 0
-            })
-            .count();
-        if n > 1 {
-            return Err(GraphError::TopologyConflict);
-        }
-    }
-    mutation.disconnect_face_interior(&edges)?;
+    let FaceRemoveCache { abc, edges, .. } = cache;
+    mutation.as_mut().disconnect_face_interior(&edges)?;
     let face = mutation
+        .as_mut()
         .storage
         .remove(&abc)
         .ok_or_else(|| GraphError::TopologyNotFound)?;

src/graph/mutation/region.rs

@@ -149,12 +149,16 @@ where
             })
             .collect::<HashMap<_, _>>();
         // If only one vertex has any outgoing edges, then this face shares
-        // exactly one vertex with other faces and is therefore non-manifold.
+        // exactly one vertex with other faces. The vertex is a singularity and
+        // forms a pinwheel.
         //
-        // This kind of non-manifold is not supported, but sometimes occurs
-        // during a batch mutation. Details of the singularity vertex are
-        // emitted and handled by calling code, either raising an error or
-        // waiting to validate after a batch mutation is complete.
+        // TODO: Half-edge graphs can support pinwheels and this topology is
+        //       common. If possible, remove this detection mechanism. If this
+        //       data is used to reject pinwheel formations, remove that as
+        //       well.
+        //
+        //       Note that this data is still used when `FaceMutation`s are
+        //       committed to detect and reject unreachable faces.
         let singularity = {
             let mut outgoing = outgoing.iter().filter(|&(_, edges)| !edges.is_empty());
             if let Some((&vertex, _)) = outgoing.next() {

src/graph/view/face.rs

@@ -1,7 +1,6 @@
 use arrayvec::ArrayVec;
 use fool::prelude::*;
 use std::cmp;
-use std::collections::HashSet;
 use std::marker::PhantomData;
 use std::mem;
 use std::ops::{Add, Deref, DerefMut, Mul};
@@ -245,21 +244,6 @@ where
     M::Target: AsStorage<Edge<G>> + AsStorage<Face<G>> + AsStorage<Vertex<G>>,
     G: Geometry,
 {
-    pub(in crate::graph) fn reachable_mutuals(&self) -> HashSet<VertexKey> {
-        self.reachable_neighboring_faces()
-            .map(|face| {
-                face.reachable_vertices()
-                    .map(|vertex| vertex.key())
-                    .collect::<HashSet<_>>()
-            })
-            .fold(
-                self.reachable_vertices()
-                    .map(|vertex| vertex.key())
-                    .collect::<HashSet<_>>(),
-                |intersection, vertices| intersection.intersection(&vertices).cloned().collect(),
-            )
-    }
-
     pub(in crate::graph) fn reachable_vertices(
         &self,
     ) -> impl Iterator<Item = VertexView<&M::Target, G>> {