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

Start calculating Movements and Conflicts #96

Merged
merged 30 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
e7919f6
Start calculating movements and conflicts
BudgieInWA Sep 22, 2022
b5110af
Store movements in Intersections and add a debug layer showing them
BudgieInWA Oct 20, 2022
fcdb296
untested: Calculate if movements using the same road in different dir…
BudgieInWA Nov 5, 2022
76d32e0
Take into account the driving direction of the roads
BudgieInWA Nov 7, 2022
fa037f3
Clear the list of movements when modifying an intersection
BudgieInWA Nov 7, 2022
93742f4
Use terser syntax
BudgieInWA Nov 7, 2022
670a890
Improve comment about using indexes to refer to roads
BudgieInWA Nov 7, 2022
b8b9f21
Merge branch 'main' into movement-conflicts
BudgieInWA Nov 8, 2022
aee6298
Merge branch 'main' into movement-conflicts
BudgieInWA Nov 8, 2022
01347e6
Extract the "can drive into/out of" logic into a fn
BudgieInWA Nov 9, 2022
d872777
Calculate movements for Connections (instead of assuming)
BudgieInWA Nov 9, 2022
b75b2f2
Fix an Uncontested case with bidi roads in calc_conflict
BudgieInWA Nov 9, 2022
156c79a
Offset arrow start/end points from the center of bidi roads
BudgieInWA Nov 9, 2022
eaf8713
Document ConflictType and conflict_level
BudgieInWA Nov 9, 2022
1502ce8
Accept test output representing the new MultiConnection Merge/Diverge…
BudgieInWA Nov 9, 2022
e9cbad0
Pass around (&OriginalRoad, &Road) tuples instead of two lists
BudgieInWA Nov 9, 2022
86432b4
Pre-filter non-drivable roads.
BudgieInWA Nov 9, 2022
e58eaa9
tmp: Add todos
BudgieInWA Nov 11, 2022
2c13409
Merge branch 'main' into movement-conflicts
BudgieInWA Nov 14, 2022
24845f3
Fix debug_movements_geojson now that InitialMap has gone away
BudgieInWA Nov 14, 2022
ad8e7e8
Make use of embedded road ids instead of passing around (id, road) pairs
BudgieInWA Nov 14, 2022
d46babd
Represent movements stored in `Intersection` as roads ids instead or …
BudgieInWA Nov 14, 2022
427aaea
Recalculate intersection movements and complexity after inserting or …
BudgieInWA Nov 14, 2022
479d636
cleanup use statements and old FIXME
BudgieInWA Nov 14, 2022
19ab938
Introduce StreetNetwork::recalculate_movements(self, i) to hold repea…
BudgieInWA Nov 15, 2022
34eecc2
Document IntersectionComplexity and note its awkwardness
BudgieInWA Nov 15, 2022
1bca406
Comment arrow offset variable
BudgieInWA Nov 15, 2022
beca662
Don't bother calculating all conflicts, and remove unneeded assertion…
BudgieInWA Nov 15, 2022
05def90
Fix comment typos
BudgieInWA Nov 15, 2022
ffe0e9d
rust style improvements
BudgieInWA Nov 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions osm2streets-js/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ impl JsStreetNetwork {
pub fn debug_clockwise_ordering_geojson(&self) -> String {
self.inner.debug_clockwise_ordering_geojson().unwrap()
}

#[wasm_bindgen(js_name = debugMovementsGeojson)]
pub fn debug_movements_geojson(&self) -> String {
self.inner.debug_movements_geojson().unwrap()
}
}

#[wasm_bindgen]
Expand Down
44 changes: 32 additions & 12 deletions osm2streets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ pub use self::lanes::{
};
pub use self::transform::Transformation;
pub use self::types::{
ControlType, DrivingSide, IntersectionComplexity, MapConfig, NamePerLanguage,
ConflictType, ControlType, DrivingSide, IntersectionComplexity, MapConfig, Movement,
NamePerLanguage,
};

mod edit;
Expand Down Expand Up @@ -86,21 +87,21 @@ impl StreetNetwork {
for i in [id.i1, id.i2] {
self.intersections.get_mut(&i).unwrap().roads.push(id);
self.sort_roads(i);
// Recalculate movements and complexity.
BudgieInWA marked this conversation as resolved.
Show resolved Hide resolved
self.recalculate_movements(i);
}
}

pub fn remove_road(&mut self, id: &OriginalRoad) -> Road {
// Since the roads are already sorted, removing doesn't hurt anything
self.intersections
.get_mut(&id.i1)
.unwrap()
.roads
.retain(|r| r != id);
self.intersections
.get_mut(&id.i2)
.unwrap()
.roads
.retain(|r| r != id);
for i in [id.i1, id.i2] {
self.intersections
.get_mut(&i)
.unwrap()
.roads
.retain(|r| r != id);
// Since the roads are already sorted, removing doesn't break the sort.
self.recalculate_movements(i);
}
self.roads.remove(id).unwrap()
}

Expand Down Expand Up @@ -260,6 +261,20 @@ impl StreetNetwork {

intersection.roads = road_centers.into_iter().map(|(r, _, _)| r).collect();
}

/// Recalculate movements, complexity, and conflict_level of an intersection.
fn recalculate_movements(&mut self, i: osm::NodeID) {
let (complexity, conflict_level, movements) =
crate::transform::classify_intersections::guess_complexity(self, &i);
let int = self.intersections.get_mut(&i).unwrap();
int.movements = movements;
int.conflict_level = conflict_level;
// The fact that an intersection represents a road leaving the map bounds is stored in the
// complexity field but guess_complexity ignores that. Make sure we don't overwrite it.
if int.complexity != IntersectionComplexity::MapEdge {
int.complexity = complexity;
}
}
}

// Mutations and supporting queries
Expand Down Expand Up @@ -548,12 +563,14 @@ pub struct Intersection {
/// This will be a placeholder until `Transformation::GenerateIntersectionGeometry` runs.
pub polygon: Polygon,
pub complexity: IntersectionComplexity,
pub conflict_level: ConflictType,
pub control: ControlType,
pub elevation: Distance,

/// All roads connected to this intersection. They may be incoming or outgoing relative to this
/// intersection. They're ordered clockwise aroundd the intersection.
pub roads: Vec<OriginalRoad>,
pub movements: Vec<Movement>,

// 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 @@ -564,16 +581,19 @@ impl Intersection {
id: osm::NodeID,
point: Pt2D,
complexity: IntersectionComplexity,
conflict_level: ConflictType,
control: ControlType,
) -> Self {
Self {
id,
point,
polygon: Polygon::dummy(),
complexity,
conflict_level,
control,
// Filled out later
roads: Vec::new(),
movements: Vec::new(),
elevation: Distance::ZERO,
trim_roads_for_merging: BTreeMap::new(),
}
Expand Down
77 changes: 75 additions & 2 deletions osm2streets/src/render.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use std::collections::BTreeMap;
use std::fs::File;
use std::io::Write;
use std::path::Path;

use anyhow::Result;
use geom::{ArrowCap, Distance, Line, PolyLine};

use crate::{DebugStreets, Direction, LaneType, StreetNetwork};
use crate::{
DebugStreets, Direction, DrivingSide, IntersectionComplexity, LaneType, StreetNetwork,
};

impl StreetNetwork {
/// Saves the plain GeoJSON rendering to a file.
Expand Down Expand Up @@ -45,7 +48,15 @@ impl StreetNetwork {
("osm_node_id", id.0.into()),
(
"complexity",
format!("{:?}", intersection.complexity).into(),
if intersection.complexity == IntersectionComplexity::MultiConnection {
format!(
"{:?} {:?}",
intersection.complexity, intersection.conflict_level
)
} else {
format!("{:?}", intersection.complexity)
}
.into(),
),
("control", format!("{:?}", intersection.control).into()),
("osm_link", id.to_string().into()),
Expand Down Expand Up @@ -240,6 +251,68 @@ impl StreetNetwork {
let output = serde_json::to_string_pretty(&obj)?;
Ok(output)
}

/// For an intersection, show all the movements.
pub fn debug_movements_geojson(&self) -> Result<String> {
// Each movement is represented as an arrow from the end of one road to the beginning of
// another. To stop arrows overlapping, arrows to/from bidirectional roads are offset from
// the center to the appropriate driving side.
let arrow_fwd_offset_dist = if self.config.driving_side == DrivingSide::Right {
Distance::meters(-1.3)
} else {
Distance::meters(1.3)
};

let mut pairs = Vec::new();

for (i, intersection) in &self.intersections {
// Find the points where the arrows should (leave, enter) the roads.
let road_points: BTreeMap<_, _> = intersection
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to implement a method on Road that could calculate these offset endpoints (for a given end and direction). Because a Road doesn't know it's own DrivingSide or dividing line (!!), it's non-trivial, so I'm happy with this currently.

I want to figure out how a Road should figure out it's own driving side, dividing line and HalfRoads pretty soon. Maybe annotating contraflow lanes within LaneSpec would make it easy enough to calculate on demand from lanes_lrt, or maybe the driving side and dividing lane should be stored on the Road as part of the OSM parsing process (where we have the most context).

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree it's annoying how often DrivingSide has to be plumbed around. Just so I understand, by diving line, you mean the point where the direction of travel changes (or the left/right edge on one-way roads)? Looking at pairs of lanes_ltr and direction should work for that, right? And that process exposes cases where there isn't one obvious dividing lane (bidi cycletracks, shared center turn lanes)

Maybe annotating contraflow lanes within LaneSpec

What do you mean by this? They have Direction already

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that is what I mean by "dividing line". If contraflow lanes were annotated as "contraflow" from the beginning, then we wouldn't get confused when trying to find the dividing line. Knowing the driving side might be enough to come up with a consistent definition / detection algorithm though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm open to trying either way. This is sounding like a good thing to unit test individually -- it's probably just a few weird edge cases I'm worrying about

.roads
.iter()
.map(|r| {
let road = &self.roads[r];
let first_road_segment = if road.id.i1 == *i {
road.trimmed_center_line.first_line()
} else {
road.trimmed_center_line.last_line().reversed()
};
// Offset the arrow start/end points if it is bidirectional.
(
r,
if road.oneway_for_driving().is_some() {
(first_road_segment.pt1(), first_road_segment.pt1())
} else {
(
first_road_segment
.shift_either_direction(arrow_fwd_offset_dist)
.pt1(),
first_road_segment
.shift_either_direction(-arrow_fwd_offset_dist)
.pt1(),
)
},
)
})
.collect();
for (a, b) in &intersection.movements {
if a != b {
if let Ok(line) = Line::new(road_points[a].0, road_points[b].1) {
pairs.push((
line.to_polyline()
.make_arrow(Distance::meters(0.5), ArrowCap::Triangle)
.to_geojson(Some(&self.gps_bounds)),
make_props(&[]),
))
}
}
}
}

let obj = geom::geometries_with_properties_to_geojson(pairs);
let output = serde_json::to_string_pretty(&obj)?;
Ok(output)
}
}

impl DebugStreets {
Expand Down
Loading