Splitting roads can reorder ways incorrectly in a route relation with loops

[ockendenjo]

A number of the bus_route relations in Lancaster, UK have recently become invalid.

User:gurglypipe has been modifying the roads in the City Centre and has split the roads in a few places. These are the relevant changesets.
https://www.openstreetmap.org/changeset/56998682
https://www.openstreetmap.org/changeset/56997445

See relation: https://www.openstreetmap.org/relation/6536546

I link to two files:
Relation version 37 - though to be valid
Relation version 39 - after some roads were split using ID

User:gurglypipe report that they have not specifically modified the relation

Using a diff tool on files, several ways have moved order in the relation. This should not happen when roads are split. This relation contains several loops, which might be why the problem has occurred??

[ockendenjo]

[bhousel]

Thanks @ockendenjo - this is related to #4589

The fix for that bug might have introduced this one, or it could be a case that iD always got wrong. It's very hard to order the member ways correctly in a relation that loops back over itself, especially when the relation is very large and iD hasn't downloaded all of its members.

My guess is that one of those ways inside the moved range was what was split, but iD hadn't downloaded the other parts of the relation - this is why iD joined w473970093 to itself.

The best way to address this issue would be to reduce the issue to a minimal condition and write a test case for it in here:
https://github.com/openstreetmap/iD/blob/master/test/spec/osm/multipolygon.js#L147

[ockendenjo]

The ways were already ordered correctly in the relation so ID would not have needed to reorder them.
The new segment of road should either have inserted before or after the existing (shortened) way, depending on the travel direction. Calculating the direction in which the way is travelled might be the difficult thing, and probably depends on the ways either side being downloaded?

Even if ID got the before/after decision wrong this would probably have been better than reordering all the ways

I've also come across a case where splitting a road has caused the stops/platforms in the relation to be reordered. This is presumably a side effect of the roads being re-ordered incorrectly?

[bhousel]

The ways were already ordered correctly in the relation so ID would not have needed to reorder them.

Maybe, but iD would have no way to know that..

The new segment of road should either have inserted before or after the existing (shortened) way, depending on the travel direction. Calculating the direction in which the way is travelled might be the difficult thing, and probably depends on the ways either side being downloaded?

In this situation, I think the new way needed to be added twice and in opposite direction, because it's an out-and-back with loops?

I think this is a map of around where the issue occurred.

I've also come across a case where splitting a road has caused the stops/platforms in the relation to be reordered. This is presumably a side effect of the roads being re-ordered incorrectly?

I don't know.. It's really hard to tell what changed between v11 and v16. Can you pin it down to a single changeset?

[bhousel]

I do really want to improve this... Some things to keep in mind:

• The splitting code was originally intended to work with multipolygons, not routes, and definitely not PTv2 routes. We think it mostly just happened to work correctly except when there are loops and repeated segments.
• We only started even thinking about routes with loops and repeated segments 2 months ago with Several broken things when editing a route relation that doubles back over a way #4589 - so I'm not surprised that there are still some bugs. We're finally starting to assemble some good test cases.
• iD does not download fully all the member ways of a relation. (Doing so could take a very long time). This makes the problem even more difficult.

[ockendenjo]

I've had another look at the changesets for the relation mentioned in my original bug report, and the screenshot you posted:

v	Changes	Effect on relation
37	Initial relation	Ordered correctly
38	Road split on Common Garden Street	Order of ways changed
39	Road split on King Street	Order of ways changed
40	Road split on Dalton Square (JOSM)	New segments added correctly
41	Relation key/value pair changed
42	Road split on Dalton Square	Order of ways changed

Road splits

Green sections are the new sections of road that were created. Blue sections are the remains of the original way after it was split

bus_route diagram

[ockendenjo]

With regards to the splitting of ways in PTv2 relations, I think that it is necessary to download the adjacent ways (before and after) in the relation.

Linear example

• We want to split the blue way in the diagram
• The grey ways (adjacent in the relation) have not been downloaded
• Until we download the adjacent ways in the relation their is an ambiguity as to whether the new segment should go before or after the existing segment
• The orange and purple ways and their connection to the blue way allow us to determine the position of the newly created green section

Loop example

• A more complicated case arises where the same way (red) appears before and after the blue way in the relation.
• In this case we have to download another adjacent way (orange, purple) in both directions in the relation to remove the ambiguity, and to work out where the newly created green section should go

[bhousel]

Hey @ockendenjo there are actually a few loop situations we should test for:

When splitting B, the resulting ways could be ordered B1,B2, or B2,B1, or both depending on what the route does.

We probably don't need to download the whole route though, if we just assume it's ordered to begin with.

[alphensebezorger]

When addressing this issue it would be wise to keep in mind that some routes still use the roles forward/backward to indicate different paths for the forward and return direction. At least bicycle and mtb routes use this method, but there might be others. This method creates a loop in the route, of which half is used for the forward trip and the other half for the return trip.
Currently, splitting ways in these loops often causes incorrect sorting of the relation members.