a-b-street/abstreet
1
1
Fork 0
mirror of https://github.com/a-b-street/abstreet.git synced 2024-12-28 08:53:26 +03:00
Code Issues Projects Releases Wiki Activity

Some useful debugging / correctness steps towards more robust blockfinding. [rebuild] [release]

Browse Source 
I have another branch that handles roads without sidewalks on one side
-- it helps in some cases, but regresses in others, so not merging it
yet. But taking a smaller step and bringing in some stricter common
endpoint logic from there.

Bringing in some useful intermediate changes
This commit is contained in:
Dustin Carlino 2022-01-09 11:07:10 +00:00
parent e1d0604718
commit 8ff4c41ea1
8 changed files with 105 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
game/src/edit/zones.rs
View File

@ -4,7 +4,7 @@ use enumset::EnumSet;
use maplit::btreeset;
use map_gui::tools::ColorDiscrete;
use map_model::{AccessRestrictions, PathConstraints, RoadID};
use map_model::{AccessRestrictions, CommonEndpoint, PathConstraints, RoadID};
use sim::TripMode;
use widgetry::mapspace::ToggleZoomed;
use widgetry::{
@ -179,7 +179,9 @@ fn draw_zone(ctx: &mut EventCtx, app: &App, members: &BTreeSet<RoadID>) -> (Togg
colorer.add_r(r.id, "restricted road");
for next in map.get_next_roads(r.id) {
if !members.contains(&next) {
colorer.add_i(r.common_endpt(map.get_r(next)), "entrance/exit");
if let CommonEndpoint::One(i) = r.common_endpoint(map.get_r(next)) {
colorer.add_i(i, "entrance/exit");
}
}
}
}

12
geom/src/ring.rs
View File

@ -1,5 +1,6 @@
use std::collections::HashSet;
use std::fmt;
use std::fmt::Write;
use anyhow::Result;
use serde::{Deserialize, Serialize};
@ -249,6 +250,17 @@ impl Ring {
// Do they match up?
orig != sorted
}
/// Print the coordinates of this ring as a `geo::LineString` for easy bug reports
pub fn as_geo_linestring(&self) -> String {
let mut output = String::new();
writeln!(output, "let line_string = geo_types::line_string![").unwrap();
for pt in &self.pts {
writeln!(output, " (x: {}, y: {}),", pt.x(), pt.y()).unwrap();
}
writeln!(output, "];").unwrap();
output
}
}
impl fmt::Display for Ring {

4
map_model/src/lib.rs
View File

@ -49,8 +49,8 @@ pub use crate::objects::building::{
};
pub use crate::objects::intersection::{Intersection, IntersectionID, IntersectionType};
pub use crate::objects::lane::{
BufferType, Lane, LaneID, LaneSpec, LaneType, NORMAL_LANE_THICKNESS, PARKING_LOT_SPOT_LENGTH,
SIDEWALK_THICKNESS,
BufferType, CommonEndpoint, Lane, LaneID, LaneSpec, LaneType, NORMAL_LANE_THICKNESS,
PARKING_LOT_SPOT_LENGTH, SIDEWALK_THICKNESS,
};
pub use crate::objects::movement::{CompressedMovementID, Movement, MovementID};
pub use crate::objects::parking_lot::{ParkingLot, ParkingLotID};

17
map_model/src/map.rs
View File

@ -12,12 +12,12 @@ use geom::{Bounds, Distance, Duration, GPSBounds, Polygon, Pt2D, Ring, Time};
use crate::raw::{OriginalRoad, RawMap};
use crate::{
osm, Area, AreaID, AreaType, Building, BuildingID, BuildingType, CompressedMovementID,
ControlStopSign, ControlTrafficSignal, DirectedRoadID, Direction, Intersection, IntersectionID,
Lane, LaneID, LaneType, Map, MapEdits, Movement, MovementID, OffstreetParking, ParkingLot,
ParkingLotID, Path, PathConstraints, PathRequest, PathV2, Pathfinder, Position, Road, RoadID,
RoutingParams, TransitRoute, TransitRouteID, TransitStop, TransitStopID, Turn, TurnID,
TurnType, Zone,
osm, Area, AreaID, AreaType, Building, BuildingID, BuildingType, CommonEndpoint,
CompressedMovementID, ControlStopSign, ControlTrafficSignal, DirectedRoadID, Direction,
Intersection, IntersectionID, Lane, LaneID, LaneType, Map, MapEdits, Movement, MovementID,
OffstreetParking, ParkingLot, ParkingLotID, Path, PathConstraints, PathRequest, PathV2,
Pathfinder, Position, Road, RoadID, RoutingParams, TransitRoute, TransitRouteID, TransitStop,
TransitStopID, Turn, TurnID, TurnType, Zone,
};
#[derive(Clone, Debug, Serialize, Deserialize)]
@ -871,7 +871,10 @@ impl Map {
let to = self.get_r(to);
turn_type == unprotected_turn_type
&& from.get_detailed_rank() < to.get_detailed_rank()
&& self.get_i(from.common_endpt(to)).is_stop_sign()
&& match from.common_endpoint(to) {
CommonEndpoint::One(i) => self.get_i(i).is_stop_sign(),
_ => false,
}
}
/// Modifies the map in-place, removing parts not essential for the bike network tool.

63
map_model/src/objects/block.rs
View File

@ -6,7 +6,7 @@ use anyhow::Result;
use abstutil::wraparound_get;
use geom::{Polygon, Pt2D, Ring};
use crate::{Direction, LaneID, Map, RoadID, RoadSideID, SideOfRoad};
use crate::{CommonEndpoint, Direction, LaneID, Map, RoadID, RoadSideID, SideOfRoad};
/// A block is defined by a perimeter that traces along the sides of roads. Inside the perimeter,
/// the block may contain buildings and interior roads. In the simple case, a block represents a
@ -127,6 +127,7 @@ impl Perimeter {
///
/// Note this may modify both perimeters and still return `false`. The modification is just to
/// rotate the order of the road loop; this doesn't logically change the perimeter.
// TODO Would it be cleaner to return a Result here and always restore the invariant?
fn try_to_merge(&mut self, other: &mut Perimeter, debug_failures: bool) -> bool {
self.undo_invariant();
other.undo_invariant();
@ -399,34 +400,10 @@ impl Perimeter {
}
pub fn to_block(self, map: &Map) -> Result<Block> {
Block::from_perimeter(map, self)
}
/// Does this perimeter completely enclose the other?
pub fn contains(&self, other: &Perimeter) -> bool {
other
.roads
.iter()
.all(|id| self.interior.contains(&id.road) || self.roads.contains(id))
}
}
impl fmt::Debug for Perimeter {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
writeln!(f, "Perimeter:")?;
for id in &self.roads {
writeln!(f, "- {:?} of {}", id.side, id.road)?;
}
Ok(())
}
}
impl Block {
fn from_perimeter(map: &Map, perimeter: Perimeter) -> Result<Block> {
// Trace along the perimeter and build the polygon
let mut pts: Vec<Pt2D> = Vec::new();
let mut first_intersection = None;
for pair in perimeter.roads.windows(2) {
for pair in self.roads.windows(2) {
let lane1 = pair[0].get_outermost_lane(map);
let road1 = map.get_parent(lane1.id);
let lane2 = pair[1].get_outermost_lane(map);
@ -445,9 +422,9 @@ impl Block {
// We're doubling back at a dead-end. Always follow the orientation of the lane.
true
} else {
match lane1.common_endpt(lane2) {
Some(i) => i == lane1.dst_i,
None => {
match lane1.common_endpoint(lane2) {
CommonEndpoint::One(i) => i == lane1.dst_i,
CommonEndpoint::Both => {
// Two different roads link the same two intersections. I don't think we
// can decide the order of points other than seeing which endpoint is
// closest to our last point.
@ -458,6 +435,11 @@ impl Block {
true
}
}
CommonEndpoint::None => bail!(
"{} and {} don't share a common endpoint",
lane1.id,
lane2.id
),
}
};
if !keep_lane_orientation {
@ -515,6 +497,27 @@ impl Block {
// in the map geometry that should be properly fixed.
//let polygon = Polygon::buggy_new(pts);
Ok(Block { perimeter, polygon })
Ok(Block {
perimeter: self,
polygon,
})
}
/// Does this perimeter completely enclose the other?
pub fn contains(&self, other: &Perimeter) -> bool {
other
.roads
.iter()
.all(|id| self.interior.contains(&id.road) || self.roads.contains(id))
}
}
impl fmt::Debug for Perimeter {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
writeln!(f, "Perimeter:")?;
for id in &self.roads {
writeln!(f, "- {:?} of {}", id.side, id.road)?;
}
Ok(())
}
}

52
map_model/src/objects/lane.rs
View File

@ -368,6 +368,7 @@ impl Lane {
/// This is just based on typical driving sides. Bidirectional or contraflow cycletracks as
/// input may produce weird results.
// TODO Reconsider this -- it's confusing
pub fn get_nearest_side_of_road(&self, map: &Map) -> RoadSideID {
let side = match (self.dir, map.get_config().driving_side) {
(Direction::Fwd, DrivingSide::Right) => SideOfRoad::Right,
@ -450,24 +451,8 @@ impl Lane {
Some(part.split(';').flat_map(parse_turn_type_from_osm).collect())
}
/// If the lanes share one endpoint, returns it. If they share two -- because they belong to
/// the same road or there are two different roads connecting the same pair of intersections --
/// then return `None`. If they share no common endpoint, panic.
/// (This is a weird API, really should be an enum with 3 cases)
pub fn common_endpt(&self, other: &Lane) -> Option<IntersectionID> {
#![allow(clippy::suspicious_operation_groupings)]
let src = self.src_i == other.src_i || self.src_i == other.dst_i;
let dst = self.dst_i == other.src_i || self.dst_i == other.dst_i;
if src && dst {
return None;
}
if src {
return Some(self.src_i);
}
if dst {
return Some(self.dst_i);
}
panic!("{} and {} have no common_endpt", self.id, other.id);
pub fn common_endpoint(&self, other: &Lane) -> CommonEndpoint {
CommonEndpoint::new((self.src_i, self.dst_i), (other.src_i, other.dst_i))
}
pub fn get_thick_polygon(&self) -> Polygon {
@ -475,6 +460,37 @@ impl Lane {
}
}
pub enum CommonEndpoint {
/// Two lanes/roads share one endpoint
One(IntersectionID),
/// Two lanes/roads share both endpoints, because they both belong to the same road, or there
/// are two different roads connecting the same pair of intersections
Both,
/// Two lanes/roads don't have any common endpoints
None,
}
impl CommonEndpoint {
pub fn new(
obj1: (IntersectionID, IntersectionID),
obj2: (IntersectionID, IntersectionID),
) -> CommonEndpoint {
#![allow(clippy::suspicious_operation_groupings)]
let src = obj1.0 == obj2.0 || obj1.0 == obj2.1;
let dst = obj1.1 == obj2.0 || obj1.1 == obj2.1;
if src && dst {
return CommonEndpoint::Both;
}
if src {
return CommonEndpoint::One(obj1.0);
}
if dst {
return CommonEndpoint::One(obj1.1);
}
CommonEndpoint::None
}
}
impl LaneSpec {
/// For a given lane type, returns some likely widths. This may depend on the type of the road,
/// so the OSM tags are also passed in. The first value returned will be used as a default.

18
map_model/src/objects/road.rs
View File

@ -10,8 +10,8 @@ use geom::{Distance, PolyLine, Polygon, Speed};
use crate::raw::{OriginalRoad, RestrictionType};
use crate::{
osm, AccessRestrictions, DrivingSide, IntersectionID, Lane, LaneID, LaneSpec, LaneType, Map,
PathConstraints, TransitStopID, Zone,
osm, AccessRestrictions, CommonEndpoint, DrivingSide, IntersectionID, Lane, LaneID, LaneSpec,
LaneType, Map, PathConstraints, TransitStopID, Zone,
};
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, PartialOrd, Ord, Serialize, Deserialize)]
@ -444,21 +444,13 @@ impl Road {
bike
}
/// Returns the common intersection between two roads, panicking if they're not adjacent
// TODO Doesn't handle two roads between the same pair of intersections
pub fn common_endpt(&self, other: &Road) -> IntersectionID {
#![allow(clippy::suspicious_operation_groupings)] // false positive
if self.src_i == other.src_i || self.src_i == other.dst_i {
self.src_i
} else if self.dst_i == other.src_i || self.dst_i == other.dst_i {
self.dst_i
} else {
panic!("{} and {} don't share an endpoint", self.id, other.id);
}
pub fn common_endpoint(&self, other: &Road) -> CommonEndpoint {
CommonEndpoint::new((self.src_i, self.dst_i), (other.src_i, other.dst_i))
}
/// Returns the other intersection of this road, panicking if this road doesn't connect to the
/// input
/// TODO This should use CommonEndpoint
pub fn other_endpt(&self, i: IntersectionID) -> IntersectionID {
if self.src_i == i {
self.dst_i

7
map_model/src/objects/zone.rs
View File

@ -10,7 +10,7 @@ use std::collections::BTreeSet;
use enumset::EnumSet;
use serde::{Deserialize, Serialize};
use crate::{IntersectionID, Map, PathConstraints, RoadID};
use crate::{CommonEndpoint, IntersectionID, Map, PathConstraints, RoadID};
#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)]
pub struct AccessRestrictions {
@ -76,7 +76,10 @@ fn floodfill(map: &Map, start: RoadID) -> Zone {
if r.access_restrictions == match_constraints && merge_zones {
queue.push(r.id);
} else {
borders.insert(map.get_r(current).common_endpt(r));
// TODO Handle other cases
if let CommonEndpoint::One(i) = map.get_r(current).common_endpoint(r) {
borders.insert(i);
}
}
}
}