Rewrite clip_polyline_to_ring and be more careful about which parts o…

…f the PolyLine are in-bounds. #127

streets_reader/src/extract.rs

@@ -11,6 +11,8 @@ use crate::MapConfig;
 
 pub struct OsmExtract {
     /// Unsplit roads. These aren't Roads yet, because they may not obey those invariants.
+    /// Note there may be multiple entries here with the same WayID. Effectively those have been
+    /// partly pre-split.
     pub roads: Vec<(WayID, Vec<Pt2D>, Tags)>,
     /// Traffic signals to the direction they apply
     pub traffic_signals: HashMap<HashablePt2D, Direction>,

streets_reader/src/lib.rs

@@ -86,6 +86,14 @@ fn extract_osm(
         timer.next();
         out.handle_way(*id, way, &streets.config);
     }
+    timer.start_iter(
+        "processing OSM ways split into pieces",
+        doc.clipped_copied_ways.len(),
+    );
+    for (id, way) in &doc.clipped_copied_ways {
+        timer.next();
+        out.handle_way(*id, way, &streets.config);
+    }
 
     timer.start_iter("processing OSM relations", doc.relations.len());
     for (id, rel) in &doc.relations {

streets_reader/src/osm_reader/clip.rs

@@ -20,48 +20,70 @@ impl Document {
 
         // For line-string ways (not areas), clip them to the boundary. way.pts and way.nodes
         // become out-of-sync.
-        for way in self.ways.values_mut() {
+        for (id, way) in &mut self.ways {
             // TODO This could just be a cul-de-sac road
             if way.pts[0] == *way.pts.last().unwrap() {
                 continue;
             }
 
-            let pl = PolyLine::unchecked_new(way.pts.clone());
-            way.pts = clip_polyline_to_ring(pl, boundary_polygon).into_points();
+            let mut polylines =
+                clip_polyline_to_ring(PolyLine::unchecked_new(way.pts.clone()), boundary_polygon);
+            // Usually there's just one result
+            if polylines.len() == 1 {
+                way.pts = polylines.pop().unwrap().into_points();
+                continue;
+            }
+
+            // But occasionally a road crossing the boundary multiple times will get split into
+            // multiple pieces. In that case, make copies of the way, each with their own geometry.
+            for pl in polylines {
+                let mut copy = way.clone();
+                copy.pts = pl.into_points();
+                self.clipped_copied_ways.push((*id, copy));
+            }
+        }
+
+        // Remove the "original" from ways that were split into pieces
+        for (id, _) in &self.clipped_copied_ways {
+            self.ways.remove(id);
         }
 
         // TODO Handle ways that're areas
         // TODO Handle relations
     }
 }
 
+/// Split a polyline into potentially multiple pieces by clipping it against a polygon boundary.
+/// Only return slices within the polygon.
 // TODO Move to geom and test better
-// If this fails for any reason, just return the input untransformed
-fn clip_polyline_to_ring(pl: PolyLine, polygon: &Polygon) -> PolyLine {
+fn clip_polyline_to_ring(pl: PolyLine, polygon: &Polygon) -> Vec<PolyLine> {
     let mut hit_distances = Vec::new();
     for pt in polygon.get_outer_ring().all_intersections(&pl) {
         if let Some((dist, _)) = pl.dist_along_of_point(pt) {
             hit_distances.push(dist);
         } else {
-            return pl;
+            // This shouldn't happen, but just return the input untransformed if it does
+            return vec![pl];
         }
     }
+    hit_distances.sort();
 
-    if hit_distances.len() == 1 {
-        // Does it start or end inside the ring?
-        if polygon.contains_pt(pl.first_pt()) {
-            return pl.exact_slice(Distance::ZERO, hit_distances[0]);
-        } else {
-            return pl.exact_slice(hit_distances[0], pl.length());
-        }
-    }
+    // Split the PolyLine into pieces, every time it crosses the polygon
+    let mut start = Distance::ZERO;
 
-    if hit_distances.len() == 2 {
-        hit_distances.sort();
-        if let Ok(slice) = pl.maybe_exact_slice(hit_distances[0], hit_distances[1]) {
-            return slice;
+    let mut slices = Vec::new();
+    for dist in hit_distances {
+        // The slice may be tiny; skip if so
+        if let Ok(slice) = pl.maybe_exact_slice(start, dist) {
+            slices.push(slice);
         }
+        start = dist;
     }
+    // And the last piece
+    slices.extend(pl.maybe_exact_slice(start, pl.length()));
+
+    // Only keep slices in bounds
+    slices.retain(|pl| polygon.contains_pt(pl.middle()));
 
-    pl
+    slices
 }

streets_reader/src/osm_reader/mod.rs

@@ -16,13 +16,17 @@ pub struct Document {
     pub nodes: BTreeMap<NodeID, Node>,
     pub ways: BTreeMap<WayID, Way>,
     pub relations: BTreeMap<RelationID, Relation>,
+
+    /// These ways share a WayID, but each have different pts
+    pub clipped_copied_ways: Vec<(WayID, Way)>,
 }
 
 pub struct Node {
     pub pt: Pt2D,
     pub tags: Tags,
 }
 
+#[derive(Clone)]
 pub struct Way {
     // Duplicates geometry, because it's convenient
     pub nodes: Vec<NodeID>,

streets_reader/src/osm_reader/reader.rs

@@ -32,6 +32,7 @@ impl Document {
             nodes: BTreeMap::new(),
             ways: BTreeMap::new(),
             relations: BTreeMap::new(),
+            clipped_copied_ways: Vec::new(),
         };
 
         // We use the lower-level xmlparser instead of roxmltree to reduce peak memory usage in

streets_reader/src/split_ways.rs

@@ -38,6 +38,8 @@ pub fn split_up_roads(
                         osm_ids.push(*node_id);
                     }
 
+                    // TODO If there happens to be an OSM node defined RIGHT where a boundary is
+                    // drawn, we might not detect it as a MapEdge?
                     let kind = if osm_ids.is_empty() {
                         IntersectionKind::MapEdge
                     } else {