Handle IteratedNoder TopologyExceptions by falling back to SnappingNoder

Notgnoshi · Mar 2, 2024 · 8b4e507 · 8b4e507
1 parent fb3a91a
commit 8b4e507
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 4 deletions.
diff --git a/generative/cxxbridge/geometry_collection.rs b/generative/cxxbridge/geometry_collection.rs
@@ -19,6 +19,33 @@ impl GeometryCollectionShim {
     pub fn get_total_geoms(&self) -> usize {
         self.geoms.len()
     }
+    pub fn get_geo_points(&self) -> Vec<geo::Point> {
+        let mut points = Vec::new();
+        let mut num_points = 0;
+        for g in &self.geoms {
+            match g {
+                geo::Geometry::Point(_) => num_points += 1,
+                geo::Geometry::MultiPoint(m) => num_points += m.len(),
+                _ => {}
+            }
+        }
+        points.reserve_exact(num_points);
+
+        for g in self.geoms.iter() {
+            match g {
+                geo::Geometry::Point(p) => {
+                    points.push(*p);
+                }
+                geo::Geometry::MultiPoint(m) => {
+                    for p in m.0.iter() {
+                        points.push(*p);
+                    }
+                }
+                _ => {}
+            }
+        }
+        points
+    }
 
     pub fn get_points(&self) -> Vec<CoordShim> {
         let mut points = Vec::new();

diff --git a/generative/cxxbridge/noder.hpp b/generative/cxxbridge/noder.hpp
@@ -27,6 +27,11 @@ node(const GeometryCollectionShim& rust_geoms, double tolerance) noexcept
         auto noder = std::make_unique<geos::noding::snap::SnappingNoder>(tolerance);
         noded = generative::noding::GeometryNoder::node(*geos_geoms, std::move(noder));
     }
+    if (!noded)
+    {
+        return nullptr;
+    }
+
     auto graph = generative::noding::GeometryGraph(*noded);
 
     auto graph_shim = std::make_unique<GeometryGraphShim>(std::move(graph));

diff --git a/generative/noding/geometry-noder.cpp b/generative/noding/geometry-noder.cpp
@@ -132,7 +132,13 @@ GeometryNoder::node(const geos::geom::Geometry& geometry,
                     std::unique_ptr<geos::noding::Noder> noder)
 {
     GeometryNoder geom_noder(geometry, std::move(noder));
-    return geom_noder.get_noded();
+    try
+    {
+        return geom_noder.get_noded();
+    } catch (const std::exception&)
+    {
+        return nullptr;
+    }
 }
 void GeometryNoder::extract_segment_strings(const geos::geom::Geometry& geometry,
                                             geos::noding::SegmentString::NonConstVect& out)
@@ -146,7 +152,9 @@ geos::noding::Noder& GeometryNoder::get_noder()
     if (!m_noder)
     {
         const geos::geom::PrecisionModel* pm = m_geometry.getFactory()->getPrecisionModel();
-        m_noder = std::make_unique<geos::noding::IteratedNoder>(pm);
+        auto noder = std::make_unique<geos::noding::IteratedNoder>(pm);
+        noder->setMaximumIterations(10);
+        m_noder = std::move(noder);
     }
 
     return *m_noder;

diff --git a/generative/noding/mod.rs b/generative/noding/mod.rs
@@ -8,7 +8,7 @@ where
     G: IntoIterator<Item = Geometry>,
 {
     let collection = cxxbridge::GeometryCollectionShim::new(geoms);
-    let ffi_graph = unsafe {
+    let mut ffi_graph = unsafe {
         // Setting the tolerance to 0 picks the IteratedNoder instead of the SnappingNoder.
         // They both have pros and cons.
         // * IteratedNoder might throw exceptions if it does not converge on pathological
@@ -17,7 +17,29 @@ where
         let tolerance = 0.0;
         cxxbridge::node(&collection, tolerance)
     };
-    (&*ffi_graph).into()
+
+    // Retry with the SnappingNoder
+    let mut insert_isolated_points = false;
+    if ffi_graph.is_null() {
+        insert_isolated_points = true;
+        let tolerance = 0.000001;
+        log::error!("GEOS IteratedNoder failed. Falling back on SnappingNoder");
+        ffi_graph = unsafe { cxxbridge::node(&collection, tolerance) }
+    }
+
+    let mut graph: GeometryGraph<Direction> = (&*ffi_graph).into();
+
+    // The SnappingNoder throws away isolated points, so add them back in. Unfortunately, this
+    // doesn't calculate any node-segment intersections, and may result in duplicate nodes.
+    if insert_isolated_points {
+        log::warn!("Adding isolated points back in ... may result in duplicate nodes");
+        let points = collection.get_geo_points();
+        for point in points.into_iter() {
+            graph.add_node(point);
+        }
+    }
+
+    graph
 }
 
 pub fn polygonize<Direction: petgraph::EdgeType>(