From e7e8bb2ca0f6d2fba2cf0ff42b7d7bd407de99b5 Mon Sep 17 00:00:00 2001
From: Dustin Carlino <dabreegster@gmail.com>
Date: Mon, 25 Jan 2021 10:04:02 -0800
Subject: [PATCH] Fix a bug with matching traffic signals tagged along a way to
 the intersection. When the signal happened to be on a node that didn't alter
 the angle of the road, it was incorrectly getting dropped before.

Not regenerating all map data just yet, but manually verified along
Newmarket Rd in Cambridge. #449
---
 convert_osm/src/extract.rs    | 15 +++++++++------
 convert_osm/src/split_ways.rs | 26 +++++++++++---------------
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/convert_osm/src/extract.rs b/convert_osm/src/extract.rs
index 1ee1f1cd92..bcccca4bc8 100644
--- a/convert_osm/src/extract.rs
+++ b/convert_osm/src/extract.rs
@@ -7,7 +7,7 @@ use abstutil::{retain_btreemap, Tags, Timer};
 use geom::{HashablePt2D, Polygon, Pt2D, Ring};
 use kml::{ExtraShape, ExtraShapes};
 use map_model::raw::{RawArea, RawBuilding, RawMap, RawParkingLot, RawRoad, RestrictionType};
-use map_model::{osm, Amenity, AreaType, DrivingSide, NamePerLanguage};
+use map_model::{osm, Amenity, AreaType, Direction, DrivingSide, NamePerLanguage};
 
 use crate::osm_geom::{get_multipolygon_members, glue_multipolygon, multipoly_geometry};
 use crate::{transit, Options};
@@ -15,8 +15,8 @@ use crate::{transit, Options};
 pub struct OsmExtract {
     /// Unsplit roads
     pub roads: Vec<(WayID, RawRoad)>,
-    /// Traffic signals to the direction they apply (or just true if unspecified)
-    pub traffic_signals: HashMap<HashablePt2D, bool>,
+    /// Traffic signals to the direction they apply
+    pub traffic_signals: HashMap<HashablePt2D, Direction>,
     pub osm_node_ids: HashMap<HashablePt2D, NodeID>,
     /// (ID, restriction type, from way ID, via node ID, to way ID)
     pub simple_turn_restrictions: Vec<(RestrictionType, WayID, NodeID, WayID)>,
@@ -69,9 +69,12 @@ pub fn extract_osm(map: &mut RawMap, opts: &Options, timer: &mut Timer) -> OsmEx
         out.osm_node_ids.insert(node.pt.to_hashable(), *id);
 
         if node.tags.is(osm::HIGHWAY, "traffic_signals") {
-            let backwards = node.tags.is("traffic_signals:direction", "backward");
-            out.traffic_signals
-                .insert(node.pt.to_hashable(), !backwards);
+            let dir = if node.tags.is("traffic_signals:direction", "backward") {
+                Direction::Back
+            } else {
+                Direction::Fwd
+            };
+            out.traffic_signals.insert(node.pt.to_hashable(), dir);
         }
         for amenity in get_bldg_amenities(&node.tags) {
             out.amenities.push((node.pt, amenity));
diff --git a/convert_osm/src/split_ways.rs b/convert_osm/src/split_ways.rs
index 12b8d1c0cb..359caf751e 100644
--- a/convert_osm/src/split_ways.rs
+++ b/convert_osm/src/split_ways.rs
@@ -3,12 +3,12 @@ use std::collections::HashMap;
 use abstutil::{Counter, Timer};
 use geom::{Distance, HashablePt2D, Pt2D};
 use map_model::raw::{OriginalRoad, RawIntersection, RawMap};
-use map_model::{osm, Amenity, IntersectionType};
+use map_model::{osm, Amenity, Direction, IntersectionType};
 
 use crate::extract::OsmExtract;
 
-/// Returns amenities and a mapping of all points to split road. (Some internal points on roads are
-/// removed, so this mapping isn't redundant.)
+/// Returns amenities and a mapping of all points to split road. (Some internal points on roads get
+/// removed in this call, so this mapping isn't redundant.)
 pub fn split_up_roads(
     map: &mut RawMap,
     mut input: OsmExtract,
@@ -80,8 +80,12 @@ pub fn split_up_roads(
                     i1,
                     i2: *i2,
                 };
-                for pt in &pts {
-                    pt_to_road.insert(pt.to_hashable(), id);
+                // Note we populate this before dedupe_angles, so even if some points are removed,
+                // we can still associate them to the road.
+                for (idx, pt) in pts.iter().enumerate() {
+                    if idx != 0 && idx != pts.len() - 1 {
+                        pt_to_road.insert(pt.to_hashable(), id);
+                    }
                 }
 
                 r.center_points = dedupe_angles(std::mem::replace(&mut pts, Vec::new()));
@@ -170,19 +174,11 @@ pub fn split_up_roads(
     timer.start("match traffic signals to intersections");
     // Handle traffic signals tagged on incoming ways and not at intersections
     // (https://wiki.openstreetmap.org/wiki/Tag:highway=traffic%20signals?uselang=en#Tag_all_incoming_ways).
-    let mut pt_to_road: HashMap<HashablePt2D, OriginalRoad> = HashMap::new();
-    for (id, r) in &map.roads {
-        for (idx, pt) in r.center_points.iter().enumerate() {
-            if idx != 0 && idx != r.center_points.len() - 1 {
-                pt_to_road.insert(pt.to_hashable(), *id);
-            }
-        }
-    }
-    for (pt, forwards) in input.traffic_signals {
+    for (pt, dir) in input.traffic_signals {
         if let Some(r) = pt_to_road.get(&pt) {
             // Example: https://www.openstreetmap.org/node/26734224
             if !map.roads[r].osm_tags.is(osm::HIGHWAY, "construction") {
-                let i = if forwards { r.i2 } else { r.i1 };
+                let i = if dir == Direction::Fwd { r.i2 } else { r.i1 };
                 map.intersections.get_mut(&i).unwrap().intersection_type =
                     IntersectionType::TrafficSignal;
             }