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

Store lane_specs_ltr on RawRoad, instead of constantly recalculating.… #897

Merged
merged 1 commit into from May 1, 2022

Conversation

dabreegster
Copy link
Collaborator

@dabreegster dabreegster commented Apr 22, 2022

a-b-street/osm2streets#2

@BudgieInWA, I think the transition to storing transformed geometry on RawRoad is nearly here. I made a bunch of cleanup commits first, and this is the first conceptual change -- storing one derived field and updating it as transformations happen. I anticipate the next step will be adding fields to RawRoad and RawIntersection for geometry, and calculating them as the last transformation on a RawMap. The unclear parts will be what happens to preview_intersection, trimmed_road_geometry, etc. Those're mainly used by the map_editor UI, where life is more complicated... the user is modifying the base geometry, and we have to decide what to recalculate and when.

}
}

// If there's a sidewalk on only one side, adjust the true center of the road.
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 think placement tags would be applied somewhere around here

Choose a reason for hiding this comment

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

I think that means that we should store a "true center" relative to the traffic divide line: center_line_points?. Placement tags describe the relationship between the osm geom and the lane markings and our job is to line markings up with each other, so lets pick a place that should always be unique on a road. The line dividing traffic is unique: the painted line on a standard road, down the middle of the median, on the inside edge of a oneway, where you would expect to see oncoming traffic, and down the middle of the "single twoway lane" situation for foot traffic and laneways (which is like where you would draw the dividing line if you were all on bikes).

With that reference, I can for e.g. find details about "the left sidewalk, after two lanes and a curb" without the other side of the road getting in the way. That way dual carriageways are treated the same as twoway roads, and we get to eliminate half of the possibility space by working with one direction at a time.

Reasoning about intersections (osm nodes, or rings in the graph) also doesn't care how the osm ways are split up, so normalising to a list of HalfRoads inward and outward, would be a good first step. Then they are matched up in order to count center lines (if there are only 2, there isn't even any crossing traffic, so its a Merge not an Intersection, etc.), ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The line dividing traffic is unique

Just to clarify a possible complication -- what about roads with two-way cycletracks on one side?
Screenshot from 2022-04-23 11-34-20
There are two lines where the direction of traffic changes (or even three -- the bollards between vehicles and cycletrack). I assume we'd use the line separating directions of vehicle traffic. If the cycletrack was a separate road to start, the center line there would be between the two directions of cycle traffic. And if we merged the two parallel ways, the vehicle center line would be take precedence.

That way dual carriageways are treated the same as twoway roads

This is one reason I like this representation

Choose a reason for hiding this comment

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

The line dividing traffic is unique

There are two lines where the direction of traffic changes (or even three -- the bollards between vehicles and cycletrack).

You're right, what I described is very car-centric. That was my natural perspective given that I understand road tagging, but not cycletrack tagging. Having one "center line" is probably still a useful restriction to place one what a "road" can be, even if we need special support for this kind of situation.

My gut feeling is that treating the cycletrack as a "nested" road, or as a separate road that references the main road's geometry, might be an ergonomic way to work with them (thought not to implement), but I haven't thought too much about how cycletracks influence the transformations in unique ways.

Allowing backwards lanes on the forward side and vice versa, but still choose one center line to take precedence is probably the simplest way.

raw.roads.get_mut(&r1).unwrap().scale_width = 0.5;
raw.roads.get_mut(&r2).unwrap().scale_width = 0.5;
for id in [r1, r2] {
// Anything derived from lane_specs_ltr will need to be changed. When we store
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was the "big conceptual leap" for this PR. Previously we mutated a scale_width on RawRoad and haphazardly applied it in some of the query methods. It was confusing to think about when to apply it and make sure not to do it twice. So I actually like this approach more.

Choose a reason for hiding this comment

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

Looks good. We can fiddle with the individual widths however we like.

We might end up with one set of widths at the start and at the end. See placement:{start,end} and width:lanes:{start,end} which exist as explicit versions of placement=transition, and I guess "width = transition", and were crucial parts of JOSM Lanes, and will probably be needed for placement impl here.

The roads can tween between start and end, and we can match the next end to the next start so that things join up. I wonder we do that to represent the curb shape when a new lane is created...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ooh, I hadn't even thought about that yet. We'll definitely add more detail over time.

let mut sidewalk_right = None;
let mut sidewalk_left = None;
for l in &lane_specs {
total_width += l.width;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note this was previously wrong. We do this shifting of the OSM center line based on sidewalks on one or both sides of the road, but we didn't apply scale_width here to the per-lane widths.

@BudgieInWA
Copy link

The unclear parts will be what happens to preview_intersection, trimmed_road_geometry

I think there is a base amount of geometry that is inherent to the road/intersection -- maybe as a function of underlying geom/attributes -- that is needed for calculating semantics. The intersection areas between any two features is how we decide what needs moving, merging, etc. so while the center points are the defining geometry, the left and right width lead us to the geometry of the thick line, which intersects with other thick lines to creates intersections, which get there own bare minimum road area geometry, which intersects with other roads or intersections that can be merged in, etc.

That baseline geometry will be the starting point for the other map type that draws curvy curbs around intersections like JOSM Lanes.

@dabreegster
Copy link
Collaborator Author

Thanks for initially validating the approach. I want to confirm one thing before I proceed.

But most importantly, the InitialMap stuff only has 3 purposes -- all actually focused on producing final geometry.
This sounds like the useful distinction to me: InitialMap is like "VisualMap" or "ActualMap", producing locale specific geometry, markings, etc. with whatever assumptions you want to make. Anything that RawMap doesn't want to test.

I'm planning to remove InitialMap entirely, and add polygon to RawIntersection and trimmed_center_points to RawRoad. Modules in transform/ will update these derived values. We'll add more complexity, like start and end width, as state on RawRoad.

In other words, the transition is away from calculating these derived values every time they're needed into storing in RawMap directly. RawMap becomes much more stateful. Is that the direction that sounds useful for the types of transformations you have in mind?

If so, my next steps before a "handover" (where hopefully the code is in good shape for you to start playing with) is to finish this refactor and get rid of InitialMap. Then I'll revisit the test scaffolding I set up previously around just one intersection, and be able to express part of a RawMap as GeoJSON. We can write the diff tests around any transformation of a RawMap.

@BudgieInWA
Copy link

In other words, the transition is away from calculating these derived values every time they're needed into storing in RawMap directly. RawMap becomes much more stateful. Is that the direction that sounds useful for the types of transformations you have in mind?

Yes, that sounds great. If I imagine the future system, I imagine that it does include some derived values (like road markings themselves, or nice curvy curbs) that are calculated on demand, or cached in some other type (like VisualMap in my example), but for now, consolidating the types and treating everything as state is better.

If so, my next steps before a "handover" (where hopefully the code is in good shape for you to start playing with) is to finish this refactor and get rid of InitialMap. Then I'll revisit the test scaffolding I set up previously around just one intersection, and be able to express part of a RawMap as GeoJSON. We can write the diff tests around any transformation of a RawMap.

Having tests that run just a single transformation on a RawMap sounds useful, as do "end-to-end" OSM to final RawMap tests.

One interesting mode of operation would be to output RawMap as OSM xml in some "normal form": consistent tagging, all geometry running down the center of the road width, intersection:areas (or whatever they are called) for intersections, etc. without worrying about the connectivity. I would be interested to see what it looks like using those outputs directly in existing tools like Map Machine. It might be a good way to integrate with such tools.

@dabreegster dabreegster marked this pull request as ready for review May 1, 2022 17:56
@dabreegster
Copy link
Collaborator Author

OK, I fixed a last few issues and am regenerating everything now. Assuming I don't find any major issues, I'll merge. Next steps are what I said previously -- adding more state to RawMap and getting rid of InitialMap entirely.

Behaviorally, this PR does have an effect near roads that have shrunken widths. There was a bug in the old implementation, and the new version causes slightly different output. It looks reasonable enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants