#21856 closed defect (fixed)
[PATCH] Split way: Wrong position of new member in PTv2 relation splitting a loop
| Reported by: | skyper | Owned by: | team |
|---|---|---|---|
| Priority: | major | Milestone: | 24.08 |
| Component: | Core | Version: | latest |
| Keywords: | template_report split way route relation order member loop | Cc: |
Description ¶
What steps will reproduce the problem? ¶
- Have an ordered PTv2 relation containing a small loop, see ptv2_relation_loop_split.osm.xz
- Split the loop (at the node
Split)
What is the expected result? ¶
The new member is added at the correct position.
What happens instead? ¶
The new member is added at the wrong position leading to a gap.
Please provide any additional information below. Attach a screenshot if possible. ¶
Similar to #19288 and not new, but I stumbled over it again.
It does not matter which of the two parts keeps the existing OSM id.
Relative:URL: ^/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2022-02-10 22:03:15 +0100 (Thu, 10 Feb 2022) Revision:18379 Build-Date:2022-02-11 02:31:13 URL:https://josm.openstreetmap.de/svn/trunk Identification: JOSM/1.5 (18379 en) Linux Debian GNU/Linux 11 (bullseye) Java version: 17.0.2+8-Debian-1deb11u1, Debian, OpenJDK 64-Bit Server VM
Change History (40)
by , 3 years ago
| Attachment: | ptv2_relation_loop_split.osm.xz added |
|---|
follow-up: 7 comment:1 by , 3 years ago
Still a problem, see changeset/123863615 and changeset/124259191 fixed in changeset/124933875
comment:2 by , 3 years ago
| Priority: | normal → major |
|---|
comment:4 by , 3 years ago
At least for PTv2 #19534 would be really helpful in general and could warn the users about the current problems with loops.
by , 3 years ago
| Attachment: | 21856.patch added |
|---|
comment:5 by , 3 years ago
| Milestone: | → 22.08 |
|---|---|
| Summary: | Split way: Wrong position of new member in PTv2 relation splitting a loop → [PATCH] Split way: Wrong position of new member in PTv2 relation splitting a loop |
@skyper: I believe I've got a patch for the problem. I'm going to check and see if it fixes any of the other tickets.
EDIT:
Didn't fix:
I might see about using RelationSorter to sort the new member instead of trying to guess. But attachment:21856.patch does fix the problem for this ticket.
comment:7 by , 3 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
The first example is fixed but my situation of comment 1 is still not working
Please find the second example attached and split at the nodes named "Split". Problem is still the order of the last way of the loop, which is in this case twice the identical way. Note, if you change the way direction everything works fine.
comment:8 by , 3 years ago
Using RelationSorter is probably going to have to be the fix. That way we aren't trying to do the same thing in different places.
comment:9 by , 3 years ago
| Milestone: | 22.08 → 22.09 |
|---|
comment:10 by , 3 years ago
| Milestone: | 22.09 → 22.10 |
|---|
comment:11 by , 3 years ago
| Milestone: | 22.10 → 22.11 |
|---|
comment:25 by , 18 months ago
| Milestone: | → 23.12 |
|---|
follow-up: 37 comment:35 by , 9 months ago
"Ad ways in a sorted manner" should be "Add ways in a sorted manner".
Also please do not implement any automatic/hidden member sorting (possibly mentioned in comment 8) as that may break correct order of members at other place that the user is touching. iD is notorious for constantly damaging valid route relations with loops.
comment:37 by , 9 months ago
Replying to aceman:
"Ad ways in a sorted manner" should be "Add ways in a sorted manner".
Oops. I guess I was "advertising ways in a sorted manner." Which still kind of makes sense, but add is what I meant.
Also please do not implement any automatic/hidden member sorting (possibly mentioned in comment 8) as that may break correct order of members at other place that the user is touching. iD is notorious for constantly damaging valid route relations with loops.
The only thing I was interested in doing with that was sorting new members (from the split). The previous behavior was not well-defined (IIRC, prior to r18539, the members were out-of-order most of the time for the cases discussed in this ticket).
In any case, it turns out that RelationSorter doesn't (currently) have the concept of locking the first/last members, so it doesn't work for this particular ticket (it has a tendency to make short loops in cases like this).
example file