Add support for complex intersection and via way restrictions #4768

bhousel · 2018-02-03T01:32:39Z

This PR adds support for creating turn restrictions in more complex intersections.

Before

You click on a junction node
The turn restriction editor shows you all the ways connected to that junction node
You select a from way, and can add restrictions to another way in that intersection
Turn restrictions via node only

After

You click on a junction node
The turn restriction editor shows you all ways and important junction nodes within 20 meters
You select a from way, and can add restrictions to any other way
Turn restrictions via node and via way are both supported automatically
(If there are no other nearby junctions found, the turn restriction editor works just as before)

This is work that I did a few months ago, and has been used internally at Mapbox by our data team. I'm confident that it is stable enough to merge into the main iD that everyone uses. 🎉

Would love to hear people's feedback on ways to make this more useful or easier to understand for casual mappers.

(The text that flashes while I'm using the editor is for debugging - I'll make this friendlier!)

↩️ ⬆️ ⬇️ 🔀

manfredbrandl · 2018-02-03T10:29:06Z

I would like to try this to give you feedback. Is it possible to try this at http://preview.ideditor.com/newturnrestrictions/ ?

bhousel · 2018-02-05T23:28:57Z

@manfredbrandl - I put a copy of this branch here for testing:
http://sonny.7thposition.com/osm/iD/

manfredbrandl · 2018-02-06T08:27:45Z

@bhousel thanks, i tried and had no problems with useability at all.

manfredbrandl · 2018-02-06T08:48:48Z

I tried somewhere else and found a bug. i cannot select the "Maria-Theresia-Allee" (grey street connecting from south-west) in the turn restrictions editor:

When i move the mouse over it "w4014543" is displayed in the bug area and the street is surrounded by the pink border (not flashing), click on the street does not change anything.
See http://sonny.7thposition.com/osm/iD/#background=basemap.at-orthofoto&disable_features=boundaries&map=20.65/47.07766/15.44361

bhousel · 2018-02-06T14:12:46Z

When i move the mouse over it "w4014543" is displayed in the bug area and the street is surrounded by the pink border (not flashing), click on the street does not change anything.

Thanks @manfredbrandl that does look like a bug!

This is the part of the algorithm where trivial sections get trimmed from the vgraph. Removing a vertex from `vertexIds` means "stop checking this one". But there were some situations where it could get removed twice, so we now just verify that `vertexId` is actually in the array before calling `splice`.

bhousel · 2018-02-07T13:33:12Z

By the way @manfredbrandl thanks for posting this intersection..
It's a good example of one where it didn't fit very well into the turn restriction editor view. This is partly because it fills the 20 meter limit of what we will show, and has very short segments, like the service and unclassified roads.

I made some tweaks to extend the leaf nodes out far beyond the viewport edge, and to better calculate a zoom and center that includes all of the important parts of the intersection..

manfredbrandl · 2018-02-07T15:54:23Z

@bhousel it was a pleasure for me to help you. Could you provide the new code on the branch so that i can test it again?

bhousel · 2018-02-07T16:39:23Z

Could you provide the new code on the branch so that i can test it again?

Just updated http://sonny.7thposition.com/osm/iD/ to ea4ac80

- includes in the message the names of the streets - also highlights related segments and nodes along the path The messages are currently a bit rough: U-Turns FROM Black Horse Pike IS allowed... VIA Main Street TO Black Horse Pike

slhh · 2018-02-07T23:01:00Z

The following screenshot seems to show a bug:
When I click on the upper red icon it toggles both red icons to green, and with the next click both back to green.
When I click on the lower red icon it toggles both red icons to green, but the next click toggles only the lower one to red. Both icons can be toggled separately, but as soon as the upper icon is clicked while both are green, they get coupled again.

I would expect both icons to toggle always seperately.

Selected node was n26415364, but the yellow line (secondary) was temporarily added for testing.

(re: #2622)

slhh · 2018-02-08T00:35:12Z

Would love to hear people's feedback on ways to make this more useful or easier to understand for casual mappers.

Here are some ideas:

Place a start arrow on the starting way pointing directly to the connecting node.
When hovering an icon, highlight all ways belonging to the restriction and/or animate a driving vehicle.
Do only show the first level of restriction icons. Split the icon into the circular part and the arrow part. Use the circular part to toggle the restriction, and use the arrow part to unhide the next level. In the next connecting node is out of view, zoom the restriction editor accordingly. Make the icons (maybe arrow part only) yellow, if some existing restrictions on further levels do exist, which are hidden.
The restriction editor doesn't really fit into the inspector of a connecting vertex anymore. Showing properties of a selected node above and below, while editing restrictions which aren't related to the node, can be confusing.
Show the full list of generated relations below the restrictions editor.

manfredbrandl · 2018-02-08T11:57:08Z

use the arrow part to unhide the next level

I would prefer to see all turn restriction with the selected from way without having to unhide several next levels when there are several via ways. At https://wiki.openstreetmap.org/wiki/Relation:restriction there is an example with three via ways:

slhh · 2018-02-09T00:22:50Z

I have found another bug. A very short (e.g. 5 m) deadend way is stretched to the frame in the restriction editor. This looks quite strange.

The turn restriction editor shows you all ways and important junction nodes within 20 meters

Especially for large junctions, which are the typical ones requiring complex turn restrictions, 20m seem to be insufficient. Here are some ideas to address this:

Analyse existing restrictions to select the default scaling
Click on the arrow part of the icons to extend as I mentiond in my previous comment.
Use small arrows where the way intersects the frame to extend on click to the next connection node. The arrows can be hidden if there isn't any connection node within resonable distance.

You select a from way, and can add restrictions to any other way
Turn restrictions via node and via way are both supported automatically

Via nodes and ways aren't redundant, and they can't be added automatically in the general case.
They have to be members for good reason, a fixed from-to pair can have different paths due to using different via members.
Unhiding selectively level by level, as mentioned in my previous comment, would give the user control which path to use.

slhh · 2018-02-09T00:47:06Z

I would prefer to see all turn restriction with the selected from way without having to unhide several next levels when there are several via ways.

My first thought was to show all existing restrictions by default, but I realized that there could be conflicts due to multiple possible via paths.

At https://wiki.openstreetmap.org/wiki/Relation:restriction there is an example with three via ways:

I think the example can be reduced to using one via way only, but in general we need more than twovia members.
Unfortunately, the current code doesn't seem to support this.

manfredbrandl · 2018-02-09T09:37:42Z

My first thought was to show all existing restrictions by default, but I realized that there could be conflicts due to multiple possible via paths.

Thats a point. Maybe we can have a conflict-checker and initially support editing junctions without conflicts only. iD is easy to use and intentionally simple. It lets you do the most basic tasks while not breaking other people's data. We can support junctions with conflicts later.

bhousel · 2018-02-09T20:45:41Z

(Trying to reply quickly to most of the feedback - sorry if I miss some)

from @slhh:

Place a start arrow on the starting way pointing directly to the connecting node.

will consider, but it seems like clutter.

When hovering an icon, highlight all ways belonging to the restriction and/or animate a driving vehicle.

👍 I've added this now, it highlights all ways along the path in the "related" way style. I might adjust it, since this is not very visible

Do only show the first level of restriction icons.

I've added a slider that will let users expand the search (see below)

The restriction editor doesn't really fit into the inspector of a connecting vertex anymore. Showing properties of a selected node above and below, while editing restrictions which aren't related to the node, can be confusing.

Agree, but this is #3136 and I don't have time to do it now.

Show the full list of generated relations below the restrictions editor.

Probably wont at this time - for some very complex intersections (like an SPUI) it can be several dozen. But if we do #3136, this would be replaced by a summary view that might list them.

I have found another bug. A very short (e.g. 5 m) deadend way is stretched to the frame in the restriction editor. This looks quite strange.

Yeah this is intentional - I do need to show the user something here and it needs to be long enough to be clickable even with an arrow stuck on top of it, but I think I don't need to extend it quite so far. I'll try a shorter amount. (can you link to the intersection?)

Especially for large junctions, which are the typical ones requiring complex turn restrictions, 20m seem to be insufficient. Here are some ideas to address this:

Yes, I noticed this too (testing with the intersection at map=20.00/39.89412/-75.09134) - I've extended it to 30m for now. Might use the detail slider mentioned below to adjust the search distance.

Analyse existing restrictions to select the default scaling

Yep, it does this now (the viewer adjusts the zoom to better fit all the key vertices).

Click on the arrow part of the icons to extend as I mentiond in my previous comment.

Part of the main design goals for this editor is to continue to allow mappers to create restrictions with one click. Our data team at Mapbox does a lot of TR mapping and this is one of their important workflows. But I'll think about whether we can use clicks on different parts of the arrows to do different things.

Unhiding selectively level by level, as mentioned in my previous comment, would give the user control which path to use.

Yeah intersections with cycles in them and multiple via-ways is a big issue. I think you'll like the slider I made. It's still rough but I like the idea of giving users control of how much detail they want to see (and defaulting them to "not much")

BTW - I'm using this intersection as my "worst case scenerio" 20.00/39.89071/-75.08993. I don't think it is modeled very well, and I would do it differently, but it is good for testing, so please don't edit it!

BTW2 - I also added the ability to tag "only" restrictions, which makes the above intersection much easier to reason about.

I think the example can be reduced to using one via way only, but in general we need more than two via members.

Yes I would also model that u-turn as a single via way. The code could handle any number, but I really don't want to go above 2 because it would encourage people to build unnecessarily complicated things.

(I decided that the larger context of the intersection is important and shouldn't be hidden from the user) Also - show detail slider only if the intersection is complex - hide the restriction editor completely if there is no real intersection (e.g. junction of `highway=path`)

slhh · 2018-02-10T16:49:05Z

Place a start arrow on the starting way pointing directly to the connecting node.

will consider, but it seems like clutter.

I was definitely missing this, but highlighting all ways along the path seems to make it dispensable now. Maybe, we will need the arrows to control or indicate something later.

Do only show the first level of restriction icons.

I've added a slider that will let users expand the search (see below)

It doesn´t expand selectively, and therefore, it doesn't help to control the via path.

The restriction editor doesn't really fit into the inspector of a connecting vertex anymore. Showing properties of a selected node above and below, while editing restrictions which aren't related to the node, can be confusing.

Agree, but this is #3136 and I don't have time to do it now.

#3136 doesn't seem to address the logical misfit.
We could also avoid avoid the logical misfit by showing restrictions only, which are touching the selected node. This might be a benefit anyway, because it gives the user a limited control of the via path and reduces conflicts.

Show the full list of generated relations below the restrictions editor.

Probably wont at this time - for some very complex intersections (like an SPUI) it can be several dozen. But if we do #3136, this would be replaced by a summary view that might list them.

Agree, it doesn't make sense at the current position of the restriction editor.
Maybe, we can address the main usecases differently:

Make the relation accessible:

A right click on the restriction icon should link to the relation. This could be indirectly though a context menu listing multiple conflicting (in view of using same from-to pair) restrictions.

Get an overview of existing relations:

Make the icons, which are representing potential restrictions only or sideeffects of restrictions only, smaller or lighter. This means the red ones which are generated by "ONLY" restrictions, and all green ones.
Add a checkbox to display existing ones only, maybe next to the slider.

I have found another bug. A very short (e.g. 5 m) deadend way is stretched to the frame in the restriction editor. This looks quite strange.

Yeah this is intentional - I do need to show the user something here and it needs to be long enough to be clickable even with an arrow stuck on top of it, but I think I don't need to extend it quite so far. I'll try a shorter amount. (can you link to the intersection?)

I don't think the extension is really required. There can also be non dead ended short ways in the junction.
You can't extend there, and these do need a solution to be clickable too. Currently it doesn't seem to be an issue anyway, because the user can click in free space to remove all icons.

can you link to the intersection?

No, it wasn't a real one. I drew a short dead ended way to the inside of the junction for testing.
It was extended to the frame across other ways. You should definitely stop extending the way before intersecting another way.

Click on the arrow part of the icons to extend as I mentiond in my previous comment.

Part of the main design goals for this editor is to continue to allow mappers to create restrictions with one click. Our data team at Mapbox does a lot of TR mapping and this is one of their important workflows. But I'll think about whether we can use clicks on different parts of the arrows to do different things.

The frequent simple restrictions would still one click only, but the rare complex ones would require more clicks. This doesn't seem to degrade efficiency significantly.

My approach of unhiding selectively level by level along a path can also be combined with your slider:
Let the slider define the default display, and let the user expand further if required.
Maybe, the slider setting can be automatically reduced to avoid any display conflicts due to different via paths.

I think you'll like the slider I made. It's still rough but I like the idea of giving users control of how much detail they want to see (and defaulting them to "not much")

Hiding existing restrictions totally doesn't seem to be good. We need to indicate that there is something hidden. This can either be a warning icon at the slider, or more a more specific indicator at the arrow part of the restriction icons of the last displayed level.

This was used to decide whether to show the "Max Via" control. (It's not really needed for a simple intersection) However since we added the "Max Distance" control, it just makes sense to show both sliders, since the distance can affect the complexity.

(using ',' with css selectors was problematic)

- absolutely positioned attach to inspector, - float slightly above field - looks much better

But we keep them around in `_oldTurns` and will put them back if the user clicks again to unrestrict the ONLY.

These should not be a thing, and if they are, I'm ok with them being treated as 2 separate ways. We can add code back in later if it turns out to be a widespread issue in OSM.

1ec5

Some comments regarding localizability. Nothing that needs to block this PR, I suppose, but certainly this PR provides more motivation for me to help improve iD’s localization support. 🙂

1ec5 · 2018-02-28T22:34:34Z

data/core.yaml

+      via: Via
+      via_node_only: "Node only"
+      via_up_to_one: "Up to 1 way"
+      via_up_to_multiple: "Up to {num} ways"


This is another place where non-anglocentric plural support would be desirable: #597.

Ok, makes sense.. The slider only goes up to 2 via ways, and I'm not even sure whether it's worth making it any more extensible than that, so I'm just going to hardcode the string as "Up to 2 ways" to make things easy on the translators.

1ec5 · 2018-02-28T22:52:29Z

data/core.yaml

+        straight_on: Straight On
+      no_turn_string: "{no} {turn} {indirect}"
+      only_turn_string: "{only} {turn} {indirect}"
+      allowed_turn_string: "{turn} {allowed} {indirect}"


In some languages, it may be difficult to translate these turn strings independently of the overall sentence. A more sound approach would be to multiply out these strings by all the possible combinations, e.g., “ONLY Left Turn”. In the meantime, languages that have problems with these strings may be able to use the imperative form, e.g., “ONLY Turn Left”.

Good point.. I'll just enumerate all of these (and leave the {indirect} as a token replacement)

1ec5 · 2018-02-28T22:54:28Z

data/core.yaml

+      via: VIA
+      to: TO
+      from_name: "{from} {fromName}"
+      from_name_to_name: "{from} {fromName} {to} {toName}"


Languages with declension rules won’t be able to translate this well, but I guess the fact that the prepositions are bold makes it feel more like a form and less like a coherent sentence anyways.

Yeah this is extra tricky because the {fromName} and {toName} might not actually be proper named roads, but rather strings like "Residential Road" or "Service Road" which could follow different rules depending on the modifier. We don't support different conjugation of the preset name strings, so I'll punt on this one for now. (But yeah, hoping the way we present it in the UI makes it seem more like a filled in form than a grammatically incorrect sentence).

- actionRestrictTurn will no longer "infer" the turn type - restrictionType *must* be passed in - this is ok because the only code we use this action (restrictions.js) already has inferred the type - this simplifies what the action actually does - moved the tests from restrict_turn.js that were really just testing the restriction type inferrence over to intersection.js (and added a few more tests for iD.osmInferRestriction)

- change the "Up to {num} ways" to "Up to 2 ways" (we don't plan to go above 2 for now) - enumerate all the turn types No/Only/Allowed x Left/Right/Straight/U - Add a line for "Click for" to let user know they can click to toggle see #4768 (review)

bhousel · 2018-03-01T06:46:20Z

Thanks everyone who tested this out and provided feedback!
I haven't commented much but I've been working pretty steadily on it for a few weeks, and it's ready to merge.

The turn restriction editor has learned a few new tricks:

Hovering to preview restrictions - Hover over a FROM way to preview all the turn restrictions along allowed paths
More color hinting - thanks @rasagy for the suggestion!
Sliders for Max Distance and Max Via Ways - I realized from testing this that there is no universal best "max distance" value for how far to search from the initial selected vertex to find more connected ways. Some parts of the world have very compact roads, while others have their dual carriageways almost 50 meters apart.
Support for only_ restrictions - Now you can click toggle to add only_ turn restrictions. Creating an only_ turn restriction will remove all other turn restrictions from the FROM way, but they will be remembered so that if you click again to remove the only_ restriction, any previous no_ restrictions will be restored.
Field Help - This field is unusually complicated for iD. I struggled a lot to make it simple, and ended up building a special help component for the field. This can be used for other fields too, but currently only works for this one. We've considered (and rejected) the idea before for special help content on unusually confusing fields, but this was more in the context of offering tag guidance (like explaining concepts like "tracktype" or "smoothness").

Add support for complex intersection and via way restrictions

c1378a1

bhousel added 4 commits February 6, 2018 14:34

Extend leaf ways, so they don't stop within the viewport

aa7925a

Better calculation for fitting the whole intersection into view

1007cfe

Handle situation where nodes have no loc (like in tests)

5ca56b6

Extend max distance to 30m, but as measured from starting vertex

ea4ac80

bhousel added 2 commits February 7, 2018 17:00

Better calc of turn arrow placmenet along short segments

cf1226c

Trying out more intuitive restriction help message

3c340a0

- includes in the message the names of the streets - also highlights related segments and nodes along the path The messages are currently a bit rough: U-Turns FROM Black Horse Pike IS allowed... VIA Main Street TO Black Horse Pike

Cycle through only_ turn restriction states

0cbff57

(re: #2622)

bhousel added 5 commits February 9, 2018 16:22

WIP: add detail slider to restriction editor

47eb525

Style tweaks, persist max detail to localstorage

3bd8d8d

Trying help text with color hint

4e1bafc

Adjust text hint colors, change "Continue" to "Straight on"

ca9aac5

bhousel added 14 commits February 20, 2018 17:01

Remove fromOnly highlighting (was confusing), try to simplify

419d1e1

Drop confusing fromOnly code, consolidate path highlighting code

156d0fd

Localize all the strings

aaed678

Remove isComplex, not used

c63f79a

This was used to decide whether to show the "Max Via" control. (It's not really needed for a simple intersection) However since we added the "Max Distance" control, it just makes sense to show both sliders, since the distance can affect the complexity.

Update tests to use '_' id separators instead of ','

46cc6cc

(using ',' with css selectors was problematic)

Return an indirect u-turn restriction for an only restriction

7fa2721

Add uiFieldHelp component, currently just for turn restrictions field

57d2aee

WIP: Add content for turn restriction field help

b3b2be9

Add icons and improve css styles for restriction field help

97cbfc5

Add close button, remove _showing state variable

de1fdaa

Add some restriction help images, adjust styles

6c6b137

Much improved field help

a700d42

- absolutely positioned attach to inspector, - float slightly above field - looks much better

Increase box shadow slightly so help popup stands out more

d87a43d

Finish help, improve text and styling

e975014

bhousel force-pushed the advanced_intersection branch from 5baaf99 to e975014 Compare February 27, 2018 20:51

bhousel added 3 commits February 27, 2018 21:14

Adjust wording of some of the restriction help sections

fb40987

When switching to ONLY restriction remove all other restrictions

f9d000a

But we keep them around in `_oldTurns` and will put them back if the user clicks again to unrestrict the ONLY.

Remove the "excludes coincident highways" check

e1cf49e

These should not be a thing, and if they are, I'm ok with them being treated as 2 separate ways. We can add code back in later if it turns out to be a widespread issue in OSM.

1ec5 reviewed Feb 28, 2018

View reviewed changes

bhousel added 2 commits February 28, 2018 23:55

This was referenced Mar 1, 2018

Support for only_straight_on turning restrictions #2622

Closed

Support turn restrictions via way #2346

Closed

bhousel merged commit 4b1f8a8 into master Mar 1, 2018

bhousel deleted the advanced_intersection branch March 1, 2018 06:46

bhousel mentioned this pull request Mar 2, 2018

Update to iD v2.7.0 openstreetmap/openstreetmap-website#1777

Closed

tyrasd mentioned this pull request Mar 6, 2018

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

Closed

This was referenced Mar 10, 2018

Weird results when inspecting certain turn restrictions #4869

Closed

indicate route membership with shadow #659

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for complex intersection and via way restrictions #4768

Add support for complex intersection and via way restrictions #4768

bhousel commented Feb 3, 2018 •

edited

Loading

manfredbrandl commented Feb 3, 2018

bhousel commented Feb 5, 2018

manfredbrandl commented Feb 6, 2018

manfredbrandl commented Feb 6, 2018 •

edited

Loading

bhousel commented Feb 6, 2018

bhousel commented Feb 7, 2018 •

edited

Loading

manfredbrandl commented Feb 7, 2018

bhousel commented Feb 7, 2018

slhh commented Feb 7, 2018

slhh commented Feb 8, 2018

manfredbrandl commented Feb 8, 2018 •

edited

Loading

slhh commented Feb 9, 2018

slhh commented Feb 9, 2018 •

edited

Loading

manfredbrandl commented Feb 9, 2018

bhousel commented Feb 9, 2018

slhh commented Feb 10, 2018 •

edited

Loading

1ec5 left a comment •

edited

Loading

1ec5 Feb 28, 2018

bhousel Mar 1, 2018

1ec5 Feb 28, 2018

bhousel Mar 1, 2018

1ec5 Feb 28, 2018

bhousel Mar 1, 2018

bhousel commented Mar 1, 2018

Add support for complex intersection and via way restrictions #4768

Add support for complex intersection and via way restrictions #4768

Conversation

bhousel commented Feb 3, 2018 • edited Loading

Before

After

manfredbrandl commented Feb 3, 2018

bhousel commented Feb 5, 2018

manfredbrandl commented Feb 6, 2018

manfredbrandl commented Feb 6, 2018 • edited Loading

bhousel commented Feb 6, 2018

bhousel commented Feb 7, 2018 • edited Loading

manfredbrandl commented Feb 7, 2018

bhousel commented Feb 7, 2018

slhh commented Feb 7, 2018

slhh commented Feb 8, 2018

manfredbrandl commented Feb 8, 2018 • edited Loading

slhh commented Feb 9, 2018

slhh commented Feb 9, 2018 • edited Loading

manfredbrandl commented Feb 9, 2018

bhousel commented Feb 9, 2018

slhh commented Feb 10, 2018 • edited Loading

1ec5 left a comment • edited Loading

Choose a reason for hiding this comment

1ec5 Feb 28, 2018

Choose a reason for hiding this comment

bhousel Mar 1, 2018

Choose a reason for hiding this comment

1ec5 Feb 28, 2018

Choose a reason for hiding this comment

bhousel Mar 1, 2018

Choose a reason for hiding this comment

1ec5 Feb 28, 2018

Choose a reason for hiding this comment

bhousel Mar 1, 2018

Choose a reason for hiding this comment

bhousel commented Mar 1, 2018

bhousel commented Feb 3, 2018 •

edited

Loading

manfredbrandl commented Feb 6, 2018 •

edited

Loading

bhousel commented Feb 7, 2018 •

edited

Loading

manfredbrandl commented Feb 8, 2018 •

edited

Loading

slhh commented Feb 9, 2018 •

edited

Loading

slhh commented Feb 10, 2018 •

edited

Loading

1ec5 left a comment •

edited

Loading