when gluing together partial multipolygons, trace along the border when possible. improves a few cases.

convert_osm/src/osm_reader.rs

@@ -1,5 +1,5 @@
 use abstutil::{FileWithProgress, Timer};
-use geom::{GPSBounds, HashablePt2D, LonLat, Polygon, Pt2D};
+use geom::{GPSBounds, HashablePt2D, LonLat, PolyLine, Polygon, Pt2D, Ring};
 use map_model::raw::{
     OriginalRoad, RawArea, RawBuilding, RawMap, RawRoad, RestrictionType, StableBuildingID,
     StableIntersectionID,
@@ -158,6 +158,8 @@ pub fn extract_osm(
         }
     }
 
+    let boundary = Ring::new(map.boundary_polygon.points().clone());
+
     let mut turn_restrictions = Vec::new();
     timer.start_iter("processing OSM relations", doc.relations.len());
     for rel in doc.relations.values() {
@@ -190,7 +192,7 @@ pub fn extract_osm(
                     }
                 }
                 if ok {
-                    let polygons = glue_multipolygon(pts_per_way);
+                    let polygons = glue_multipolygon(pts_per_way, &boundary);
                     if polygons.is_empty() {
                         println!("Relation {} failed to glue multipolygon", rel.id);
                     } else {
@@ -322,7 +324,7 @@ fn get_area_type(tags: &BTreeMap<String, String>) -> Option<AreaType> {
 }
 
 // The result could be more than one disjoint polygon.
-fn glue_multipolygon(mut pts_per_way: Vec<Vec<Pt2D>>) -> Vec<Polygon> {
+fn glue_multipolygon(mut pts_per_way: Vec<Vec<Pt2D>>, boundary: &Ring) -> Vec<Polygon> {
     // First deal with all of the closed loops.
     let mut polygons: Vec<Polygon> = Vec::new();
     pts_per_way.retain(|pts| {
@@ -364,12 +366,35 @@ fn glue_multipolygon(mut pts_per_way: Vec<Vec<Pt2D>>) -> Vec<Polygon> {
         }
     }
 
-    // Some ways of the multipolygon are clipped out. Connect the ends in the most straightforward
-    // way. Later polygon clipping will trim to the boundary.
-    if result[0] != *result.last().unwrap() {
-        result.push(result[0]);
+    if result[0] == *result.last().unwrap() {
+        polygons.push(Polygon::new(&result));
+        return polygons;
     }
-    polygons.push(Polygon::new(&result));
+
+    // Some ways of the multipolygon must be clipped out. First try to trace along the boundary.
+    let result_pl = PolyLine::new(result);
+    let hits = boundary.all_intersections(&result_pl);
+    if hits.len() != 2 {
+        // Give up and just connect the ends directly.
+        let mut pts = result_pl.points().clone();
+        pts.push(pts[0]);
+        polygons.push(Polygon::new(&pts));
+        return polygons;
+    }
+    let trimmed_result = result_pl.trim_to_endpts(hits[0], hits[1]);
+    let boundary_glue = boundary.get_shorter_slice_btwn(hits[0], hits[1]);
+
+    let mut trimmed_pts = trimmed_result.points().clone();
+    if trimmed_result.last_pt() == boundary_glue.first_pt() {
+        trimmed_pts.pop();
+        trimmed_pts.extend(boundary_glue.points().clone());
+    } else {
+        assert_eq!(trimmed_result.last_pt(), boundary_glue.last_pt());
+        trimmed_pts.pop();
+        trimmed_pts.extend(boundary_glue.reversed().points().clone());
+    }
+    assert_eq!(trimmed_pts[0], *trimmed_pts.last().unwrap());
+    polygons.push(Polygon::new(&trimmed_pts));
     polygons
 }
 

data/screenshots/montlake/MANIFEST

@@ -1,14 +1,14 @@
 86a9b0e7d962c8018cce79780b887548  ../data/screenshots/pending_montlake/01x01_i52.png
-39b7b6cd6fdc808905185df80648aeac  ../data/screenshots/pending_montlake/02x01_i1.png
-8031bcc147e871442f61bf2dcffd1bef  ../data/screenshots/pending_montlake/03x01_i0.png
+5c0ccf2525f2a58b65a14575370b4bd1  ../data/screenshots/pending_montlake/02x01_i1.png
+6b746afdd82b17e934c3dc575733cd9f  ../data/screenshots/pending_montlake/03x01_i0.png
 3b89724ee90dbc27ed63b433134d462b  ../data/screenshots/pending_montlake/01x02_i7.png
 037de8352136becc9c8d6fc3c055441c  ../data/screenshots/pending_montlake/02x02_i125.png
-3639aa262e615ec329ef5d3ad66dfac1  ../data/screenshots/pending_montlake/03x02_i30.png
+d3b91fcd132cbe12ebc8ffea22263f2f  ../data/screenshots/pending_montlake/03x02_i30.png
 8544a73fb09878e5cf50cd68ae101b4c  ../data/screenshots/pending_montlake/01x03_i96.png
 28d361824615d3493f7eb68566bbd415  ../data/screenshots/pending_montlake/02x03_i5.png
-8dcf86c432fec6b21c40b7a00e644ff7  ../data/screenshots/pending_montlake/03x03_i13.png
+c6cb6bcb9898d0d2e13616413cd886db  ../data/screenshots/pending_montlake/03x03_i13.png
 4b213980a9062d8b643030fb3ed7c2de  ../data/screenshots/pending_montlake/01x04_i3.png
-130d1c9b47886deb1131e2aee17bfd63  ../data/screenshots/pending_montlake/02x04_i26.png
+9bb9c707c6f0b7cb4ddc8a6900a3e245  ../data/screenshots/pending_montlake/02x04_i26.png
 b1215a9922c8c61f2b7704fef603e7ae  ../data/screenshots/pending_montlake/03x04_i12.png
 64067453731a63ccad74e0074ade57de  ../data/screenshots/pending_montlake/01x05_i41.png
 663179f6274a68124fe348d3d469be27  ../data/screenshots/pending_montlake/02x05_i40.png

docs/articles/map/article.md

@@ -107,8 +107,9 @@ takes a few seconds to load a serialized map.
   areas
   - Areas usually come from a relation of multiple ways, with the points out of
     order. Gluing all the points together fails when the .osm has some ways
-    clipped out. In that case, just use a straight line to try to close off the
-    polygon.
+    clipped out. In that case, try to trace along the map boundary if the
+    partial area intersects the boundary in a clear way. Otherwise, just use a
+    straight line to try to close off the polygon.
   - Also read traffic signal locations and turn restrictions between OSM ways
 - `split_ways.rs`: Split OSM ways into road segments
   - OSM ways cross many intersections, so treat points with multiple ways and

geom/src/polyline.rs

@@ -68,13 +68,12 @@ impl PolyLine {
         Some(result)
     }
 
-    pub(crate) fn make_polygons_for_boundary(pts: Vec<Pt2D>, thickness: Distance) -> Polygon {
-        // Points WILL repeat -- fast-path some stuff.
-        let pl = PolyLine {
-            pts,
-            length: Distance::ZERO,
-        };
-        pl.make_polygons(thickness)
+    // Only to be called by Ring.
+    pub(crate) fn new_for_ring(pts: Vec<Pt2D>) -> PolyLine {
+        let length = pts.windows(2).fold(Distance::ZERO, |so_far, pair| {
+            so_far + pair[0].dist_to(pair[1])
+        });
+        PolyLine { pts, length }
     }
 
     pub fn to_thick_boundary(
@@ -636,6 +635,16 @@ impl PolyLine {
         None
     }
 
+    pub fn trim_to_endpts(&self, pt1: Pt2D, pt2: Pt2D) -> PolyLine {
+        assert!(pt1 != pt2);
+        let mut dist1 = self.dist_along_of_point(pt1).unwrap().0;
+        let mut dist2 = self.dist_along_of_point(pt2).unwrap().0;
+        if dist1 > dist2 {
+            std::mem::swap(&mut dist1, &mut dist2);
+        }
+        self.exact_slice(dist1, dist2)
+    }
+
     pub fn get_bounds(&self) -> Bounds {
         Bounds::from(&self.pts)
     }

geom/src/ring.rs

@@ -1,4 +1,4 @@
-use crate::{Distance, PolyLine, Polygon, Pt2D};
+use crate::{Distance, Line, PolyLine, Polygon, Pt2D};
 use serde_derive::{Deserialize, Serialize};
 use std::collections::HashSet;
 use std::fmt;
@@ -37,9 +37,41 @@ impl Ring {
     }
 
     pub fn make_polygons(&self, thickness: Distance) -> Polygon {
-        // TODO Has a weird corner. Use the polygon offset thing instead? And move the
-        // implementation here, ideally.
-        PolyLine::make_polygons_for_boundary(self.pts.clone(), thickness)
+        // TODO Has a weird corner. Use the polygon offset thing instead?
+        PolyLine::new_for_ring(self.pts.clone()).make_polygons(thickness)
+    }
+
+    pub fn all_intersections(&self, other: &PolyLine) -> Vec<Pt2D> {
+        let mut hits = Vec::new();
+        for l1 in self.pts.windows(2).map(|pair| Line::new(pair[0], pair[1])) {
+            for l2 in other.lines() {
+                if let Some(pt) = l1.intersection(&l2) {
+                    hits.push(pt);
+                }
+            }
+        }
+        hits
+    }
+
+    pub fn get_shorter_slice_btwn(&self, pt1: Pt2D, pt2: Pt2D) -> PolyLine {
+        assert!(pt1 != pt2);
+        let pl = PolyLine::new_for_ring(self.pts.clone());
+
+        let mut dist1 = pl.dist_along_of_point(pt1).unwrap().0;
+        let mut dist2 = pl.dist_along_of_point(pt2).unwrap().0;
+        if dist1 > dist2 {
+            std::mem::swap(&mut dist1, &mut dist2);
+        }
+
+        let candidate1 = pl.exact_slice(dist1, dist2);
+        let candidate2 = pl
+            .exact_slice(dist2, pl.length())
+            .extend(pl.exact_slice(Distance::ZERO, dist1));
+        if candidate1.length() < candidate2.length() {
+            candidate1
+        } else {
+            candidate2
+        }
     }
 }