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

Several broken things when editing a route relation that doubles back over a way #4589

Closed
bhousel opened this issue Dec 4, 2017 · 5 comments
Labels
bug A bug - let's fix this! bug-release-blocker An important bug - let's get this fixed in the next release! priority A top priority issue that has a big impact or matter most to new mappers

Comments

@bhousel
Copy link
Member

bhousel commented Dec 4, 2017

Reported to me over the weekend - it looks like we have an issue with splitting ways that belong to route relations. After they split, they don't all belong to the route relation.

This probably used to work and we changed it to work like this for Turn Restrictions.

(When splitting a way with a turn restriction, you only want one of the roads to keep the relation, but when splitting a road with other kind of relations, you want all of the surviving ways to belong to the relation).

@bhousel bhousel added bug A bug - let's fix this! bug-release-blocker An important bug - let's get this fixed in the next release! priority A top priority issue that has a big impact or matter most to new mappers labels Dec 4, 2017
@slhh
Copy link
Contributor

slhh commented Dec 4, 2017

When splitting a way with a turn restriction, you only want one of the roads to keep the relation,

When spitting a way which is a via member, both ways need to become via members.

but when splitting a road with other kind of relations, you want all of the surviving ways to belong to the relation

At least for PTv2 routes, the members do also need to have in the correct order.

@bhousel
Copy link
Member Author

bhousel commented Dec 4, 2017

Gotcha @slhh that makes sense.. So I guess there are 3 cases of what to do when splitting a relation member:

  1. (multipolygons, sites, and other random relations) - all surviving ways become members
  2. (routes) - all surviving ways become members, but order must be preserved
  3. (turn restrictions) - only the ways attached to the via node remain as members of the relation / special code to consider the case where the way being split is a via way.

(1 and 2 might be the same thing actually)

@bhousel
Copy link
Member Author

bhousel commented Jan 9, 2018

I realized today that this issue isn't what I thought it was.
We currently handle all of the situations above and have tests for them.

This particular issue only occurs when a road is split that belongs to a relation several times, which can happen often with routes.

screenshot 2018-01-09 13 38 13

@bhousel bhousel changed the title When splitting a way with a route relation, not all descendant ways belong to the route relation Several broken things when editing a route relation that doubles back over a way Jan 9, 2018
@bhousel
Copy link
Member Author

bhousel commented Jan 9, 2018

Need to fix:

  • actionAddMember
  • actionSplit
  • osmJoinWays

bhousel added a commit that referenced this issue Jan 10, 2018
bhousel added a commit that referenced this issue Jan 12, 2018
(re #4589)

Strongly prefer to generate a forward path that preserves the order
of the members array. For multipolygons and most relations, member
order does not matter - but for routes, it does. If we started this
sequence backwards (i.e. next member way attaches to the start node
and not the end node), reverse the initial way before continuing.
@bhousel bhousel added wip Work in progress and removed wip Work in progress labels Jan 18, 2018
@bhousel
Copy link
Member Author

bhousel commented Jan 18, 2018

This was done 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug - let's fix this! bug-release-blocker An important bug - let's get this fixed in the next release! priority A top priority issue that has a big impact or matter most to new mappers
Projects
None yet
Development

No branches or pull requests

2 participants