From ff74cad4bea0e7c3f861cf8ca92259e624dc31b6 Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Tue, 13 Dec 2022 10:33:10 +0000 Subject: [PATCH] Overhaul StreetNetwork road splitting + clipping. #127 Also has the side effect of getting rid of handling for tiny roundabouts, because it's hard to reason about... --- osm2streets/src/road.rs | 4 + streets_reader/src/clip.rs | 125 ---------------- streets_reader/src/lib.rs | 3 +- streets_reader/src/osm_reader/clip.rs | 48 +++--- streets_reader/src/split_ways.rs | 201 ++++++++++---------------- 5 files changed, 110 insertions(+), 271 deletions(-) delete mode 100644 streets_reader/src/clip.rs diff --git a/osm2streets/src/road.rs b/osm2streets/src/road.rs index a837a8eb..f8dba4d2 100644 --- a/osm2streets/src/road.rs +++ b/osm2streets/src/road.rs @@ -443,6 +443,10 @@ impl Road { pub fn common_endpoint(&self, other: &Road) -> CommonEndpoint { CommonEndpoint::new((self.src_i, self.dst_i), (other.src_i, other.dst_i)) } + + pub fn from_osm_way(&self, way: osm::WayID) -> bool { + self.osm_ids.iter().any(|id| id.osm_way_id == way) + } } impl StreetNetwork { diff --git a/streets_reader/src/clip.rs b/streets_reader/src/clip.rs deleted file mode 100644 index 9414e570..00000000 --- a/streets_reader/src/clip.rs +++ /dev/null @@ -1,125 +0,0 @@ -use abstutil::Timer; -use anyhow::Result; - -use osm2streets::{IntersectionControl, IntersectionID, IntersectionKind, StreetNetwork}; - -// TODO This needs to update turn restrictions too -pub fn clip_map(streets: &mut StreetNetwork, timer: &mut Timer) -> Result<()> { - timer.start("clipping map to boundary"); - - // So we can use retain without borrowing issues - let boundary_polygon = streets.boundary_polygon.clone(); - let boundary_ring = boundary_polygon.get_outer_ring(); - - // First, just remove roads that both start and end outside the boundary polygon. - streets.retain_roads(|r| { - let first_in = boundary_polygon.contains_pt(r.reference_line.first_pt()); - let last_in = boundary_polygon.contains_pt(r.reference_line.last_pt()); - let light_rail_ok = if r.is_light_rail() { - // Make sure it's in the boundary somewhere - r.reference_line - .points() - .iter() - .any(|pt| boundary_polygon.contains_pt(*pt)) - } else { - false - }; - first_in || last_in || light_rail_ok - }); - - // Get rid of orphaned intersections too - streets.intersections.retain(|_, i| !i.roads.is_empty()); - - // Look for intersections outside the map. If they happen to be connected to multiple roads, - // then we'll need to copy the intersection for each connecting road. This effectively - // disconnects two roads in the map that would be connected if we left in some - // partly-out-of-bounds road. - let intersection_ids: Vec = streets.intersections.keys().cloned().collect(); - for old_id in intersection_ids { - if streets - .boundary_polygon - .contains_pt(streets.intersections[&old_id].point) - { - continue; - } - - let mut old_intersection = streets.intersections.remove(&old_id).unwrap(); - // Derived data is handled independantly for MapEdge intersections, so set it here. - old_intersection.kind = IntersectionKind::MapEdge; - old_intersection.control = IntersectionControl::Uncontrolled; - old_intersection.movements = Vec::new(); - - if old_intersection.roads.len() <= 1 { - // We don't need to make copies of the intersection; put it back - streets.intersections.insert(old_id, old_intersection); - continue; - } - for r in old_intersection.roads.clone() { - let mut copy = old_intersection.clone(); - copy.roads = vec![r]; - copy.id = streets.next_intersection_id(); - // Leave osm_ids alone; all copies of this intersection share provenance - - let road = streets.roads.get_mut(&r).unwrap(); - if road.src_i == old_id { - road.src_i = copy.id; - } - if road.dst_i == old_id { - road.dst_i = copy.id; - } - - streets.intersections.insert(copy.id, copy); - } - } - - // For all the map edge intersections, find the one road they connect to and trim their points. - for intersection in streets.intersections.values_mut() { - if intersection.kind != IntersectionKind::MapEdge { - continue; - } - assert_eq!(intersection.roads.len(), 1); - let r = intersection.roads[0]; - - let road = streets.roads.get_mut(&r).unwrap(); - let boundary_pts = boundary_ring.all_intersections(&road.reference_line); - if boundary_pts.is_empty() { - // The intersection is out-of-bounds, but a road leading to it doesn't cross the - // boundary? - warn!("{} interacts with boundary strangely", r); - continue; - } - - if road.src_i == intersection.id { - // Starting out-of-bounds - let boundary_pt = boundary_pts[0]; - if let Some(pl) = road.reference_line.get_slice_starting_at(boundary_pt) { - road.reference_line = pl; - road.update_center_line(streets.config.driving_side); - intersection.point = road.reference_line.first_pt(); - } else { - warn!("{} interacts with boundary strangely", r); - continue; - } - } else { - // Ending out-of-bounds - // For light rail, the center-line might cross the boundary twice. When we're looking - // at the outbound end, take the last time we hit the boundary - let boundary_pt = *boundary_pts.last().unwrap(); - if let Some(pl) = road.reference_line.get_slice_ending_at(boundary_pt) { - road.reference_line = pl; - road.update_center_line(streets.config.driving_side); - intersection.point = road.reference_line.last_pt(); - } else { - warn!("{} interacts with boundary strangely", r); - continue; - } - } - } - - if streets.roads.is_empty() { - bail!("There are no roads inside the clipping polygon"); - } - - timer.stop("clipping map to boundary"); - Ok(()) -} diff --git a/streets_reader/src/lib.rs b/streets_reader/src/lib.rs index 7a76048c..7224f146 100644 --- a/streets_reader/src/lib.rs +++ b/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 diff --git a/streets_reader/src/osm_reader/clip.rs b/streets_reader/src/osm_reader/clip.rs index b44e9a08..29a3b354 100644 --- a/streets_reader/src/osm_reader/clip.rs +++ b/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 } diff --git a/streets_reader/src/split_ways.rs b/streets_reader/src/split_ways.rs index 0d2b07b8..2133751e 100644 --- a/streets_reader/src/split_ways.rs +++ b/streets_reader/src/split_ways.rs @@ -1,16 +1,16 @@ -use std::collections::{btree_map::Entry, BTreeMap, HashMap}; +use std::collections::{hash_map::Entry, HashMap}; -use abstutil::{Counter, Tags, Timer}; -use geom::{Distance, HashablePt2D, PolyLine, Pt2D}; +use abstutil::{Counter, Timer}; +use geom::{HashablePt2D, PolyLine, Pt2D}; use osm2streets::{ - osm, Direction, IntersectionControl, IntersectionID, IntersectionKind, OriginalRoad, Road, - RoadID, StreetNetwork, + Direction, IntersectionControl, IntersectionID, IntersectionKind, OriginalRoad, Road, RoadID, + StreetNetwork, }; use super::OsmExtract; -/// Returns a mapping of all points to the split road. Some internal points on roads get removed -/// here, so this mapping isn't redundant. +/// Also returns a mapping of all points to the split road. Some internal points on roads get +/// removed here, so this mapping isn't redundant. pub fn split_up_roads( streets: &mut StreetNetwork, mut input: OsmExtract, @@ -18,79 +18,45 @@ pub fn split_up_roads( ) -> HashMap { timer.start("splitting up roads"); - let mut roundabout_centers: Vec<(Pt2D, Vec)> = Vec::new(); - // Note we iterate over this later and assign IDs based on the order, so HashMap would be - // non-deterministic - let mut pt_to_intersection: BTreeMap = BTreeMap::new(); - - input.roads.retain(|(id, pts, tags)| { - if should_collapse_roundabout(pts, tags) { - info!("Collapsing tiny roundabout {}", id); - - let ids: Vec = pts - .iter() - .map(|pt| input.osm_node_ids[&pt.to_hashable()]) - .collect(); - - // Arbitrarily use the first node's ID - // TODO Test more carefully after opaque IDs - let id = ids[0]; - - for pt in pts { - pt_to_intersection.insert(pt.to_hashable(), id); - } - - roundabout_centers.push((Pt2D::center(pts), ids)); - - false - } else { - true - } - }); + // Note all logic here is based on treating points as HashablePt2D, not as OSM node IDs. That's + // because some members of way.pts might be synthetic, from clipping. + // Create intersections for any points shared by at least 2 roads, and for endpoints of every + // road. let mut counts_per_pt = Counter::new(); + let mut pt_to_intersection_id: HashMap = HashMap::new(); for (_, pts, _) in &input.roads { - for (idx, raw_pt) in pts.iter().enumerate() { - let pt = raw_pt.to_hashable(); - let count = counts_per_pt.inc(pt); + for (idx, pt) in pts.iter().enumerate() { + let hash_pt = pt.to_hashable(); + let count = counts_per_pt.inc(hash_pt); - // All start and endpoints of ways are also intersections. if count == 2 || idx == 0 || idx == pts.len() - 1 { - if let Entry::Vacant(e) = pt_to_intersection.entry(pt) { - let id = input.osm_node_ids[&pt]; - e.insert(id); - } - } - } - } + if let Entry::Vacant(entry) = pt_to_intersection_id.entry(hash_pt) { + // Clipped points won't have any OSM ID. + let mut osm_ids = Vec::new(); + if let Some(node_id) = input.osm_node_ids.get(&hash_pt) { + osm_ids.push(*node_id); + } - let mut osm_id_to_id: HashMap = HashMap::new(); - for (pt, osm_id) in &pt_to_intersection { - let id = streets.insert_intersection( - vec![*osm_id], - pt.to_pt2d(), - // Assume a complicated intersection, until we determine otherwise. - IntersectionKind::Intersection, - if input.traffic_signals.remove(pt).is_some() { - IntersectionControl::Signalled - } else { - // TODO default to uncontrolled, guess StopSign as a transform - IntersectionControl::Signed - }, - ); - osm_id_to_id.insert(*osm_id, id); - } + let kind = if osm_ids.is_empty() { + IntersectionKind::MapEdge + } else { + // Assume a complicated intersection, until we determine otherwise + IntersectionKind::Intersection + }; + let control = if osm_ids.is_empty() { + IntersectionControl::Uncontrolled + } else if input.traffic_signals.remove(&hash_pt).is_some() { + IntersectionControl::Signalled + } else { + // TODO default to uncontrolled, guess StopSign as a transform + IntersectionControl::Signed + }; - // Set roundabouts to their center - for (pt, osm_ids) in roundabout_centers { - let id = streets.insert_intersection( - osm_ids.clone(), - pt, - IntersectionKind::Intersection, - IntersectionControl::Signed, - ); - for osm_id in osm_ids { - osm_id_to_id.insert(osm_id, id); + let id = streets.insert_intersection(osm_ids, *pt, kind, control); + entry.insert(id); + } + } } } @@ -102,15 +68,14 @@ pub fn split_up_roads( timer.next(); let mut tags = orig_tags.clone(); let mut pts = Vec::new(); - let endpt1 = pt_to_intersection[&orig_pts[0].to_hashable()]; - let mut i1 = endpt1; + let mut i1 = pt_to_intersection_id[&orig_pts[0].to_hashable()]; for pt in orig_pts { pts.push(*pt); if pts.len() == 1 { continue; } - if let Some(i2) = pt_to_intersection.get(&pt.to_hashable()) { + if let Some(i2) = pt_to_intersection_id.get(&pt.to_hashable()) { let id = streets.next_road_id(); // Note we populate this before simplify_linestring, so even if some points are @@ -121,37 +86,29 @@ pub fn split_up_roads( } } + let i1_node_id = input.osm_node_ids.get(&pts[0].to_hashable()).cloned(); + let i2_node_id = input.osm_node_ids.get(&pt.to_hashable()).cloned(); + let untrimmed_center_line = simplify_linestring(std::mem::take(&mut pts)); match PolyLine::new(untrimmed_center_line) { Ok(pl) => { + // TODO If either endpoint is on the boundary, we won't record any OSM ID. + // Should we just store WayID and not OriginalRoad? + let mut osm_ids = Vec::new(); + if let (Some(i1_node), Some(i2_node)) = (i1_node_id, i2_node_id) { + osm_ids.push(OriginalRoad { + osm_way_id: *osm_way_id, + i1: i1_node, + i2: i2_node, + }); + } + streets.roads.insert( id, - Road::new( - id, - vec![OriginalRoad { - osm_way_id: *osm_way_id, - i1, - i2: *i2, - }], - osm_id_to_id[&i1], - osm_id_to_id[i2], - pl, - tags, - &streets.config, - ), + Road::new(id, osm_ids, i1, *i2, pl, tags, &streets.config), ); - streets - .intersections - .get_mut(&osm_id_to_id[&i1]) - .unwrap() - .roads - .push(id); - streets - .intersections - .get_mut(&osm_id_to_id[i2]) - .unwrap() - .roads - .push(id); + streets.intersections.get_mut(&i1).unwrap().roads.push(id); + streets.intersections.get_mut(&i2).unwrap().roads.push(id); } Err(err) => { error!("Skipping {id}: {err}"); @@ -173,8 +130,12 @@ pub fn split_up_roads( let mut restrictions = Vec::new(); for (restriction, from_osm, via_osm, to_osm) in input.simple_turn_restrictions { // A via node might not be an intersection - let via_id = if let Some(x) = osm_id_to_id.get(&via_osm) { - *x + let via_id = if let Some(i) = streets + .intersections + .values() + .find(|i| i.osm_ids.contains(&via_osm)) + { + i.id } else { continue; }; @@ -185,8 +146,8 @@ pub fn split_up_roads( // If some of the roads are missing, they were likely filtered out -- usually service // roads. if let (Some(from), Some(to)) = ( - roads.iter().find(|r| r.osm_ids[0].osm_way_id == from_osm), - roads.iter().find(|r| r.osm_ids[0].osm_way_id == to_osm), + roads.iter().find(|r| r.from_osm_way(from_osm)), + roads.iter().find(|r| r.from_osm_way(to_osm)), ) { restrictions.push((from.id, restriction, to.id)); } @@ -207,13 +168,11 @@ pub fn split_up_roads( let via_candidates: Vec<&Road> = streets .roads .values() - .filter(|r| r.osm_ids[0].osm_way_id == via_osm) + .filter(|r| r.from_osm_way(via_osm)) .collect(); if via_candidates.len() != 1 { warn!( - "Couldn't resolve turn restriction from way {} to way {} via way {}. Candidate \ - roads for via: {:?}. See {}", - from_osm, to_osm, via_osm, via_candidates, rel_osm + "Couldn't resolve turn restriction from way {from_osm} to way {to_osm} via way {via_osm}. Candidate roads for via: {:?}. See {rel_osm}", via_candidates ); continue; } @@ -223,20 +182,20 @@ pub fn split_up_roads( .roads_per_intersection(via.src_i) .into_iter() .chain(streets.roads_per_intersection(via.dst_i).into_iter()) - .find(|r| r.osm_ids[0].osm_way_id == from_osm); + .find(|r| r.from_osm_way(from_osm)); let maybe_to = streets .roads_per_intersection(via.src_i) .into_iter() .chain(streets.roads_per_intersection(via.dst_i).into_iter()) - .find(|r| r.osm_ids[0].osm_way_id == to_osm); + .find(|r| r.from_osm_way(to_osm)); match (maybe_from, maybe_to) { (Some(from), Some(to)) => { complicated_restrictions.push((from.id, via.id, to.id)); } _ => { warn!( - "Couldn't resolve turn restriction from {} to {} via {:?}", - from_osm, to_osm, via + "Couldn't resolve turn restriction from {from_osm} to {to_osm} via {:?}", + via ); } } @@ -273,8 +232,8 @@ pub fn split_up_roads( timer.stop("match traffic signals to intersections"); timer.start("calculate intersection movements"); - let intersection_ids = osm_id_to_id.values(); - for &i in intersection_ids { + let intersection_ids: Vec<_> = streets.intersections.keys().cloned().collect(); + for i in intersection_ids { streets.sort_roads(i); streets.update_movements(i); } @@ -298,15 +257,3 @@ fn simplify_linestring(pts: Vec) -> Vec { let epsilon = 0.5; Pt2D::simplify_rdp(pts, epsilon) } - -/// Many "roundabouts" like https://www.openstreetmap.org/way/427144965 are so tiny that they wind -/// up with ridiculous geometry, cause constant gridlock, and prevent merging adjacent blocks. -/// -/// Note https://www.openstreetmap.org/way/394991047 is an example of something that shouldn't get -/// modified. The only distinction, currently, is length -- but I'd love a better definition. -/// Possibly the number of connecting roads. -fn should_collapse_roundabout(pts: &[Pt2D], tags: &Tags) -> bool { - tags.is_any("junction", vec!["roundabout", "circular"]) - && pts[0] == *pts.last().unwrap() - && PolyLine::unchecked_new(pts.to_vec()).length() < Distance::meters(50.0) -}