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

"only_*" turn restrictions shouldn't affect turns in opposite direction of way #4849

Closed
tyrasd opened this issue Mar 6, 2018 · 6 comments
Closed
Labels
bug-release-blocker An important bug - let's get this fixed in the next release!

Comments

@tyrasd
Copy link
Member

tyrasd commented Mar 6, 2018

I noticed the following regression related to the reworked turn restrictions editor (see #4768 / #2622): When a short road segment in a (moderately) complex intersection is used as a from member of a only-type turn restriction, not only are all "outgoing" turns marked as prohibited (i.e. turns one could otherwise take when driving on the road towards the via node or way(s)), but also all turns on the other end of this road, basically transforming it into a dead-end road. Here's an example (location):

turn-restriction-only-opposite

Notice how the southern end of the selected way turns into an all-red situation after adding the only-restriction at the northern end of the road.

Another confusing follow-up effect of this is that some intersections are displayed differently on how far one is zoomed in, as seen here (same location as above):

turn-restriction-only-zooming

//edit: a second side effect of this is that all previously mapped no restrictions on the opposite end of a way are removed after adding an only restriction on one end. In this example, the initial southern no_u_turn restricton is affected by this.

I think that only-type turn restrictions should only affect the direction of the way towards the respective via member(s) (even though the osm wiki page about turn restrictions doesn't state this explicitly). And as far as I can see, this is also how every routing software implements these.

@bhousel
Copy link
Member

bhousel commented Mar 6, 2018

I think that only-type turn restrictions should only affect the direction of the way towards the respective via member(s) (even though the osm wiki page about turn restrictions doesn't state this explicitly). And as far as I can see, this is also how every routing software implements these.

Yes, this is something that came up in our internal testing and I took an approach that might not be perfect.. I was trying to follow the rule on the osm wiki:

If it is "only_", then you know that the only routing originating from the "from" member leads to the "to" member.

We agree that that this wording is probably an oversimplification - when the FROM way is bidirectional.

I ended up adding some code in here to say "only follow this rule and restrict other paths that are local to the intersection". The situation we ran into with testing was that the FROM way could trail off for miles and that only_ restriction was affecting another intersection far away, if we applied the rule literally as written.
https://github.com/openstreetmap/iD/blob/master/modules/osm/intersection.js#L468

I think your approach of "only_ only applies to paths that leave the FROM way towards the VIA" is also good.

It would make me even happier if we could just say "only_ restrictions require a oneway FROM" - but I've definitely seen a few "in the wild" that break this rule, so changing it would require a discussion with the wider community.

Another confusing follow-up effect of this is that some intersections are displayed differently on how far one is zoomed in, as seen here (same location as above):

Yeah that's a consequence of the "local via" rule I mentioned above.. What is considered "local" depends on that slider.

@bhousel bhousel added bug A bug - let's fix this! considering Not Actionable - still considering if this is something we want labels Mar 6, 2018
@bhousel
Copy link
Member

bhousel commented Mar 6, 2018

And as far as I can see, this is also how every routing software implements these.

Should also add - If this is the case then we should definitely change iD to match what they do 😆

@tyrasd
Copy link
Member Author

tyrasd commented Mar 6, 2018

here's an example: relation/2227207

OSRM and Graphhopper happly route across this turn restriction in both directions:
https://www.openstreetmap.org/directions?engine=osrm_car&route=47.05877%2C15.47019%3B47.05915%2C15.46974

//edit: in iD this location looks like this:
selection_132

@bhousel bhousel removed the considering Not Actionable - still considering if this is something we want label Mar 6, 2018
@bhousel
Copy link
Member

bhousel commented Mar 6, 2018

Yeah I thought about this more.. I'll definitely change it and push out an update in the next day or so.

@slhh
Copy link
Contributor

slhh commented Mar 6, 2018

If it is "only_", then you know that the only routing originating from the "from" member leads to the "to" member.

We agree that that this wording is probably an oversimplification - when the FROM way is bidirectional.

+1
Applying the rule literally would make the from member a dead end road in the other direction.
Only restrictions are likely used quite often with bidirectional roads. The German wiki requests to always use only restrictions where only one routing is allowed, because the restriction is signed this way. This does likely apply some to other countries too.

@bhousel bhousel added bug-release-blocker An important bug - let's get this fixed in the next release! and removed bug A bug - let's fix this! labels Mar 8, 2018
@bhousel
Copy link
Member

bhousel commented Mar 10, 2018

This is fixed now - only_ turn restrictions will only count if they leave the from way towards the via node or way(s).

screenshot 2018-03-09 22 41 20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-release-blocker An important bug - let's get this fixed in the next release!
Projects
None yet
Development

No branches or pull requests

3 participants