Opened 11 years ago
Closed 5 years ago
#9599 closed defect (fixed)
[Patch] "Join Overlapping Areas" creates a new way when joining 2 buildings
Reported by: | didier2020 | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | 20.02 |
Component: | Core | Version: | tested |
Keywords: | join area | Cc: | hjart |
Description (last modified by )
for this exemple, "Shift J" (Join overlapping areas) create a new ways (id=0)
why not
using "R" (Inverser le chemins) on one of the 2 ways => "Shift J" modify one ways and delete the other
it's seems better ?
Attachments (5)
Change History (29)
by , 11 years ago
comment:1 by , 11 years ago
Priority: | minor → normal |
---|
comment:2 by , 11 years ago
Description: | modified (diff) |
---|---|
Summary: | "Maj J" create a new way → "Join Overlapping Areas" creates a new way when joining 2 buildings |
comment:3 by , 11 years ago
Milestone: | → 14.02 |
---|
follow-up: 7 comment:6 by , 11 years ago
Seems to be complex. JoinArea algorithm first split way into small piece, then clears unnecessary ways, join others.
According to splitting algorithm, initial way became the first piece, and could (or not) be deleted !
If it is not deleted, it will be used by the join algorithm.
Changing way direction, change the order of broken pieces of way, so initial way could be deleted in the first case, and kept in the other.
Somehow, the algorithm use the current dataset as a working space, leaving in undo/redo history many temporary way.
comment:7 by , 11 years ago
Replying to Balaitous:
Seems to be complex. JoinArea algorithm first split way into small piece, then clears unnecessary ways, join others.
According to splitting algorithm, initial way became the first piece, and could (or not) be deleted !
If it is not deleted, it will be used by the join algorithm.
Changing way direction, change the order of broken pieces of way, so initial way could be deleted in the first case, and kept in the other.
How about temporally storing the lowest id ? In fact undo/redo needs to keep it somewhere anyway.
follow-up: 9 comment:8 by , 11 years ago
I have put an exemple :
As you can see many way were created, and then deleted. Where are the 2 initials way ?
This is what I call using the dataset as a working space.
I think it should be better to split the algorithm into two parts :
- A geometry part, witch operate without any knowledge of dataset -> separate of JoinAreaAction, reusable for other purpose;
- A dataset part, check of user selection, tag management and update of geometry -> the aim of JoinAreaAction.
comment:9 by , 11 years ago
I think it should be better to split the algorithm into two parts :
- A geometry part, witch operate without any knowledge of dataset -> separate of JoinAreaAction, reusable for other purpose;
- A dataset part, check of user selection, tag management and update of geometry -> the aim of JoinAreaAction.
+1
comment:10 by , 11 years ago
Milestone: | 14.04 → 14.05 |
---|
Feel free to improve the algorithms :) But starting from next week :)
comment:11 by , 11 years ago
I do not consider the number of temporary ways created critical. When joining a way existing on the server (having a "real" id) with another way, the id of the existing way is persisted.
Code clean-ups, especially when bugs are removed or features are added, are very welcome. However, one has to make one's choice whether it's worth the effort in this case. ;-)
comment:12 by , 11 years ago
Milestone: | 14.05 → 14.06 |
---|
comment:13 by , 11 years ago
Milestone: | 14.06 |
---|
I'd remove this nice-to-have algorithm-code-cleanup from the release planning …
comment:15 by , 9 years ago
Cc: | added |
---|
comment:16 by , 9 years ago
Uh, 3 years since it was first reported? Sorry for maybe being a bit too noisy, but I just feel like letting you know that I'd appreciate if the id of the older building/polygon was kept.
comment:17 by , 5 years ago
Keywords: | join area added |
---|
Still reproducible:
Relative:URL: ^/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2020-02-03 22:02:21 +0100 (Mon, 03 Feb 2020) Revision:15813 Build-Date:2020-02-04 02:30:59 URL:https://josm.openstreetmap.de/svn/trunk
by , 5 years ago
Attachment: | perplexing.osm added |
---|
I think the results of this action are more or less unpredictable for non-trivial cases, and are also sometimes perplexing with trivial cases. With the attached example nothing is changed (that's correct) but one way is marked as modified and would be uploaded.
by , 5 years ago
Attachment: | 9599.patch added |
---|
comment:18 by , 5 years ago
Milestone: | → 20.02 |
---|---|
Summary: | "Join Overlapping Areas" creates a new way when joining 2 buildings → [Patch] "Join Overlapping Areas" creates a new way when joining 2 buildings |
With the patch, the oldest way should be kept for each created polygon. Quite a lot of new code is needed for that.
Part of the problem is that CombineWayAction selects the first old way and not the oldest. I don't dare to change that.
I'll add unit tests and maybe I can also fix the problem shown in perplexing.osm.
comment:21 by , 5 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
There is still a problem when inner ways of multipolygons are involved.
by , 5 years ago
Attachment: | join-mp-ways.osm added |
---|
special case with multipolygon: select all ways and join areas
comment:23 by , 5 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
There is a regression: When I select the inner ways of join-mp-ways.osm and join them I see a popup asking me to confirm the removal of a member. Ths should not appear.
exemple fusion