From a9021348abded03cbedbdb77b382aafd52e6a37f Mon Sep 17 00:00:00 2001
From: Dustin Carlino <dabreegster@gmail.com>
Date: Mon, 26 Dec 2022 15:24:28 +0000
Subject: [PATCH] Check invariants after every transformation. Use this to
 flush out a case where we need to remove orphaned intersections immediately,
 and simplify the intersection geometry transform a bit. #136

---
 osm2streets/src/geometry/mod.rs               |  4 +---
 .../src/transform/collapse_intersections.rs   |  4 ++++
 .../src/transform/intersection_geometry.rs    | 24 +++++++------------
 osm2streets/src/transform/mod.rs              | 15 ++++++++++++
 tests/src/lib.rs                              |  7 +++---
 5 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/osm2streets/src/geometry/mod.rs b/osm2streets/src/geometry/mod.rs
index 13c80a47..89196aa4 100644
--- a/osm2streets/src/geometry/mod.rs
+++ b/osm2streets/src/geometry/mod.rs
@@ -109,9 +109,7 @@ pub fn intersection_polygon(
         roads.insert(r.id, r);
     }
 
-    if roads.is_empty() {
-        bail!("{intersection_id} has no roads");
-    }
+    assert!(!roads.is_empty());
 
     let results = Results {
         intersection_id,
diff --git a/osm2streets/src/transform/collapse_intersections.rs b/osm2streets/src/transform/collapse_intersections.rs
index e3e7ff57..73f5b399 100644
--- a/osm2streets/src/transform/collapse_intersections.rs
+++ b/osm2streets/src/transform/collapse_intersections.rs
@@ -119,4 +119,8 @@ pub fn trim_deadends(streets: &mut StreetNetwork) {
 
     // It's possible we need to do this in a fixed-point until there are no changes, but meh.
     // Results look good so far.
+
+    // We may have created orphaned intersections. Clean up here.
+    // TODO Anywhere calling remove_road potentially causes this too
+    streets.intersections.retain(|_, i| !i.roads.is_empty());
 }
diff --git a/osm2streets/src/transform/intersection_geometry.rs b/osm2streets/src/transform/intersection_geometry.rs
index 23cddb0f..70e70b75 100644
--- a/osm2streets/src/transform/intersection_geometry.rs
+++ b/osm2streets/src/transform/intersection_geometry.rs
@@ -9,7 +9,6 @@ pub fn generate(streets: &mut StreetNetwork, timer: &mut Timer) {
         streets.intersections.len(),
     );
     // It'd be nice to mutate in the loop, but the borrow checker won't let us
-    let mut remove_dangling_nodes = Vec::new();
     let mut set_polygons = Vec::new();
 
     // Set trim distances for all roads
@@ -36,19 +35,15 @@ pub fn generate(streets: &mut StreetNetwork, timer: &mut Timer) {
             Err(err) => {
                 error!("Can't make intersection geometry for {}: {}", i.id, err);
 
-                // If we haven't removed disconnected roads, we may have dangling nodes around.
-                if let Some(r) = i.roads.iter().next() {
-                    // Don't trim lines back at all
-                    let road = &streets.roads[r];
-                    let pt = if road.src_i == i.id {
-                        road.center_line.first_pt()
-                    } else {
-                        road.center_line.last_pt()
-                    };
-                    set_polygons.push((i.id, Circle::new(pt, Distance::meters(3.0)).to_polygon()));
+                let r = i.roads[0];
+                // Don't trim lines back at all
+                let road = &streets.roads[&r];
+                let pt = if road.src_i == i.id {
+                    road.center_line.first_pt()
                 } else {
-                    remove_dangling_nodes.push(i.id);
-                }
+                    road.center_line.last_pt()
+                };
+                set_polygons.push((i.id, Circle::new(pt, Distance::meters(3.0)).to_polygon()));
             }
         }
     }
@@ -70,7 +65,4 @@ pub fn generate(streets: &mut StreetNetwork, timer: &mut Timer) {
     for (i, polygon) in set_polygons {
         streets.intersections.get_mut(&i).unwrap().polygon = polygon;
     }
-    for i in remove_dangling_nodes {
-        streets.intersections.remove(&i).unwrap();
-    }
 }
diff --git a/osm2streets/src/transform/mod.rs b/osm2streets/src/transform/mod.rs
index 6e9824f8..3c9961b0 100644
--- a/osm2streets/src/transform/mod.rs
+++ b/osm2streets/src/transform/mod.rs
@@ -139,6 +139,21 @@ impl StreetNetwork {
         timer.stop("simplify StreetNetwork");
     }
 
+    /// Apply a sequence of transformations, but also check invariants after every step. More
+    /// expensive and may crash, but useful for testing.
+    pub fn apply_transformations_with_invariant_checks(
+        &mut self,
+        transformations: Vec<Transformation>,
+        timer: &mut Timer,
+    ) {
+        timer.start("simplify StreetNetwork");
+        for transformation in transformations {
+            transformation.apply(self, timer);
+            self.check_invariants();
+        }
+        timer.stop("simplify StreetNetwork");
+    }
+
     /// Apply a sequence of transformations, but also save a copy of the `StreetNetwork` before
     /// each step. Some steps may also internally add debugging info.
     pub fn apply_transformations_stepwise_debugging(
diff --git a/tests/src/lib.rs b/tests/src/lib.rs
index 18d92a9d..0d86d591 100644
--- a/tests/src/lib.rs
+++ b/tests/src/lib.rs
@@ -31,9 +31,10 @@ mod tests {
             &mut timer,
         )?;
         street_network.check_invariants();
-        street_network
-            .apply_transformations(Transformation::standard_for_clipped_areas(), &mut timer);
-        street_network.check_invariants();
+        street_network.apply_transformations_with_invariant_checks(
+            Transformation::standard_for_clipped_areas(),
+            &mut timer,
+        );
         street_network.save_to_geojson(format!("{path}/geometry.json"))?;
 
         let road_network: RoadNetwork = street_network.into();