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 importing all footways and shared-use paths! #87, #77 #90

Merged
merged 3 commits into from
Sep 15, 2022

Conversation

dabreegster
Copy link
Contributor

This introduces two new lane types and starts importing from OSM. The experiment for separate sidewalk handling is refined to only deal with parallel sidewalks and crossings. All the footways are imported always.

I'm immensely excited by this PR. Previously, most footways were either not imported at all or mangled into bike lanes. This is a huge step forwards that doesn't get stuck on the problem of parallel sidewalks. Some demos:

Screenshot from 2022-09-15 10-59-05
Screenshot from 2022-09-15 10-58-38
Screenshot from 2022-09-15 10-58-08

I've validated with the test explorer, clicking through to open the OSM ways and spot-check things. I'm next going to update A/B Street with these changes and work through integration issues there.

This introduces two new lane types and starts importing from OSM. The
experiment for separate sidewalk handling is refined to only deal with
parallel sidewalks and crossings. All the footways are imported always.
// If we're only handling sidewalks tagged on roads, skip a bunch of ways
// If we're only handling sidewalks tagged on roads, skip crossings and separate sidewalks
// Note we have to do this here -- get_lane_specs_ltr doesn't support decisions like
// "actually, let's pretend this road doesn't exist at all"
if opts.map_config.inferred_sidewalks {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this experiment now just means "bring in highway=footway, footway={sidewalk,crossing} ways". All of the new stuff in this PR works both with and without this experiment


// If it's a primarily cycleway, have directional bike lanes and add a shoulder for walking
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still odd to use two directional lanes for this case. I think street_network::Direction and everything downstream needs to have a both-ways case. It's a very complex change downstream, affecting routing, rendering, traffic simulation... so not attempting it yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I agree. An "alternates" case too.

Most reads of the value will boil down to "is travel supported in this direction", right? Routing could implement "alternating" and use that for "both ways" too.

I called it TravelDirection in my types.

@@ -32,6 +32,11 @@ pub enum LaneType {
Construction,
LightRail,
Buffer(BufferType),
/// Some kind of pedestrian-only path unassociated with a road
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very related to #77. I'm not happy with this classification as "only pedestrians" and "both pedestrians and cyclists." Things like living streets or alleyways, where motor vehicles are also allowed, aren't captured yet. Maybe we just need one shared use lane type, with a way to specify the legal or cultural priority between vehicles. For instance, I know many streets in the US without pavement, where it's effectively shared use and motor vehicles get priority.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think when it comes down to it, every lane is a list of specific designations and restrictions. There are also classifications, informed by the locale, I suppose, like "only pedestrians", "bike lane" or "bus lane (bikes allowed)" that inform markup decisions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like this way of thinking about it. Going to start a fresh issue on this soon to consolidate all of the thoughts around this...

}

pub fn describe(self) -> &'static str {
match self {
LaneType::Driving => "a general-purpose driving lane",
LaneType::Biking => "a protected bike lane",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"protected" never ever made sense, I erroneously wrote this long ago.

Also this is a localization issue... "cycling lane" or "cycle lane" would fit better in the UK. But anyway.

street_network/src/lanes/mod.rs Outdated Show resolved Hide resolved
@@ -66,6 +66,25 @@ export const makeLanePolygonLayer = (text) => {

return new L.geoJSON(JSON.parse(text), {
style: function (feature) {
if (feature.properties.type == "Footway") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The styling is not great. Any ideas for distinguishing the two?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's time to start copying design from the strassenraumkarte-neukoelln :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I already lifted the dashed outline idea and path color from there ;)

dabreegster added a commit to a-b-street/abstreet that referenced this pull request Sep 15, 2022
a-b-street/osm2streets#90.

Also remove the pedestrian plaza area type for now. It adds visual
clutter and has no editing/simulation/semantic impact yet.
@dabreegster
Copy link
Contributor Author

a-b-street/abstreet#994 has more rendering demo. Mostly smooth sailing there, I expect to finish ironing out some integration problems and merge both in a few hours!

@dabreegster dabreegster merged commit 322044d into main Sep 15, 2022
@dabreegster dabreegster deleted the import_footways branch September 15, 2022 22:26
dabreegster added a commit to a-b-street/abstreet that referenced this pull request Sep 16, 2022
a-b-street/osm2streets#90.

Also remove the pedestrian plaza area type for now. It adds visual
clutter and has no editing/simulation/semantic impact yet.
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.

2 participants