From 3cdbe74948cc2a8c9c433457dc44616fa469d844 Mon Sep 17 00:00:00 2001
From: Dustin Carlino <dabreegster@gmail.com>
Date: Fri, 15 Jan 2021 20:52:11 -0800
Subject: [PATCH] Use a queue to merge short roads, instead of a potentially
 quadratic scan. #114

---
 map_model/src/make/merge_intersections.rs | 57 ++++++++++++-----------
 1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/map_model/src/make/merge_intersections.rs b/map_model/src/make/merge_intersections.rs
index 022c3d87a4..80e5330554 100644
--- a/map_model/src/make/merge_intersections.rs
+++ b/map_model/src/make/merge_intersections.rs
@@ -1,9 +1,9 @@
-use std::collections::BTreeSet;
+use std::collections::{BTreeSet, VecDeque};
 
 use geom::Distance;
 
 use crate::osm::NodeID;
-use crate::raw::RawMap;
+use crate::raw::{OriginalRoad, RawMap};
 
 // Manually adjust this to try locally. Need to work through issues with merging before enabling
 // generally.
@@ -14,35 +14,36 @@ const SHORT_ROAD_THRESHOLD: Distance = Distance::const_meters(0.0);
 pub fn merge_short_roads(map: &mut RawMap) -> BTreeSet<NodeID> {
     let mut merged = BTreeSet::new();
 
-    // An expensive fixed-point approach. When we merge one road, the IDs of some other roads might
-    // change, so it's simplest just to start over.
-    // TODO But since merge_short_road tells us what road IDs were deleted and created, it wouldn't
-    // be hard to make a single pass.
-    loop {
-        let mut changes = false;
-        for (id, road) in map.roads.clone() {
-            // See https://wiki.openstreetmap.org/wiki/Proposed_features/junction%3Dintersection
-            if road.osm_tags.is("junction", "intersection")
-                || map
-                    .trimmed_road_geometry(id)
-                    .map(|pl| pl.length() < SHORT_ROAD_THRESHOLD)
-                    .unwrap_or(false)
-            {
-                match map.merge_short_road(id) {
-                    Ok((i, _, _, _)) => {
-                        merged.insert(i);
-                        changes = true;
-                        break;
-                    }
-                    Err(err) => {
-                        warn!("Not merging short road / junction=intersection: {}", err);
-                    }
+    let mut queue: VecDeque<OriginalRoad> = VecDeque::new();
+    for r in map.roads.keys() {
+        queue.push_back(*r);
+    }
+
+    while !queue.is_empty() {
+        let id = queue.pop_front().unwrap();
+
+        // The road might've been deleted
+        if !map.roads.contains_key(&id) {
+            continue;
+        }
+
+        // See https://wiki.openstreetmap.org/wiki/Proposed_features/junction%3Dintersection
+        if map.roads[&id].osm_tags.is("junction", "intersection")
+            || map
+                .trimmed_road_geometry(id)
+                .map(|pl| pl.length() < SHORT_ROAD_THRESHOLD)
+                .unwrap_or(false)
+        {
+            match map.merge_short_road(id) {
+                Ok((i, _, _, new_roads)) => {
+                    merged.insert(i);
+                    queue.extend(new_roads);
+                }
+                Err(err) => {
+                    warn!("Not merging short road / junction=intersection: {}", err);
                 }
             }
         }
-        if !changes {
-            break;
-        }
     }
 
     merged