From 322fb81a9d9cb43392c59de8c6cb6dd1291f19df Mon Sep 17 00:00:00 2001
From: Dustin Carlino <dabreegster@gmail.com>
Date: Sat, 16 Jan 2021 10:24:03 -0800
Subject: [PATCH] Add more (still disabled) heuristics for automatically
 merging short roads. Look for roads between dual carriageways. #457

---
 map_model/src/make/merge_intersections.rs | 105 +++++++++++++++++++---
 1 file changed, 92 insertions(+), 13 deletions(-)

diff --git a/map_model/src/make/merge_intersections.rs b/map_model/src/make/merge_intersections.rs
index 80e5330554..e91591a91c 100644
--- a/map_model/src/make/merge_intersections.rs
+++ b/map_model/src/make/merge_intersections.rs
@@ -1,13 +1,10 @@
 use std::collections::{BTreeSet, VecDeque};
 
-use geom::Distance;
+use geom::{Angle, Distance};
 
+use crate::osm;
 use crate::osm::NodeID;
-use crate::raw::{OriginalRoad, RawMap};
-
-// Manually adjust this to try locally. Need to work through issues with merging before enabling
-// generally.
-const SHORT_ROAD_THRESHOLD: Distance = Distance::const_meters(0.0);
+use crate::raw::{OriginalRoad, RawMap, RawRoad};
 
 /// Merge tiny "roads" that're actually just part of a complicated intersection. Returns all
 /// surviving intersections adjacent to one of these merged roads.
@@ -17,6 +14,11 @@ pub fn merge_short_roads(map: &mut RawMap) -> BTreeSet<NodeID> {
     let mut queue: VecDeque<OriginalRoad> = VecDeque::new();
     for r in map.roads.keys() {
         queue.push_back(*r);
+
+        // TODO Remove after improving this heuristic.
+        if connects_dual_carriageway(map, r) {
+            info!("{} connects dual carriageways", r);
+        }
     }
 
     while !queue.is_empty() {
@@ -27,13 +29,7 @@ pub fn merge_short_roads(map: &mut RawMap) -> BTreeSet<NodeID> {
             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)
-        {
+        if should_merge(map, &id) {
             match map.merge_short_road(id) {
                 Ok((i, _, _, new_roads)) => {
                     merged.insert(i);
@@ -48,3 +44,86 @@ pub fn merge_short_roads(map: &mut RawMap) -> BTreeSet<NodeID> {
 
     merged
 }
+
+fn should_merge(map: &RawMap, id: &OriginalRoad) -> bool {
+    // See https://wiki.openstreetmap.org/wiki/Proposed_features/junction%3Dintersection
+    if map.roads[id].osm_tags.is("junction", "intersection") {
+        return true;
+    }
+
+    let road_length = if let Some(pl) = map.trimmed_road_geometry(*id) {
+        pl.length()
+    } else {
+        // The road or something near it collapsed down into a single point or something. This can
+        // happen while merging several short roads around a single junction.
+        return false;
+    };
+
+    // Any road anywhere shorter than this should get merged.
+    // TODO Disabled by default, because map.merge_short_road still has bugs in some cases.
+    if road_length < Distance::meters(0.0) {
+        return true;
+    }
+
+    // Roads connecting dual carriageways can use a longer threshold for merging.
+    if connects_dual_carriageway(map, id) && road_length < Distance::meters(0.0) {
+        return true;
+    }
+
+    false
+}
+
+// Does this road go between two divided one-ways? Ideally they're tagged explicitly
+// (https://wiki.openstreetmap.org/wiki/Tag:dual_carriageway%3Dyes), but we can also apply simple
+// heuristics to guess this.
+fn connects_dual_carriageway(map: &RawMap, id: &OriginalRoad) -> bool {
+    let connectors_angle = angle(&map.roads[id]);
+    // There are false positives like https://www.openstreetmap.org/way/4636259 when we're looking
+    // at a segment along a marked dual carriageway. Filter out by requiring the intersecting dual
+    // carriageways to differ by a minimum angle.
+    let within_degrees = 10.0;
+
+    let mut i1_dual_carriageway = false;
+    let mut oneway_names_i1: BTreeSet<String> = BTreeSet::new();
+    for r in map.roads_per_intersection(id.i1) {
+        let road = &map.roads[&r];
+        if r == *id || connectors_angle.approx_eq(angle(road), within_degrees) {
+            continue;
+        }
+        if road.osm_tags.is("dual_carriageway", "yes") {
+            i1_dual_carriageway = true;
+        }
+        if road.osm_tags.is("oneway", "yes") {
+            if let Some(name) = road.osm_tags.get(osm::NAME) {
+                oneway_names_i1.insert(name.to_string());
+            }
+        }
+    }
+
+    let mut i2_dual_carriageway = false;
+    let mut oneway_names_i2: BTreeSet<String> = BTreeSet::new();
+    for r in map.roads_per_intersection(id.i2) {
+        let road = &map.roads[&r];
+        if r == *id || connectors_angle.approx_eq(angle(road), within_degrees) {
+            continue;
+        }
+        if road.osm_tags.is("dual_carriageway", "yes") {
+            i2_dual_carriageway = true;
+        }
+        if road.osm_tags.is("oneway", "yes") {
+            if let Some(name) = road.osm_tags.get(osm::NAME) {
+                oneway_names_i2.insert(name.to_string());
+            }
+        }
+    }
+
+    (i1_dual_carriageway && i2_dual_carriageway)
+        || oneway_names_i1
+            .intersection(&oneway_names_i2)
+            .next()
+            .is_some()
+}
+
+fn angle(r: &RawRoad) -> Angle {
+    r.center_points[0].angle_to(*r.center_points.last().unwrap())
+}