Overhaul StreetNetwork road splitting + clipping. #127

Also has the side effect of getting rid of handling for tiny
roundabouts.

3 tests break, with clipped roads appearing on the wrong side of the
boundary.

osm2streets/src/road.rs

@@ -494,6 +494,10 @@ impl Road {
 
         Some(pl)
     }
+
+    pub fn from_osm_way(&self, way: osm::WayID) -> bool {
+        self.osm_ids.iter().any(|id| id.osm_way_id == way)
+    }
 }
 
 impl StreetNetwork {

streets_reader/src/lib.rs

@@ -13,13 +13,12 @@ pub use self::extract::OsmExtract;
 use osm_reader::Document;
 
 // TODO Clean up the public API of all of this
-mod clip;
 pub mod extract;
 pub mod osm_reader;
 pub mod split_ways;
 
 /// Create a `StreetNetwork` from the contents of an `.osm.xml` file. If `clip_pts` is specified,
-/// use theese as a boundary polygon. (Use `LonLat::read_osmosis_polygon` or similar to produce
+/// use theese as a boundary polygon. (Use `LonLat::read_geojson_polygon` or similar to produce
 /// these.)
 ///
 /// You probably want to do `StreetNetwork::apply_transformations` on the result to get a useful
@@ -50,16 +49,18 @@ fn extract_osm(
     clip_pts: Option<Vec<LonLat>>,
     timer: &mut Timer,
 ) -> Result<(OsmExtract, Document)> {
-    let mut doc = Document::read(osm_xml_input, &streets.gps_bounds, timer)?;
+    let mut doc = Document::read(
+        osm_xml_input,
+        clip_pts.as_ref().map(|pts| GPSBounds::from(pts.clone())),
+        timer,
+    )?;
+    // If GPSBounds aren't provided above, they'll be computed in the Document
+    streets.gps_bounds = doc.gps_bounds.clone().unwrap();
 
     if let Some(pts) = clip_pts {
-        let gps_bounds = GPSBounds::from(pts.clone());
-        streets.boundary_polygon = Ring::new(gps_bounds.convert(&pts))?.into_polygon();
-        streets.gps_bounds = gps_bounds;
+        streets.boundary_polygon = Ring::new(streets.gps_bounds.convert(&pts))?.into_polygon();
         doc.clip(&streets.boundary_polygon);
     } else {
-        // Use the boundary from .osm.
-        streets.gps_bounds = doc.gps_bounds.clone();
         streets.boundary_polygon = streets.gps_bounds.to_bounds().get_rectangle();
         // No need to clip the Document in this case.
     }

streets_reader/src/osm_reader/clip.rs

@@ -1,4 +1,4 @@
-use geom::{PolyLine, Ring, Polygon};
+use geom::{Distance, PolyLine, Polygon};
 
 use super::Document;
 
@@ -9,12 +9,14 @@ impl Document {
     pub fn clip(&mut self, boundary_polygon: &Polygon) {
         // Remove all nodes that're out-of-bounds. Don't fix up ways and relations referring to
         // these.
-        self.nodes.retain(|_, node| boundary_polygon.contains_pt(node.pt));
+        self.nodes
+            .retain(|_, node| boundary_polygon.contains_pt(node.pt));
 
         // Remove ways that have no nodes within bounds.
         // TODO If there's a way that geometrically crosses the boundary but only has nodes outside
         // it, this'll remove it. Is that desirable?
-        self.ways.retain(|_, way| way.nodes.iter().any(|node| self.nodes.contains_key(node)));
+        self.ways
+            .retain(|_, way| way.nodes.iter().any(|node| self.nodes.contains_key(node)));
 
         // For line-string ways (not areas), clip them to the boundary. way.pts and way.nodes
         // become out-of-sync.
@@ -25,7 +27,7 @@ impl Document {
             }
 
             let pl = PolyLine::unchecked_new(way.pts.clone());
-            way.pts = clip_polyline_to_ring(pl, boundary_polygon.get_outer_ring()).into_points();
+            way.pts = clip_polyline_to_ring(pl, boundary_polygon).into_points();
         }
 
         // TODO Handle ways that're areas
@@ -34,20 +36,32 @@ impl Document {
 }
 
 // TODO Move to geom and test better
-fn clip_polyline_to_ring(pl: PolyLine, ring: &Ring) -> PolyLine {
-    let hits = ring.all_intersections(&pl);
-    if hits.len() == 2 {
-        if let Some((mut dist1, _)) = pl.dist_along_of_point(hits[0]) {
-            if let Some((mut dist2, _)) = pl.dist_along_of_point(hits[1]) {
-                if dist1 > dist2 {
-                    std::mem::swap(&mut dist1, &mut dist2);
-                }
-                if let Ok(slice) = pl.maybe_exact_slice(dist1, dist2) {
-                    return slice;
-                }
-            }
+// If this fails for any reason, just return the input untransformed
+fn clip_polyline_to_ring(pl: PolyLine, polygon: &Polygon) -> 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;
+        }
+    }
+
+    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());
         }
     }
-    // If this fails for any reason, just return the input untransformed
+
+    if hit_distances.len() == 2 {
+        hit_distances.sort();
+        if let Ok(slice) = pl.maybe_exact_slice(hit_distances[0], hit_distances[1]) {
+            return slice;
+        }
+    }
+
     pl
 }

streets_reader/src/osm_reader/mod.rs

@@ -11,7 +11,8 @@ mod multipolygon;
 mod reader;
 
 pub struct Document {
-    pub gps_bounds: GPSBounds,
+    // This is guaranteed to be filled out after Document::read
+    pub gps_bounds: Option<GPSBounds>,
     pub nodes: BTreeMap<NodeID, Node>,
     pub ways: BTreeMap<WayID, Way>,
     pub relations: BTreeMap<RelationID, Relation>,

streets_reader/src/osm_reader/reader.rs

@@ -22,9 +22,13 @@ use super::{Document, Node, Relation, Way};
 
 impl Document {
     /// Parses raw OSM XML and extracts all objects.
-    pub fn read(raw_string: &str, input_gps_bounds: &GPSBounds, timer: &mut Timer) -> Result<Self> {
+    pub fn read(
+        raw_string: &str,
+        gps_bounds: Option<GPSBounds>,
+        timer: &mut Timer,
+    ) -> Result<Self> {
         let mut doc = Self {
-            gps_bounds: input_gps_bounds.clone(),
+            gps_bounds,
             nodes: BTreeMap::new(),
             ways: BTreeMap::new(),
             relations: BTreeMap::new(),
@@ -42,25 +46,27 @@ impl Document {
             match obj.name {
                 "bounds" => {
                     // If we weren't provided with GPSBounds, use this.
-                    if doc.gps_bounds != GPSBounds::new() {
+                    if doc.gps_bounds.is_some() {
                         continue;
                     }
-                    doc.gps_bounds.update(LonLat::new(
-                        obj.attribute("minlon").parse::<f64>().unwrap(),
-                        obj.attribute("minlat").parse::<f64>().unwrap(),
-                    ));
-                    doc.gps_bounds.update(LonLat::new(
-                        obj.attribute("maxlon").parse::<f64>().unwrap(),
-                        obj.attribute("maxlat").parse::<f64>().unwrap(),
-                    ));
+                    doc.gps_bounds = Some(GPSBounds::from(vec![
+                        LonLat::new(
+                            obj.attribute("minlon").parse::<f64>().unwrap(),
+                            obj.attribute("minlat").parse::<f64>().unwrap(),
+                        ),
+                        LonLat::new(
+                            obj.attribute("maxlon").parse::<f64>().unwrap(),
+                            obj.attribute("maxlat").parse::<f64>().unwrap(),
+                        ),
+                    ]));
                 }
                 "node" => {
-                    if doc.gps_bounds == GPSBounds::new() {
+                    if doc.gps_bounds.is_none() {
                         warn!(
                             "No clipping polygon provided and the .osm is missing a <bounds> element, \
                              so figuring out the bounds manually."
                         );
-                        doc.gps_bounds = scrape_bounds(raw_string);
+                        doc.gps_bounds = Some(scrape_bounds(raw_string));
                     }
 
                     let id = NodeID(obj.attribute("id").parse::<i64>().unwrap());
@@ -71,7 +77,7 @@ impl Document {
                         obj.attribute("lon").parse::<f64>().unwrap(),
                         obj.attribute("lat").parse::<f64>().unwrap(),
                     )
-                    .to_pt(&doc.gps_bounds);
+                    .to_pt(doc.gps_bounds.as_ref().unwrap());
                     let tags = read_tags(&mut reader);
                     doc.nodes.insert(id, Node { pt, tags });
                 }