diff --git a/docs/design/walking.md b/docs/design/walking.md index 4b7b224493..6327b61e64 100644 --- a/docs/design/walking.md +++ b/docs/design/walking.md @@ -65,6 +65,9 @@ And what about modeling shared left-turn lanes? - Are these even that important to model? Usually used for turning into parking lots or driveways, which we're not modeling at all. +One-way sidewalk lanes would NOT solve the turn-chains: +- think about crossing N, then W at a 4-way. legitimately doing two turns in sequence. and this is fine! + An alternative: - in sim, pathfinding, map model trace, etc layers only, using some new abstraction instead of raw lanes and implied turns @@ -77,3 +80,16 @@ An alternative: - this abstraction can just say whether to go backwards on a sidewalk or not - whether or not sidewalks later get split into 2 lanes, I think this would be helpful. +- places to change... + - map model pathfinding. + - proper type, backed by VecDeque + - backrefs can store the intermediate piece often + - complication with not crossing a sidewalk? maybe that can be + deduped there, in one spot + - trace_route should move to become part of this Path type + - no more partly duped code btwn walking/driving + - Traversable::slice can probably also go away, or only get + called by this one place? + - sim layer no longer needs to pick turns + - walking code no longer needs to calculate contraflow itself! + - maybe should plumb start/end dist_along into pathfinding too? diff --git a/map_model/src/make/bus_stops.rs b/map_model/src/make/bus_stops.rs index 56c1db3a37..e0d5aaa402 100644 --- a/map_model/src/make/bus_stops.rs +++ b/map_model/src/make/bus_stops.rs @@ -117,8 +117,14 @@ pub fn verify_bus_routes(map: &Map, routes: Vec, timer: &mut Timer) -> { let bs1 = map.get_bs(*stop1); let bs2 = map.get_bs(*stop2); - if Pathfinder::shortest_distance(map, bs1.driving_lane, bs2.driving_lane, false) - .is_none() + if Pathfinder::shortest_distance( + map, + bs1.driving_lane, + bs1.dist_along, + bs2.driving_lane, + bs2.dist_along, + false, + ).is_none() { warn!( "Removing route {} since {:?} and {:?} aren't connected", diff --git a/map_model/src/map.rs b/map_model/src/map.rs index 6659f93207..c21c6fd9f6 100644 --- a/map_model/src/map.rs +++ b/map_model/src/map.rs @@ -400,9 +400,8 @@ impl Map { // Sidewalks are bidirectional if lane.is_sidewalk() { for t in &self.get_i(lane.src_i).turns { - let turn = self.get_t(*t); - if turn.src == l { - turns.push(turn); + if t.src == l { + turns.push(self.get_t(*t)); } } } diff --git a/map_model/src/pathfind.rs b/map_model/src/pathfind.rs index 3def368e7e..dd5b5acca2 100644 --- a/map_model/src/pathfind.rs +++ b/map_model/src/pathfind.rs @@ -1,7 +1,31 @@ +use dimensioned::si; use geom::{Line, Pt2D}; use ordered_float::NotNaN; use std::collections::{BinaryHeap, HashMap, VecDeque}; -use {LaneID, LaneType, Map}; +use {LaneID, LaneType, Map, TurnID}; + +#[derive(Debug, PartialEq)] +pub enum PathStep { + // Original direction + Lane(LaneID), + // Sidewalks only! + ContraflowLane(LaneID), + Turn(TurnID), +} + +pub struct Path { + // TODO way to encode start/end dist? I think it's needed for trace_route later... + // actually not start dist -- that really changes all the time + steps: VecDeque, +} + +impl Path { + fn new(steps: Vec) -> Path { + Path { + steps: VecDeque::from(steps), + } + } +} pub enum Pathfinder { ShortestDistance { goal_pt: Pt2D, is_bike: bool }, @@ -15,13 +39,16 @@ impl Pathfinder { pub fn shortest_distance( map: &Map, start: LaneID, + start_dist: si::Meter, end: LaneID, + end_dist: si::Meter, is_bike: bool, - ) -> Option> { + ) -> Option { // TODO using first_pt here and in heuristic_dist is particularly bad for walking // directions let goal_pt = map.get_l(end).first_pt(); - Pathfinder::ShortestDistance { goal_pt, is_bike }.pathfind(map, start, end) + Pathfinder::ShortestDistance { goal_pt, is_bike } + .pathfind(map, start, start_dist, end, end_dist) } fn expand(&self, map: &Map, current: LaneID) -> Vec<(LaneID, NotNaN)> { @@ -61,10 +88,21 @@ impl Pathfinder { } } - fn pathfind(&self, map: &Map, start: LaneID, end: LaneID) -> Option> { + fn pathfind( + &self, + map: &Map, + start: LaneID, + start_dist: si::Meter, + end: LaneID, + end_dist: si::Meter, + ) -> Option { assert_eq!(map.get_l(start).lane_type, map.get_l(end).lane_type); if start == end { - return Some(VecDeque::from(vec![start])); + if start_dist > end_dist { + assert_eq!(map.get_l(start).lane_type, LaneType::Sidewalk); + return Some(Path::new(vec![PathStep::ContraflowLane(start)])); + } + return Some(Path::new(vec![PathStep::Lane(start)])); } // This should be deterministic, since cost ties would be broken by LaneID. @@ -78,14 +116,14 @@ impl Pathfinder { // Found it, now produce the path if current == end { - let mut path: VecDeque = VecDeque::new(); + let mut steps: VecDeque = VecDeque::new(); let mut lookup = current; loop { - path.push_front(lookup); + steps.push_front(PathStep::Lane(lookup)); if lookup == start { - assert_eq!(path[0], start); - assert_eq!(*path.back().unwrap(), end); - return Some(path); + assert_eq!(steps[0], PathStep::Lane(start)); + assert_eq!(*steps.back().unwrap(), PathStep::Lane(end)); + return Some(Path { steps }); } lookup = backrefs[&lookup]; }