Road direction and views

[dabreegster]

Unorganized...

Views

Roads have a fixed frame-of-reference oriented the way OSM defines them right now. It'd be useful to have a view in the opposite direction. The view could help reason with front/back and left/right of the road. It could be implemented maybe as individual helper methods, or if we want to do something like road.view_incoming_to(intersection) and then ask a bunch of queries, it could be something iterator-like: struct View<'a> { road: &'a Road }. There are many parts of the code, including in the movements render PR, that get the "first" line segment close to an intersection and force it to point one way or another. Reasoning about all of the roads pointed to/from an intersection would really simplify things.

Finish getting rid of InitialMap

#2. At a glance, I think Road could start storing a PolyLine today instead of points. And it should hopefully not be much more work to store trimmed_center_points, trim_distance_start, trim_distance_end and in intersection, the polygon. There were subtle implications somewhere, I don't remember, but it'd be really helpful to finally push this through.

It'll mean the structs internally get more stateful, but that's fine internally. We can document this kind of thing in CONTRIBUTING.md

IDs

To start, the Road struct should embed its ID. This'll let call-sites avoid working with awkward pairs. And it's necessary for any kind of view / frame-of-reference reasoning, to know the intersection endpoints.

For naming consistency, we should have RoadID and IntersectionID. These should be opaque. Because we merge roads and intersections, there's not a 1-to-1 mapping with the original OSM IDs. We can just store a list of OriginalRoad and osm::NodeID instead.

OSM tags

On that note, osm_tags probably don't belong on Road. As we merge roads, they become nonsensical. In osm2lanes, the returned structure has nice normalized name, highway rank, speed limit, etc. We could move that direction. Or in the short term, just remove osm_tags, and make streets_reader::Document visible in the API. Callers can use get back both a StreetNetwork and Document and do lookups of whatever they need, and the burden is on them to handle roads with multiple OSM ways.

Turn restrictions

Preserving turn restrictions across transformations is already a nightmare:

osm2streets/osm2streets/src/transform/merge_short_road.rs

Line 44 in 6ff55ed

// TODO Fix up turn restrictions. Many cases:

In the spirit of the above, we could try a different approach. Don't store any road-to-road turn restrictions in Road. Keep around some untransformed form of the original OSM graph. When we generate movements, refer back to it by asking queries like osm_graph.is_turn_allowed(original_road1, original_road2). That code will have to sort out what multiple way IDs means.

Lane-level turn restrictions probably belong in LaneSpec.

Placement tags

perth_stretched_lights is an example where placement tags would help with parallelish roads that overlap to start with. #6 is probably low-hanging fruit to implement now.

[dabreegster]

Ben, I can aim to get a PR out for IDs and InitialMap the rest of this week. Could you maybe look at placement tags?

[BudgieInWA]

Yes, I will work on placement tags. Actually getting my hands dirty in the nitty gritties while working on the movements PR, and having this chat, has given me a much better understanding of how everything fits together. I've got more confidence in making changes all over the place and I've worked on placement related stuff before, so I'm well positioned to tackle it!

[dabreegster]

I started on the opaque ID journey, but realized there's a big choice in there. Do we want RoadID(usize) or RoadID(usize, IntersectionID, IntersectionID)? The latter is like what we have now. Benefit is it's convenient to know the two endpoints from the ID; tons of transform logic currently passes that around. Road will of course have both of the intersection IDs listed, but I wonder if holding onto &Roads in the middle of transforms will get ugly with borrow checking.

A reason to just do RoadID(usize) is

osm2streets/osm2streets/src/transform/merge_short_road.rs

Line 103 in 2c26a2f

// Fix up all roads connected to i2. Delete them and create a new copy; the ID changes,

. When we merge, we have to do all this crazy work to rewrite existing RoadIDs everywhere. If it's opaque, then we just update i1 or i2 in a Road.

I'm tempted to attempt RoadID(usize) and maybe introduce a "only use this internally when necessary" RoadSegment(RoadID, IntersectionID, IntersectionID) helper for convenience in some of those transforms

[BudgieInWA]

I'm tempted to attempt RoadID(usize) and maybe introduce a "only use this internally when necessary" RoadSegment(RoadID, IntersectionID, IntersectionID) helper for convenience in some of those transforms

Let's do this! I didn't this line, and was going to suggest the same thing. Rewriting RoadIDs everywhere sounds like a horrible time.

[BudgieInWA]

Road directions and Views

I've been thinking about the directed road view idea while reviewing the recent refactors, and what seems practical to solve the problem of viewing a set of roads in a specific orientation. There are a couple of pieces of the path towards implementing a directed road View that would already give us 80% of the benefit.

enum WhichEnd { Start, End }
impl WhichEnd {
  fn opposite(&self) -> Self { ... }
}

struct DirectedRoadID {
  id: RoadID,
  dir: Direction,
}
impl DirectedRoadID {
  fn start(&self) -> WhichEnd { ... } // which end is the "start" from this frame of reference?
}

impl Road {
  fn trimmed_end_point(end: WhichEnd) -> Pt2D { ... }
  fn trimmed_end_segment(end: WhichEnd) -> Line { ... }
  fn trimmed_end_cross_section(end: WhichEnd) -> Line { ... }
  fn center_line_from_end(end: WhichEnd) -> PolyLine { ... }
  fn can_drive_from_end(end: WhichEnd) -> bool { ... }
  //...
}

I think having direction dependent methods on Road take a "which end" argument and doing the direction switching themselves gives us what we want with probably the least implementation overhead. If the DirectedRoadID gives us the "which end" value (and that value has an opposite method) and we can pass it straight into the Road methods, then we don't need a View.

fn basic_intersection_boundary(streets, intersection) -> Ring {
  Ring::from_points(
    intersection
      .roads_outwards
      .map(|r| streets.roads[r.id].trimmed_end_cross_section(r.start()))
      .map(|cross_section| cross_section.points())
      .flatten()
  )
}

I'm gonna leave these examples undocumented/unexplained as a sort of meditation on this solution (not only out of lazyness). If its impossible to make sense of it, it might not be as useful as I think...

[dabreegster]

This all makes sense to me, and the intersection boundary logic becomes eerily simple! IIUC, this approach will have some branching logic in all of those methods on Road that take WhichEnd to handle the 2 cases. But it'll be pretty straightforward logic, so is certainly a big step forwards. The full-blown iterator view idea might be harder, or require us to think about lifetimes, or think about how to expose a reversed PolyLine, or something like that. No need to jump straight to that yet.

[dabreegster]

Maybe the only clarification: is cross_section the bit where a road polygon "meets" the intersection polygon? So it's a line tangent to the start/end line's endpt?

[BudgieInWA]

This all makes sense to me, and the intersection boundary logic becomes eerily simple!

Yep! A good boundary algorithm should be more complicated than that example, but the complications should represent more nuanced boundary ideas, not juggling start/end forward/backward ideas!

IIUC, this approach will have some branching logic in all of those methods on Road that take WhichEnd to handle the 2 cases. But it'll be pretty straightforward logic.

Yes, exactly. This is the inherent complexity that we need to represent somewhere, and this is a consistent way to do it. I predict it will be so straight forward as to be boring, once the symmetry is revealed by the consistent API.

Maybe the only clarification: is cross_section the bit where a road polygon "meets" the intersection polygon? So it's a line tangent to the start/end line's endpt?

Yes, that's the concept I am trying to name there. (Hopefully it can non-tangent in the future too :P #95)

[dabreegster]

We've pretty much implemented everything captured here except for the view API, so renaming