Reduce the number of points along curvy roads. When they're tagged in OSM too close together, it explodes PolyLine shifting. #833

convert_osm/src/split_ways.rs

@@ -119,15 +119,15 @@ pub fn split_up_roads(map: &mut RawMap, mut input: OsmExtract, timer: &mut Timer
                     i1,
                     i2: *i2,
                 };
-                // Note we populate this before dedupe_angles, so even if some points are removed,
-                // we can still associate them to the road.
+                // Note we populate this before simplify_linestring, 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::take(&mut pts));
+                r.center_points = simplify_linestring(std::mem::take(&mut pts));
                 // Start a new road
                 map.roads.insert(id, r.clone());
                 r.osm_tags.remove(osm::ENDPT_FWD);
@@ -233,8 +233,12 @@ pub fn split_up_roads(map: &mut RawMap, mut input: OsmExtract, timer: &mut Timer
     }
 }
 
-// TODO Consider doing this in PolyLine::new always. extend() there does this too.
-fn dedupe_angles(pts: Vec<Pt2D>) -> Vec<Pt2D> {
+// TODO Consider doing this in PolyLine::new always. extend() there also attempts the angle
+// deduping.
+fn simplify_linestring(pts: Vec<Pt2D>) -> Vec<Pt2D> {
+    // Remove interior points that have nearly the same angle as the previous line segment
+    //
+    // TODO Possibly the RDP simplification below would handle this (and way more robustly)
     let mut result: Vec<Pt2D> = Vec::new();
     for pt in pts {
         let l = result.len();
@@ -247,7 +251,17 @@ fn dedupe_angles(pts: Vec<Pt2D>) -> Vec<Pt2D> {
         }
         result.push(pt);
     }
-    result
+
+    // Also reduce the number of points along curves. They're wasteful, and when they're too close
+    // together, actually break PolyLine shifting:
+    // https://github.com/a-b-street/abstreet/issues/833
+    //
+    // The epsilon is in units of meters; points closer than this will get simplified. 0.1 is too
+    // loose -- a curve with too many points was still broken, but 1.0 was too aggressive -- curves
+    // got noticeably flattened. At 0.5, some intersetion polygons get a bit worse, but only in
+    // places where they were already pretty broken.
+    let epsilon = 0.5;
+    Pt2D::simplify_rdp(result, epsilon)
 }
 
 /// Many "roundabouts" like https://www.openstreetmap.org/way/427144965 are so tiny that they wind

geom/src/conversions.rs

@@ -0,0 +1,14 @@
+//! Conversions between this crate and `geo`. Long-term, we should think about directly using `geo`
+//! or wrapping it, but in the meantime...
+//!
+//! TODO Also, there's no consistency between standalone methods like this and From/Into impls.
+
+use crate::Pt2D;
+
+pub fn pts_to_line_string(raw_pts: &[Pt2D]) -> geo::LineString<f64> {
+    let pts: Vec<geo::Point<f64>> = raw_pts
+        .iter()
+        .map(|pt| geo::Point::new(pt.x(), pt.y()))
+        .collect();
+    pts.into()
+}

geom/src/find_closest.rs

@@ -5,6 +5,7 @@ use aabb_quadtree::QuadTree;
 use geo::algorithm::contains::Contains;
 use geo::prelude::{ClosestPoint, EuclideanDistance};
 
+use crate::conversions::pts_to_line_string;
 use crate::{Bounds, Distance, Pt2D};
 
 // TODO Maybe use https://crates.io/crates/spatial-join proximity maps
@@ -87,11 +88,3 @@ where
             .map(|(k, pt, _)| (k, pt))
     }
 }
-
-fn pts_to_line_string(raw_pts: &[Pt2D]) -> geo::LineString<f64> {
-    let pts: Vec<geo::Point<f64>> = raw_pts
-        .iter()
-        .map(|pt| geo::Point::new(pt.x(), pt.y()))
-        .collect();
-    pts.into()
-}

geom/src/lib.rs

@@ -25,6 +25,7 @@ pub use crate::time::Time;
 mod angle;
 mod bounds;
 mod circle;
+mod conversions;
 mod distance;
 mod duration;
 mod find_closest;

geom/src/pt.rs

@@ -1,8 +1,10 @@
 use std::fmt;
 
+use geo::algorithm::simplify::Simplify;
 use ordered_float::NotNan;
 use serde::{Deserialize, Serialize};
 
+use crate::conversions::pts_to_line_string;
 use crate::{
     deserialize_f64, serialize_f64, trim_f64, Angle, Distance, GPSBounds, LonLat, EPSILON_DIST,
 };
@@ -133,6 +135,16 @@ impl Pt2D {
             y_nan: NotNan::new(self.y()).unwrap(),
         }
     }
+
+    /// Simplifies a list of points using Ramer-Douglas-Peuckr
+    pub fn simplify_rdp(pts: Vec<Pt2D>, epsilon: f64) -> Vec<Pt2D> {
+        pts_to_line_string(&pts)
+            .simplify(&epsilon)
+            .into_points()
+            .into_iter()
+            .map(|pt| pt.into())
+            .collect()
+    }
 }
 
 impl fmt::Display for Pt2D {

tests/goldenfiles/blockfinding.txt

@@ -1,16 +1,16 @@
 data/system/us/seattle/maps/montlake.bin
-    158 single blocks (0 failures to blockify), 1 partial merges, 0 failures to blockify partitions
+    157 single blocks (0 failures to blockify), 1 partial merges, 0 failures to blockify partitions
 data/system/us/seattle/maps/downtown.bin
-    1449 single blocks (0 failures to blockify), 10 partial merges, 0 failures to blockify partitions
+    1443 single blocks (0 failures to blockify), 13 partial merges, 0 failures to blockify partitions
 data/system/us/seattle/maps/lakeslice.bin
-    1033 single blocks (2 failures to blockify), 5 partial merges, 1 failures to blockify partitions
+    1033 single blocks (1 failures to blockify), 4 partial merges, 0 failures to blockify partitions
 data/system/us/phoenix/maps/tempe.bin
-    407 single blocks (0 failures to blockify), 4 partial merges, 0 failures to blockify partitions
+    406 single blocks (2 failures to blockify), 4 partial merges, 0 failures to blockify partitions
 data/system/gb/leeds/maps/north.bin
-    2589 single blocks (4 failures to blockify), 18 partial merges, 0 failures to blockify partitions
+    2582 single blocks (5 failures to blockify), 20 partial merges, 2 failures to blockify partitions
 data/system/gb/bristol/maps/east.bin
-    1061 single blocks (1 failures to blockify), 7 partial merges, 1 failures to blockify partitions
+    1061 single blocks (3 failures to blockify), 7 partial merges, 1 failures to blockify partitions
 data/system/gb/london/maps/camden.bin
-    3519 single blocks (9 failures to blockify), 33 partial merges, 0 failures to blockify partitions
+    3519 single blocks (5 failures to blockify), 34 partial merges, 0 failures to blockify partitions
 data/system/gb/london/maps/southwark.bin
-    3487 single blocks (5 failures to blockify), 42 partial merges, 1 failures to blockify partitions
+    3477 single blocks (4 failures to blockify), 46 partial merges, 0 failures to blockify partitions