From 7de93eaeb1798a763b16aaca7f031ae9fc0d23d0 Mon Sep 17 00:00:00 2001 From: Sean Olson Date: Thu, 24 Jan 2019 12:04:04 -0800 Subject: [PATCH] Allow pinwheels and singularities in graphs. --- src/graph/mesh.rs | 62 +----------------------------------- src/graph/mutation/face.rs | 61 +++++++---------------------------- src/graph/mutation/region.rs | 14 +++++--- src/graph/view/face.rs | 16 ---------- 4 files changed, 21 insertions(+), 132 deletions(-) diff --git a/src/graph/mesh.rs b/src/graph/mesh.rs index 01bc9655..656cca70 100644 --- a/src/graph/mesh.rs +++ b/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::>::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::>::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::>(); - let key = graph - .faces() - .find(|face| { - face.vertices() - .map(|vertex| vertex.geometry.clone()) - .map(|position| (position.x, position.y)) - .collect::>() - == *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] diff --git a/src/graph/mutation/face.rs b/src/graph/mutation/face.rs index 1b37e7c5..2b2fc4cc 100644 --- a/src/graph/mutation/face.rs +++ b/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> + AsStorage> + AsStorage>, { - // 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, edges: Vec, phantom: PhantomData, } @@ -308,6 +308,7 @@ impl FaceRemoveCache where G: Geometry, { + // TODO: Should this require consistency? pub fn snapshot(storage: M, abc: FaceKey) -> Result 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( mut mutation: N, cache: FaceRemoveCache, @@ -523,50 +524,10 @@ where M: Consistent + From> + Into>, 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::>(); - let interior = next - .reachable_interior_edges() - .map(|edge| edge.key()) - .collect::>(); - 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)?; diff --git a/src/graph/mutation/region.rs b/src/graph/mutation/region.rs index 4cd96e2b..e17b3241 100644 --- a/src/graph/mutation/region.rs +++ b/src/graph/mutation/region.rs @@ -149,12 +149,16 @@ where }) .collect::>(); // 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() { diff --git a/src/graph/view/face.rs b/src/graph/view/face.rs index 33481aab..a59a2c00 100644 --- a/src/graph/view/face.rs +++ b/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> + AsStorage> + AsStorage>, G: Geometry, { - pub(in crate::graph) fn reachable_mutuals(&self) -> HashSet { - self.reachable_neighboring_faces() - .map(|face| { - face.reachable_vertices() - .map(|vertex| vertex.key()) - .collect::>() - }) - .fold( - self.reachable_vertices() - .map(|vertex| vertex.key()) - .collect::>(), - |intersection, vertices| intersection.intersection(&vertices).cloned().collect(), - ) - } - pub(in crate::graph) fn reachable_vertices( &self, ) -> impl Iterator> {