-
Notifications
You must be signed in to change notification settings - Fork 8
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
Start reviving the cycleway snapping / sidepath zipping experiment #212
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
a8b40d2
Just rename 'snap cycleways' to 'zip parallel sidepaths'
dabreegster c528488
Relax the definition of is_cycleway to allow sidewalks, so st_georges…
dabreegster c9ac89c
Express sidepath zipping as an operation
dabreegster 3a3a0a9
Allow a sidepath to be zipped manually from Street Explorer
dabreegster File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,3 +3,4 @@ | |
mod collapse_intersection; | ||
mod collapse_short_road; | ||
mod update_geometry; | ||
pub mod zip_sidepath; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,202 @@ | ||
use geom::{Distance, PolyLine}; | ||
|
||
use crate::{BufferType, Direction, IntersectionID, LaneSpec, LaneType, RoadID, StreetNetwork}; | ||
|
||
// We're only pattern matching on one type of parallel sidepath right now. This represents a single | ||
// Road that's parallel to one or more main_roads. | ||
// | ||
// X--X | ||
// S M | ||
// S M | ||
// S M | ||
// S X | ||
// S M | ||
// S M | ||
// X--X | ||
// | ||
// S is the sidepath segment. X are intersections. M are main roads -- note there are two matching | ||
// up to this one sidepath. The '-'s are short connector roads between the two. | ||
pub struct Sidepath { | ||
sidepath: RoadID, | ||
sidepath_center: PolyLine, | ||
main_road_src_i: IntersectionID, | ||
main_road_dst_i: IntersectionID, | ||
main_roads: Vec<RoadID>, | ||
} | ||
|
||
impl Sidepath { | ||
pub fn new(streets: &StreetNetwork, start: RoadID) -> Option<Self> { | ||
const SHORT_ROAD_THRESHOLD: Distance = Distance::const_meters(10.0); | ||
|
||
let sidepath_road = &streets.roads[&start]; | ||
|
||
// Look at other roads connected to both endpoints. One of them should be "very short." | ||
let mut main_road_endpoints = Vec::new(); | ||
for i in sidepath_road.endpoints() { | ||
let mut connector_candidates = Vec::new(); | ||
for road in streets.roads_per_intersection(i) { | ||
if road.untrimmed_length() < SHORT_ROAD_THRESHOLD { | ||
connector_candidates.push(road.id); | ||
} | ||
} | ||
if connector_candidates.len() == 1 { | ||
main_road_endpoints.push(streets.roads[&connector_candidates[0]].other_side(i)); | ||
} | ||
} | ||
|
||
if main_road_endpoints.len() != 2 { | ||
return None; | ||
} | ||
|
||
// Often the main road parallel to this sidepath segment is just one road, but it might | ||
// be more. | ||
let main_road_src_i = main_road_endpoints[0]; | ||
let main_road_dst_i = main_road_endpoints[1]; | ||
|
||
// Find all main road segments "parallel to" this sidepath, by pathfinding between the | ||
// main road intersections. We don't care about the order, but simple_path does. In | ||
// case it's one-way for driving, try both. | ||
if let Some(path) = streets | ||
.simple_path(main_road_src_i, main_road_dst_i, &[LaneType::Driving]) | ||
.or_else(|| streets.simple_path(main_road_dst_i, main_road_src_i, &[LaneType::Driving])) | ||
{ | ||
return Some(Self { | ||
sidepath: sidepath_road.id, | ||
sidepath_center: sidepath_road.center_line.clone(), | ||
main_road_src_i, | ||
main_road_dst_i, | ||
main_roads: path.into_iter().map(|(r, _)| r).collect(), | ||
}); | ||
} | ||
|
||
None | ||
} | ||
|
||
pub fn debug(&self, streets: &mut StreetNetwork, label: String) { | ||
streets.debug_road(self.sidepath, format!("sidepath {label}")); | ||
streets.debug_intersection(self.main_road_src_i, format!("src_i of {label}")); | ||
streets.debug_intersection(self.main_road_dst_i, format!("dst_i of {label}")); | ||
for x in &self.main_roads { | ||
streets.debug_road(*x, format!("main road along {label}")); | ||
} | ||
} | ||
|
||
pub fn zip(self, streets: &mut StreetNetwork) { | ||
assert!(streets.roads.contains_key(&self.sidepath)); | ||
|
||
// Remove the sidepath, but remember the lanes it contained | ||
let mut sidepath_lanes = streets.remove_road(self.sidepath).lane_specs_ltr; | ||
|
||
// TODO Preserve osm_ids | ||
|
||
// TODO Re-evaluate this! | ||
// The sidepath likely had shoulder lanes assigned to it by get_lane_specs_ltr, because we have | ||
// many partially competing strategies for representing shared walking/cycling roads. Remove | ||
// those. | ||
if sidepath_lanes[0].lt == LaneType::Shoulder { | ||
sidepath_lanes.remove(0); | ||
} | ||
if sidepath_lanes.last().as_ref().unwrap().lt == LaneType::Shoulder { | ||
sidepath_lanes.pop(); | ||
} | ||
|
||
// The sidepath was tagged as a separate way due to some kind of physical separation. We'll | ||
// represent that with a buffer lane. | ||
let buffer = LaneSpec { | ||
// TODO Use https://wiki.openstreetmap.org/wiki/Proposed_features/separation if | ||
// available | ||
lt: LaneType::Buffer(BufferType::Planters), | ||
dir: Direction::Fwd, | ||
width: LaneSpec::typical_lane_width(LaneType::Buffer(BufferType::Planters)), | ||
allowed_turns: Default::default(), | ||
}; | ||
|
||
// For every main road segment corresponding to the sidepath, we need to insert these | ||
// sidepath_lanes somewhere. | ||
// | ||
// - Fixing the direction of the lanes | ||
// - Appending them on the left or right side (and "inside" the inferred sidewalk on the road) | ||
// - Inserting the buffer | ||
for r in self.main_roads { | ||
let main_road = &streets.roads[&r]; | ||
// Which side is closer to the sidepath? | ||
let (left, right) = main_road | ||
.get_untrimmed_sides(streets.config.driving_side) | ||
.unwrap(); | ||
// TODO georust has a way to check distance of linestrings. But for now, just check the | ||
// middles | ||
let snap_to_left = self.sidepath_center.middle().dist_to(left.middle()) | ||
< self.sidepath_center.middle().dist_to(right.middle()); | ||
|
||
// Does the sidepath point the same direction as this main road? We can use the left or | ||
// right side, doesn't matter. | ||
// TODO Check this logic very carefully; angles always lead to bugs. 90 is a very generous | ||
// definition of parallel. But we have a binary decision to make, so maybe we should even | ||
// use 180. | ||
let oriented_same_way = self | ||
.sidepath_center | ||
.overall_angle() | ||
.approx_eq(left.overall_angle(), 90.0); | ||
|
||
// Where should we insert the sidepath lanes? If the main road already has a sidewalk, | ||
// let's assume it should stay at the outermost part of the road. (That isn't always true, | ||
// but it's an assumption we'll take for now.) | ||
let insert_idx = if snap_to_left { | ||
if main_road.lane_specs_ltr[0].lt.is_walkable() { | ||
1 | ||
} else { | ||
0 | ||
} | ||
} else { | ||
if main_road | ||
.lane_specs_ltr | ||
.last() | ||
.as_ref() | ||
.unwrap() | ||
.lt | ||
.is_walkable() | ||
{ | ||
main_road.lane_specs_ltr.len() - 1 | ||
} else { | ||
main_road.lane_specs_ltr.len() | ||
} | ||
}; | ||
|
||
streets.debug_road(r, format!("snap_to_left = {snap_to_left}, oriented_same_way = {oriented_same_way}, insert_idx = {insert_idx}")); | ||
|
||
// This logic thankfully doesn't depend on driving side at all! | ||
let mut insert_lanes = Vec::new(); | ||
for mut lane in sidepath_lanes.clone() { | ||
if !oriented_same_way { | ||
lane.dir = lane.dir.opposite(); | ||
} | ||
insert_lanes.push(lane); | ||
} | ||
// TODO Do we ever need to reverse the order of the lanes? | ||
let mut buffer_lane = buffer.clone(); | ||
if snap_to_left { | ||
// TODO I'm not sure what direction the buffer lane should face. This is a very strong | ||
// argument for Direction::Both. | ||
buffer_lane.dir = insert_lanes.last().as_ref().unwrap().dir; | ||
insert_lanes.push(buffer_lane); | ||
} else { | ||
buffer_lane.dir = insert_lanes[0].dir; | ||
insert_lanes.insert(0, buffer_lane); | ||
} | ||
|
||
let main_road = streets.roads.get_mut(&r).unwrap(); | ||
splice_in(&mut main_road.lane_specs_ltr, insert_idx, insert_lanes); | ||
} | ||
|
||
// After this transformation, we should run CollapseDegenerateIntersections to handle the | ||
// intersection where the side road originally crossed the sidepath, and TrimDeadendCycleways | ||
// to clean up any small cycle connection roads. | ||
} | ||
} | ||
|
||
// Insert all of `insert` at `idx` in `target` | ||
fn splice_in<T>(target: &mut Vec<T>, idx: usize, insert: Vec<T>) { | ||
let tail = target.split_off(idx); | ||
target.extend(insert); | ||
target.extend(tail); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
use crate::{Sidepath, StreetNetwork}; | ||
|
||
/// Find sidepath segments that exist as separate objects, parallel to a main road. Zip (or "snap") | ||
/// them into the main road, inserting a buffer lane to represent the physical division. | ||
pub fn zip_sidepaths(streets: &mut StreetNetwork) { | ||
let mut sidepaths = Vec::new(); | ||
for r in streets.roads.values() { | ||
// TODO Or footpath | ||
if r.is_cycleway() { | ||
sidepaths.extend(Sidepath::new(streets, r.id)); | ||
} | ||
} | ||
|
||
for (idx, sidepath) in sidepaths.into_iter().enumerate() { | ||
streets.maybe_start_debug_step(format!("snap sidepath {idx}")); | ||
sidepath.debug(streets, idx.to_string()); | ||
sidepath.zip(streets); | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This had no effect. Even if it was false, we would choose to collapse.