Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache roads per intersection. #93 #98

Merged
merged 8 commits into from
Oct 2, 2022
49 changes: 42 additions & 7 deletions osm2streets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,44 @@ impl StreetNetwork {
}
}

// TODO Might be better to maintain this instead of doing a search everytime.
pub fn roads_per_intersection(&self, i: osm::NodeID) -> Vec<OriginalRoad> {
let mut results = Vec::new();
for id in self.roads.keys() {
if id.i1 == i || id.i2 == i {
results.push(*id);
pub fn insert_road(&mut self, id: OriginalRoad, road: Road) {
self.roads.insert(id, road);
// TODO Re-sort
self.intersections.get_mut(&id.i1).unwrap().roads.push(id);
self.intersections.get_mut(&id.i2).unwrap().roads.push(id);
}

pub fn remove_road(&mut self, id: &OriginalRoad) -> Road {
// TODO Re-sort
self.intersections
.get_mut(&id.i1)
.unwrap()
.roads
.retain(|r| r != id);
self.intersections
.get_mut(&id.i2)
.unwrap()
.roads
.retain(|r| r != id);
self.roads.remove(id).unwrap()
}

pub fn retain_roads<F: Fn(&OriginalRoad, &Road) -> bool>(&mut self, f: F) {
let mut remove = Vec::new();
for (id, r) in &self.roads {
if !f(id, r) {
remove.push(*id);
}
}
results
for id in remove {
self.remove_road(&id);
}
}

// TODO This'll eventually give a response in clockwise order
// TODO Consider not cloning. Many callers will have to change
pub fn roads_per_intersection(&self, i: osm::NodeID) -> Vec<OriginalRoad> {
self.intersections[&i].roads.clone()
}

pub fn new_osm_node_id(&self, start: i64) -> osm::NodeID {
Expand Down Expand Up @@ -544,6 +573,11 @@ pub struct Intersection {
pub control: ControlType,
pub elevation: Distance,

/// All roads connected to this intersection. They may be incoming or outgoing relative to this
/// intersection.
/// TODO: Guarantee they're sorted clockwise
pub roads: Vec<OriginalRoad>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a good type that has an insert_at to take advantage of the fact that roads is already sorted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like https://doc.rust-lang.org/std/vec/struct.Vec.html#method.insert ? When we're adding a new road, we could calculate its position in the list and add it, or just recalculate the ordering for the intersection. It'll depend on the sorting algorithm

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Performance doesn't really matter, so Vec works if we want to do it the "find and insert" way.


// true if src_i matches this intersection (or the deleted/consolidated one, whatever)
pub trim_roads_for_merging: BTreeMap<(osm::WayID, bool), Pt2D>,
}
Expand All @@ -555,6 +589,7 @@ impl Intersection {
complexity,
control,
// Filled out later
roads: Vec::new(),
elevation: Distance::ZERO,
trim_roads_for_merging: BTreeMap::new(),
}
Expand Down
11 changes: 6 additions & 5 deletions osm2streets/src/transform/collapse_intersections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ pub fn collapse_intersection(streets: &mut StreetNetwork, i: NodeID) {
assert_eq!(roads.len(), 2);
let mut r1 = roads[0];
let mut r2 = roads[1];
assert_ne!(r1, r2);

// We'll keep r1's way ID, so it's a little more convenient for debugging to guarantee r1 is
// the longer piece.
Expand All @@ -157,11 +158,11 @@ pub fn collapse_intersection(streets: &mut StreetNetwork, i: NodeID) {
}

info!("Collapsing degenerate {}", i);
streets.intersections.remove(&i).unwrap();
// We could be more careful merging percent_incline and osm_tags, but in practice, it doesn't
// matter for the short segments we're merging.
let mut new_road = streets.roads.remove(&r1).unwrap();
let mut road2 = streets.roads.remove(&r2).unwrap();
let mut new_road = streets.remove_road(&r1);
let mut road2 = streets.remove_road(&r2);
streets.intersections.remove(&i).unwrap();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Order matters! remove_road cleans up the two intersections, so they need to both exist until the end


// There are 4 cases, easy to understand on paper. Preserve the original direction of r1
let (new_i1, new_i2) = if r1.i2 == r2.i1 {
Expand Down Expand Up @@ -195,7 +196,7 @@ pub fn collapse_intersection(streets: &mut StreetNetwork, i: NodeID) {
i1: new_i1,
i2: new_i2,
};
streets.roads.insert(new_r1, new_road);
streets.insert_road(new_r1, new_road);

// We may need to fix up turn restrictions. r1 and r2 both become new_r1.
let rewrite = |x: &OriginalRoad| *x == r1 || *x == r2;
Expand Down Expand Up @@ -242,7 +243,7 @@ pub fn trim_deadends(streets: &mut StreetNetwork) {
}

for r in remove_roads {
streets.roads.remove(&r).unwrap();
streets.remove_road(&r);
}
for i in remove_intersections {
streets.delete_intersection(i);
Expand Down
28 changes: 15 additions & 13 deletions osm2streets/src/transform/merge_short_road.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl StreetNetwork {
if !self.intersections.contains_key(&short.i1)
|| !self.intersections.contains_key(&short.i2)
{
self.roads.remove(&short).unwrap();
self.remove_road(&short);
bail!(
"One endpoint of {} has already been deleted, skipping",
short
Expand Down Expand Up @@ -95,17 +95,10 @@ impl StreetNetwork {
.extend(trim_roads_for_merging);
}

self.roads.remove(&short).unwrap();
self.remove_road(&short);

// Arbitrarily keep i1 and destroy i2. If the intersection types differ, upgrade the
// surviving interesting.
{
// Don't use delete_intersection; we're manually fixing up connected roads
let i = self.intersections.remove(&i2).unwrap();
if i.control == ControlType::TrafficSignal {
self.intersections.get_mut(&i1).unwrap().control = ControlType::TrafficSignal;
}
}
// Arbitrarily keep i1 and destroy i2. Don't actually remove the intersection until later;
// remove_road needs the intersection to exist

// Fix up all roads connected to i2. Delete them and create a new copy; the ID changes,
// since one intersection changes.
Expand All @@ -115,7 +108,7 @@ impl StreetNetwork {
let mut new_to_old = BTreeMap::new();
for r in self.roads_per_intersection(i2) {
deleted.push(r);
let road = self.roads.remove(&r).unwrap();
let road = self.remove_road(&r);
let mut new_id = r;
if r.i1 == i2 {
new_id.i1 = i1;
Expand All @@ -133,10 +126,19 @@ impl StreetNetwork {
old_to_new.insert(r, new_id);
new_to_old.insert(new_id, r);

self.roads.insert(new_id, road);
self.insert_road(new_id, road);
created.push(new_id);
}

// If the intersection types differ, upgrade the surviving interesting.
{
// Don't use delete_intersection; we're manually fixing up connected roads
let i = self.intersections.remove(&i2).unwrap();
if i.control == ControlType::TrafficSignal {
self.intersections.get_mut(&i1).unwrap().control = ControlType::TrafficSignal;
}
}

// If we're deleting the target of a simple restriction somewhere, update it.
for (from_id, road) in &mut self.roads {
let mut fix_trs = Vec::new();
Expand Down
4 changes: 2 additions & 2 deletions osm2streets/src/transform/remove_disconnected.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ pub fn remove_disconnected_roads(streets: &mut StreetNetwork) {
for p in partitions.iter().skip(1) {
for id in p {
info!("Removing {} because it's disconnected from most roads", id);
streets.roads.remove(id).unwrap();
streets.remove_road(id);
next_roads.remove(id.i1, *id);
next_roads.remove(id.i2, *id);
}
}

// Also remove cul-de-sacs here. TODO Support them properly, but for now, they mess up parking
// hint matching (loop PolyLine) and pathfinding later.
streets.roads.retain(|id, _| id.i1 != id.i2);
streets.retain_roads(|id, _| id.i1 != id.i2);

// Remove intersections without any roads
streets
Expand Down
2 changes: 1 addition & 1 deletion osm2streets/src/transform/sausage_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ fn fix(streets: &mut StreetNetwork, id1: OriginalRoad, id2: OriginalRoad) {
assert!(streets.roads.contains_key(&id2));

// Arbitrarily remove the 2nd
let mut road2 = streets.roads.remove(&id2).unwrap();
let mut road2 = streets.remove_road(&id2);
// And modify the 1st
let road1 = streets.roads.get_mut(&id1).unwrap();

Expand Down
2 changes: 1 addition & 1 deletion osm2streets/src/transform/snappy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub fn snap_cycleways(streets: &mut StreetNetwork) {
snapped_ids.push(cycleway_id);

// Remove the separate cycleway
let deleted_cycleway = streets.roads.remove(&cycleway_id).unwrap();
let deleted_cycleway = streets.remove_road(&cycleway_id);

// Add it as an attribute to the roads instead
for (road_id, dir) in roads {
Expand Down
12 changes: 7 additions & 5 deletions streets_reader/src/clip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub fn clip_map(streets: &mut StreetNetwork, timer: &mut Timer) -> Result<()> {

// This is kind of indirect and slow, but first pass -- just remove roads that start or end
// outside the boundary polygon.
streets.roads.retain(|_, r| {
streets.retain_roads(|_, r| {
let first_in = boundary_polygon.contains_pt(r.osm_center_points[0]);
let last_in = boundary_polygon.contains_pt(*r.osm_center_points.last().unwrap());
let light_rail_ok = if r.is_light_rail() {
Expand Down Expand Up @@ -63,12 +63,13 @@ pub fn clip_map(streets: &mut StreetNetwork, timer: &mut Timer) -> Result<()> {
info!("Disconnecting {} from some other stuff (starting OOB)", id);
}

let mut mut_r = streets.remove_road(&id);

let i = streets.intersections.get_mut(&move_i).unwrap();
i.complexity = IntersectionComplexity::MapEdge;
i.control = ControlType::Border;

// Now trim it.
let mut mut_r = streets.roads.remove(&id).unwrap();
let center = PolyLine::must_new(mut_r.osm_center_points.clone());
let border_pt = boundary_ring.all_intersections(&center)[0];
if let Some(pl) = center.reversed().get_slice_ending_at(border_pt) {
Expand All @@ -78,7 +79,7 @@ pub fn clip_map(streets: &mut StreetNetwork, timer: &mut Timer) -> Result<()> {
continue;
}
i.point = mut_r.osm_center_points[0];
streets.roads.insert(
streets.insert_road(
OriginalRoad {
osm_way_id: id.osm_way_id,
i1: move_i,
Expand Down Expand Up @@ -120,12 +121,13 @@ pub fn clip_map(streets: &mut StreetNetwork, timer: &mut Timer) -> Result<()> {
info!("Disconnecting {} from some other stuff (ending OOB)", id);
}

let mut mut_r = streets.remove_road(&id);

let i = streets.intersections.get_mut(&move_i).unwrap();
i.complexity = IntersectionComplexity::MapEdge;
i.control = ControlType::Border;

// Now trim it.
let mut mut_r = streets.roads.remove(&id).unwrap();
let center = PolyLine::must_new(mut_r.osm_center_points.clone());
let border_pt = boundary_ring.all_intersections(&center.reversed())[0];
if let Some(pl) = center.get_slice_ending_at(border_pt) {
Expand All @@ -135,7 +137,7 @@ pub fn clip_map(streets: &mut StreetNetwork, timer: &mut Timer) -> Result<()> {
continue;
}
i.point = *mut_r.osm_center_points.last().unwrap();
streets.roads.insert(
streets.insert_road(
OriginalRoad {
osm_way_id: id.osm_way_id,
i1: id.i1,
Expand Down
2 changes: 1 addition & 1 deletion streets_reader/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ pub fn osm_to_street_network(

// Need to do a first pass of removing cul-de-sacs here, or we wind up with loop PolyLines when
// doing the parking hint matching.
streets.roads.retain(|r, _| r.i1 != r.i2);
streets.retain_roads(|r, _| r.i1 != r.i2);

use_barrier_nodes(
&mut streets,
Expand Down
2 changes: 1 addition & 1 deletion streets_reader/src/split_ways.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub fn split_up_roads(
let osm_center_pts = simplify_linestring(std::mem::take(&mut pts));
match Road::new(osm_center_pts, tags, &streets.config) {
Ok(road) => {
streets.roads.insert(id, road);
streets.insert_road(id, road);
}
Err(err) => {
error!("Skipping {id}: {err}");
Expand Down