diff --git a/data/screenshots/montlake/MANIFEST b/data/screenshots/montlake/MANIFEST index c512ac5059..d1d4ace382 100644 --- a/data/screenshots/montlake/MANIFEST +++ b/data/screenshots/montlake/MANIFEST @@ -1,11 +1,11 @@ 32fe6088d0626aea9e2fdfddef6f4da5 ../data/screenshots/pending_montlake/01x01_i19.png -13fd5fb8bf19120156418ac290d6ee4d ../data/screenshots/pending_montlake/02x01_i24.png -99bdbb64f8f357eb9ecc49c0241efe67 ../data/screenshots/pending_montlake/03x01_i0.png +966f567fb9aa9855f4a5b92001cf7cff ../data/screenshots/pending_montlake/02x01_i24.png +8c1227160b6a8244bbdb7c8b504d69aa ../data/screenshots/pending_montlake/03x01_i0.png abad174a1a96560ae30c2db15ab6c2cb ../data/screenshots/pending_montlake/01x02_i5.png -a15c1c86d0b902b963abf828e5b63bd4 ../data/screenshots/pending_montlake/02x02_i9.png -8212df248d5a3cc957fb9f9ba3f0d615 ../data/screenshots/pending_montlake/03x02_i8.png -e2347d5b9b240f14df5cb7a7b33a3fe3 ../data/screenshots/pending_montlake/01x03_i20.png -5f64e71201de37ebd39d7e12a0f8dbfa ../data/screenshots/pending_montlake/02x03_i71.png +5e0e26848557b3386a8aa1839ad58560 ../data/screenshots/pending_montlake/02x02_i9.png +0a109c274e02027fe355611dc354b74e ../data/screenshots/pending_montlake/03x02_i8.png +ff1f5a0a4efd4d6048de5a3703082695 ../data/screenshots/pending_montlake/01x03_i20.png +9162c2429dace890c01b9f1106af7bfe ../data/screenshots/pending_montlake/02x03_i71.png 6b7de9c100ca476a4aa2fe12830dd44a ../data/screenshots/pending_montlake/03x03_i77.png e62c06cbcc8b82714009fcc3360d5377 ../data/screenshots/pending_montlake/01x04_i4.png 2d0ad4590f8ceeaf33f0d785bbcc4cd2 ../data/screenshots/pending_montlake/02x04_i1.png @@ -16,6 +16,6 @@ bdc20ec3835d5ea97dfe876c7e91b409 ../data/screenshots/pending_montlake/02x05_i25 c74934011cc0a3edee1b0715a6c917b1 ../data/screenshots/pending_montlake/01x06_i40.png c4a802fcc9efb8a26652f3db55a89e62 ../data/screenshots/pending_montlake/02x06_i124.png 0109c9ae20b1a1974d5bd4f8607634b2 ../data/screenshots/pending_montlake/03x06_i2.png -a1af056db050da5fab430fc4758d611d ../data/screenshots/pending_montlake/01x07_i26.png -052ee656b3064277306bce62239d9aa2 ../data/screenshots/pending_montlake/02x07_i85.png +abc2e0ef3a1e2bdcb9be7cd0e8d47c6c ../data/screenshots/pending_montlake/01x07_i26.png +2af4fcf2c1dc5d9c2ca4d32a6a6fa561 ../data/screenshots/pending_montlake/02x07_i85.png a9a97911411e5408655fb6301c859578 ../data/screenshots/pending_montlake/03x07_i27.png diff --git a/game/src/edit/stop_signs.rs b/game/src/edit/stop_signs.rs index c3d4e7cf51..00f230bb50 100644 --- a/game/src/edit/stop_signs.rs +++ b/game/src/edit/stop_signs.rs @@ -69,7 +69,7 @@ impl State for StopSignEditor { if let Some(r) = self.selected_sign { if ctx.input.contextual_action(Key::Space, "toggle stop sign") { let mut sign = ui.primary.map.get_stop_sign(self.id).clone(); - sign.flip_sign(r, &ui.primary.map); + sign.flip_sign(r); let mut new_edits = ui.primary.map.get_edits().clone(); new_edits.stop_sign_overrides.insert(self.id, sign); apply_map_edits(&mut ui.primary, &ui.cs, ctx, new_edits); @@ -103,10 +103,10 @@ impl State for StopSignEditor { ui.cs.get_def("selected stop sign", Color::BLUE), octagon.clone(), ); - if !sign.roads[r].enabled { + if !sign.roads[r].must_stop { batch.push(ui.cs.get("stop sign pole").alpha(0.6), pole.clone()); } - } else if !sign.roads[r].enabled { + } else if !sign.roads[r].must_stop { batch.push( ui.cs.get("stop sign on side of road").alpha(0.6), octagon.clone(), diff --git a/game/src/render/intersection.rs b/game/src/render/intersection.rs index 6d7c2c1e9d..8619bf8382 100644 --- a/game/src/render/intersection.rs +++ b/game/src/render/intersection.rs @@ -69,7 +69,7 @@ impl DrawIntersection { } IntersectionType::StopSign => { for ss in map.get_stop_sign(i.id).roads.values() { - if ss.enabled { + if ss.must_stop { if let Some((octagon, pole)) = DrawIntersection::stop_sign_geom(ss, map) { default_geom .push(cs.get_def("stop sign on side of road", Color::RED), octagon); @@ -94,7 +94,7 @@ impl DrawIntersection { // Returns the (octagon, pole) if there's room to draw it. pub fn stop_sign_geom(ss: &RoadWithStopSign, map: &Map) -> Option<(Polygon, Polygon)> { let trim_back = Distance::meters(0.1); - let rightmost = &map.get_l(*ss.travel_lanes.last().unwrap()).lane_center_pts; + let rightmost = &map.get_l(ss.rightmost_lane).lane_center_pts; // TODO The dream of trimming f64's was to isolate epsilon checks like this... if rightmost.length() - trim_back <= EPSILON_DIST { // TODO warn diff --git a/map_model/src/map.rs b/map_model/src/map.rs index 95a76c8cad..a75024fed7 100644 --- a/map_model/src/map.rs +++ b/map_model/src/map.rs @@ -109,7 +109,7 @@ impl Map { for i in &m.intersections { match i.intersection_type { IntersectionType::StopSign => { - stop_signs.insert(i.id, ControlStopSign::new(&m, i.id, timer)); + stop_signs.insert(i.id, ControlStopSign::new(&m, i.id)); } IntersectionType::TrafficSignal => { traffic_signals.insert(i.id, ControlTrafficSignal::new(&m, i.id, timer)); @@ -539,7 +539,7 @@ impl Map { pub fn is_turn_allowed(&self, t: TurnID) -> bool { if let Some(ss) = self.stop_signs.get(&t.parent) { - ss.get_priority(t) != TurnPriority::Banned + ss.get_priority(t, self) != TurnPriority::Banned } else if let Some(ts) = self.traffic_signals.get(&t.parent) { ts.phases .iter() @@ -684,7 +684,7 @@ impl Map { } for id in self.edits.stop_sign_overrides.keys() { if !new_edits.stop_sign_overrides.contains_key(id) { - all_stop_sign_edits.insert(*id, ControlStopSign::new(self, *id, timer)); + all_stop_sign_edits.insert(*id, ControlStopSign::new(self, *id)); } } for id in self.edits.traffic_signal_overrides.keys() { @@ -812,8 +812,7 @@ impl Map { // Do this before applying intersection policy edits. match i.intersection_type { IntersectionType::StopSign => { - self.stop_signs - .insert(id, ControlStopSign::new(self, id, timer)); + self.stop_signs.insert(id, ControlStopSign::new(self, id)); } IntersectionType::TrafficSignal => { self.traffic_signals @@ -899,7 +898,7 @@ impl Map { let mut delete_stop_signs = Vec::new(); for (id, ss) in &self.edits.stop_sign_overrides { - if *ss == ControlStopSign::new(self, *id, timer) { + if *ss == ControlStopSign::new(self, *id) { delete_stop_signs.push(*id); } } diff --git a/map_model/src/stop_signs.rs b/map_model/src/stop_signs.rs index 39f519415a..1161844cfd 100644 --- a/map_model/src/stop_signs.rs +++ b/map_model/src/stop_signs.rs @@ -1,9 +1,10 @@ use crate::{IntersectionID, LaneID, Map, RoadID, TurnID, TurnPriority, TurnType}; -use abstutil::{deserialize_btreemap, serialize_btreemap, Error, Timer, Warn}; +use abstutil::{deserialize_btreemap, serialize_btreemap}; use serde_derive::{Deserialize, Serialize}; -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap}; -// TODO Some of these are probably old notes: +// TODO These are old notes, they don't reflect current reality. But some of the ideas here should +// be implemented, so keeping them... // 1) Pedestrians always have right-of-way. (for now -- should be toggleable later) // 2) Incoming roads without a stop sign have priority over roads with a sign. // 3) Agents with a stop sign have to actually wait some amount of time before starting the turn. @@ -29,12 +30,6 @@ use std::collections::{BTreeMap, HashMap, HashSet}; #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] pub struct ControlStopSign { pub id: IntersectionID, - // Turns may be present here as Banned. - #[serde( - serialize_with = "serialize_btreemap", - deserialize_with = "deserialize_btreemap" - )] - pub turns: BTreeMap, #[serde( serialize_with = "serialize_btreemap", deserialize_with = "deserialize_btreemap" @@ -44,15 +39,16 @@ pub struct ControlStopSign { #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] pub struct RoadWithStopSign { - pub travel_lanes: Vec, - pub enabled: bool, + pub rightmost_lane: LaneID, + pub must_stop: bool, } impl ControlStopSign { - pub fn new(map: &Map, id: IntersectionID, timer: &mut Timer) -> ControlStopSign { - let mut ss = smart_assignment(map, id).get(timer); - ss.validate(map).unwrap().get(timer); - + pub fn new(map: &Map, id: IntersectionID) -> ControlStopSign { + let mut ss = ControlStopSign { + id, + roads: BTreeMap::new(), + }; for r in &map.get_i(id).roads { let travel_lanes: Vec = map .get_r(*r) @@ -70,248 +66,57 @@ impl ControlStopSign { ss.roads.insert( *r, RoadWithStopSign { - travel_lanes, - enabled: false, + rightmost_lane: *travel_lanes.last().unwrap(), + must_stop: false, }, ); } } - ss.recalculate_stop_signs(map); + if ss.roads.len() <= 2 { + // Degenerate roads and deadends don't need any stop signs. + return ss; + } + + // What's the rank of each road? + let mut rank: HashMap = HashMap::new(); + for r in ss.roads.keys() { + rank.insert(*r, map.get_r(*r).get_rank()); + } + let mut ranks: Vec = rank.values().cloned().collect(); + ranks.sort(); + ranks.dedup(); + // Highest rank is first + ranks.reverse(); + + // If all roads have the same rank, all-way stop. Otherwise, everything stops except the + // highest-priority roads. + for (r, cfg) in ss.roads.iter_mut() { + if ranks.len() == 1 || rank[r] != ranks[0] { + cfg.must_stop = true; + } + } ss } - pub fn get_priority(&self, turn: TurnID) -> TurnPriority { - self.turns[&turn] - } - - // TODO rm - pub fn could_be_priority_turn(&self, id: TurnID, map: &Map) -> bool { - for (t, pri) in &self.turns { - if *pri == TurnPriority::Protected && map.get_t(id).conflicts_with(map.get_t(*t)) { - return false; - } - } - true - } - - pub fn lane_has_stop_sign(&self, lane: LaneID) -> bool { - for ss in self.roads.values() { - if ss.travel_lanes.contains(&lane) { - return ss.enabled; - } - } - false - } - - // Returns both errors and warnings. - fn validate(&self, map: &Map) -> Result, Error> { - let mut warnings = Vec::new(); - - // Does the assignment cover the correct set of turns? - let all_turns = &map.get_i(self.id).turns; - // TODO Panic after stabilizing merged intersection issues. - if self.turns.len() != all_turns.len() { - warnings.push(format!( - "Stop sign for {} has {} turns but should have {}", - self.id, - self.turns.len(), - all_turns.len() - )); - } - for t in all_turns { - if !self.turns.contains_key(t) { - warnings.push(format!("Stop sign for {} is missing {}", self.id, t)); - } - // Are all of the SharedSidewalkCorner prioritized? - if map.get_t(*t).turn_type == TurnType::SharedSidewalkCorner { - assert_eq!(self.turns[t], TurnPriority::Protected); - } - } - - // Do any of the priority turns conflict? - let priority_turns: Vec = self - .turns - .iter() - .filter_map(|(turn, pri)| { - if *pri == TurnPriority::Protected { - Some(*turn) + // TODO Or cache + pub fn get_priority(&self, turn: TurnID, map: &Map) -> TurnPriority { + match map.get_t(turn).turn_type { + TurnType::SharedSidewalkCorner => TurnPriority::Protected, + // TODO This actually feels like a policy bit that should be flippable. + TurnType::Crosswalk => TurnPriority::Protected, + _ => { + if self.roads[&map.get_l(turn.src).parent].must_stop { + TurnPriority::Yield } else { - None - } - }) - .collect(); - for t1 in &priority_turns { - for t2 in &priority_turns { - if map.get_t(*t1).conflicts_with(map.get_t(*t2)) { - return Err(Error::new(format!( - "Stop sign has conflicting priority turns {:?} and {:?}", - t1, t2 - ))); + TurnPriority::Protected } } } - - Ok(Warn::empty_warnings(warnings)) } - pub fn change(&mut self, t: TurnID, pri: TurnPriority, map: &Map) { - let turn = map.get_t(t); - self.turns.insert(t, pri); - if turn.turn_type == TurnType::Crosswalk { - for id in &turn.other_crosswalk_ids { - self.turns.insert(*id, pri); - } - } - self.recalculate_stop_signs(map); - } - - // TODO Actually want to recalculate individual turn priorities for everything when anything - // changes! I think the base data model needs to become 'roads' with some Banned overrides. - pub fn flip_sign(&mut self, r: RoadID, map: &Map) { + pub fn flip_sign(&mut self, r: RoadID) { let ss = self.roads.get_mut(&r).unwrap(); - ss.enabled = !ss.enabled; - let new_pri = if ss.enabled { - TurnPriority::Yield - } else { - TurnPriority::Protected - }; - for l in ss.travel_lanes.clone() { - for (turn, _) in map.get_next_turns_and_lanes(l, self.id) { - self.turns.insert(turn.id, new_pri); - - // Upgrade some turns to priority - if new_pri == TurnPriority::Yield && self.could_be_priority_turn(turn.id, map) { - match turn.turn_type { - TurnType::Straight | TurnType::Right | TurnType::Crosswalk => { - self.turns.insert(turn.id, TurnPriority::Protected); - } - _ => {} - } - } - } - } - } - - fn recalculate_stop_signs(&mut self, map: &Map) { - for ss in self.roads.values_mut() { - ss.enabled = false; - for l in &ss.travel_lanes { - for (turn, _) in map.get_next_turns_and_lanes(*l, self.id) { - match self.turns[&turn.id] { - TurnPriority::Yield | TurnPriority::Banned => { - ss.enabled = true; - } - _ => {} - } - } - } - } + ss.must_stop = !ss.must_stop; } } - -fn smart_assignment(map: &Map, id: IntersectionID) -> Warn { - // Count the number of roads with incoming lanes to determine degenerate/deadends. Might have - // one incoming road to two outgoing. Don't count sidewalks as incoming; crosswalks always - // yield anyway. - let mut incoming_roads: HashSet = HashSet::new(); - for l in &map.get_i(id).incoming_lanes { - if map.get_l(*l).lane_type.is_for_moving_vehicles() { - incoming_roads.insert(map.get_l(*l).parent); - } - } - if incoming_roads.len() <= 2 { - return for_degenerate_and_deadend(map, id); - } - - // Higher numbers are higher rank roads - let mut rank_per_incoming_lane: HashMap = HashMap::new(); - let mut ranks: HashSet = HashSet::new(); - let mut highest_rank = 0; - // TODO should just be incoming, but because of weirdness with sidewalks... - for l in map - .get_i(id) - .incoming_lanes - .iter() - .chain(map.get_i(id).outgoing_lanes.iter()) - { - let rank = map.get_parent(*l).get_rank(); - rank_per_incoming_lane.insert(*l, rank); - highest_rank = highest_rank.max(rank); - ranks.insert(rank); - } - if ranks.len() == 1 { - return Warn::ok(all_way_stop(map, id)); - } - - let mut ss = ControlStopSign { - id, - turns: BTreeMap::new(), - roads: BTreeMap::new(), - }; - for t in &map.get_i(id).turns { - if map.get_t(*t).turn_type == TurnType::SharedSidewalkCorner { - ss.turns.insert(*t, TurnPriority::Protected); - } else if rank_per_incoming_lane[&t.src] == highest_rank { - // If it's the highest rank road, prioritize main turns and make others yield. - ss.turns.insert(*t, TurnPriority::Yield); - if ss.could_be_priority_turn(*t, map) { - match map.get_t(*t).turn_type { - TurnType::Straight | TurnType::Right | TurnType::Crosswalk => { - ss.turns.insert(*t, TurnPriority::Protected); - } - _ => {} - } - } - } else { - // Lower rank roads have to stop. - ss.turns.insert(*t, TurnPriority::Yield); - } - } - Warn::ok(ss) -} - -fn all_way_stop(map: &Map, id: IntersectionID) -> ControlStopSign { - let mut ss = ControlStopSign { - id, - turns: BTreeMap::new(), - roads: BTreeMap::new(), - }; - for t in &map.get_i(id).turns { - if map.get_t(*t).turn_type == TurnType::SharedSidewalkCorner { - ss.turns.insert(*t, TurnPriority::Protected); - } else { - ss.turns.insert(*t, TurnPriority::Yield); - } - } - ss -} - -fn for_degenerate_and_deadend(map: &Map, id: IntersectionID) -> Warn { - let mut ss = ControlStopSign { - id, - turns: BTreeMap::new(), - roads: BTreeMap::new(), - }; - for t in &map.get_i(id).turns { - // Only the crosswalks should conflict with other turns. - let priority = match map.get_t(*t).turn_type { - TurnType::Crosswalk => TurnPriority::Yield, - TurnType::LaneChangeLeft | TurnType::LaneChangeRight => TurnPriority::Yield, - _ => TurnPriority::Protected, - }; - ss.turns.insert(*t, priority); - } - - // Due to a few observed issues (multiple driving lanes road (a temporary issue) and bad - // intersection geometry), sometimes more turns conflict than really should. For now, just - // detect and fallback to an all-way stop. - if let Err(err) = ss.validate(map) { - return Warn::warn( - all_way_stop(map, id), - format!("Giving up on for_degenerate_and_deadend({}): {}", id, err), - ); - } - - Warn::ok(ss) -} diff --git a/map_model/src/turn.rs b/map_model/src/turn.rs index 6743020b8a..7c3af86616 100644 --- a/map_model/src/turn.rs +++ b/map_model/src/turn.rs @@ -51,7 +51,8 @@ impl TurnType { #[derive(Serialize, Deserialize, Debug, PartialEq, Clone, Copy, PartialOrd)] pub enum TurnPriority { - // Can't do this turn at all! + // For stop signs: Can't currently specify this! + // For traffic signals: Can't do this turn right now. Banned, // For stop signs: cars have to stop before doing this turn, and are accepted with the lowest priority. // For traffic signals: Cars can do this immediately if there are no previously accepted conflicting turns. diff --git a/sim/src/mechanics/intersection.rs b/sim/src/mechanics/intersection.rs index 959a50a45a..eaab5e411a 100644 --- a/sim/src/mechanics/intersection.rs +++ b/sim/src/mechanics/intersection.rs @@ -250,7 +250,7 @@ impl State { return false; } - let our_priority = sign.turns[&req.turn]; + let our_priority = sign.get_priority(req.turn, map); assert!(our_priority != TurnPriority::Banned); let our_time = self.waiting[req];