Modify

Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#10730 closed enhancement (fixed)

Better use of existing object id while splitting

Reported by: josm@… Owned by: team
Priority: major Milestone: 15.10
Component: Core Version:
Keywords: template_report Cc: Klumbumbus

Description

If i split a way the longest part (with most nodes - to be simple) should get the old OSM id. This way the history is keep better.

Right now the first part (in term of way direction) gets the id, but often i split away a tiny part and delete it.

Please provide any additional information below. Attach a screenshot if possible.

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2014-10-21 19:23:06
Last Changed Author: Don-vip
Revision: 7643
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Relative URL: ^/trunk
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2014-10-21 21:19:16 +0200 (Tue, 21 Oct 2014)
Last Changed Rev: 7643

Identification: JOSM/1.5 (7643 de) Windows 7 64-Bit
Memory Usage: 212 MB / 247 MB (38 MB allocated, but free)
Java version: 1.7.0_71, Oracle Corporation, Java HotSpot(TM) Client VM
VM arguments: [-Djava.security.manager, -Djava.security.policy=file:C:\Program Files (x86)\Java\jre7\lib\security\javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>\bin, -Djnlpx.origFilenameArg=C:\Users\zuse\AppData\LocalLow\Sun\Java\Deployment\cache\6.0\56\1ee8cfb8-51aa42b0, -Djnlpx.remove=false, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.splashport=55784, -Djnlp.application.href=https://josm.openstreetmap.de/download/josm.jnlp, -Djnlpx.jvm=<java.home>\bin\javaw.exe, -Djnlpx.vmargs=LURqYXZhLnV0aWwuQXJyYXlzLnVzZUxlZ2FjeU1lcmdlU29ydD10cnVlAC1Eam5scC5hcHBsaWNhdGlvbi5ocmVmPWh0dHBzOi8vam9zbS5vcGVuc3RyZWV0bWFwLmRlL2Rvd25sb2FkL2pvc20uam5scAA=]
Dataset consistency test: No problems found

Plugins:
- FastDraw (30762)
- FixAddresses (30737)
- OpeningHoursEditor (30737)
- PicLayer (30762)
- RoadSigns (30738)
- areaselector (1412926941)
- buildings_tools (30762)
- imagery_offset_db (30762)
- junctionchecking (30762)
- lakewalker (30737)
- log4j (30762)
- mirrored_download (30762)
- notes (v0.9.4)
- public_transport (30762)
- reltoolbox (30762)
- reverter (30737)
- terracer (30737)
- todo (29154)
- turnrestrictions (30762)
- undelete (30762)
- utilsplugin2 (30762)
- waydownloader (30762)
- wikipedia (30738)

Last errors/warnings:
- E: Defekte Objektvorlage "frequency-Frequency" - Anzahl der Datenworte in 'display_values' und in 'values' müssen gleich sein
- E: Defekte Objektvorlage "frequency-Frequency" - Anzahl der Datenworte in 'display_values' und in 'values' müssen gleich sein
- E: Defekte Objektvorlage "frequency-Frequency" - Anzahl der Datenworte in 'display_values' und in 'values' müssen gleich sein
- E: Defekte Objektvorlage "frequency-Frequency" - Anzahl der Datenworte in 'display_values' und in 'values' müssen gleich sein
- E: Defekte Objektvorlage "railway:signal:speed_limit:speed-Speed (km/h, in case of light signals separate the possible values (number or 'off') with ';')" - Anzahl der Datenworte in 'display_values' und in 'values' müssen gleich sein

Attachments (4)

10730-alpha.patch (9.9 KB ) - added by simon04 8 years ago.
2015-09-24-000920_351x126_scrot.png (6.0 KB ) - added by simon04 8 years ago.
split.png (30.0 KB ) - added by Klumbumbus 8 years ago.
split2.png (36.5 KB ) - added by Klumbumbus 8 years ago.

Download all attachments as: .zip

Change History (32)

comment:1 by Klumbumbus, 9 years ago

Cc: Klumbumbus added

comment:2 by skyper, 9 years ago

At least ATM it is predictable which part remains with the id.
Not sure if it will be easier with above mentioned method, especially once you end up with several parts with almost equal lenght. How would you determine the lenght by the way, counting the nodes or calculating the real lenght ?

In your mentioned case I usually change the direction of the way twice (before splitting and after it again). Have to admit that it is kind of cumbersome and JOSM still needs the option to solve several direction conflicts at once if you change the direction of several ways at once.

comment:3 by cmuelle8, 9 years ago

Just a rough thought on this:

  • let the current behavior unmodified, i.e. first part of split ways gets old way id (current behavior)
  • instead, modify delete action to have a look at the edit history / the undo buffer:

If a way is deleted and its

  • old way id is included/found in the edit history and
  • a split way action is the youngest action recorded for this id

Then

  • assign the old way id to one of the remaining parts

Obviously this id swap itself needs to be recorded in the edit history, atomic to the delete action. Whole Expl.:

  • If a delete action which transfered the way id to a remaining part (i.e. way) of a split way is undone by the user, then the remaining part must be reassigned a new way id, and the (to be) undeleted way needs its original, old way id restored.
  • Anything else would probably break the undo chain.

It's just a thought. First, the edit history object of a split-way-event needs to be checked to determine if all needed information is included to carry out such an operation. If negative, maybe advance this object first to include the necessary info.

by simon04, 8 years ago

Attachment: 10730-alpha.patch added

by simon04, 8 years ago

comment:4 by simon04, 8 years ago

Milestone: 15.10
Summary: better use of existing object id while splitting[Alpha patch] better use of existing object id while splitting

attachment:10730-alpha.patch is a preliminary patch which allows experts to select the chunk of the way which should reuse the existing id. What has to be done is to somehow select/highlight the selected chunk; this is tricky since its Way object is not yet part of the dataset …


comment:5 by simon04, 8 years ago

Priority: minormajor

comment:6 by simon04, 8 years ago

Resolution: fixed
Status: newclosed

In 8886/josm:

fix #10730 - Way splitting: reuse id/history of existing object for longest split segment and allow expert users to select this segment

by Klumbumbus, 8 years ago

Attachment: split.png added

comment:7 by Klumbumbus, 8 years ago

Resolution: fixed
Status: closedreopened

First of all, this is a great improvement, thanks. Just one thing:

In the preferences at "look and feel" I checked "Show object id in selection list". In the new split window there is now displayed twice [id: 0], which is a bit confusing. I think this should either be hidden or the selected segment should display the new/old id.


comment:8 by Klumbumbus, 8 years ago

Another thing:
With each selected node the space on top of the list becomes bigger.


Revision: 8892
Repository Root: http://josm.openstreetmap.de/svn
Relative URL: ^/trunk
Last Changed Author: simon04
Last Changed Date: 2015-10-17 18:39:12 +0200 (Sat, 17 Oct 2015)
Build-Date: 2015-10-18 01:32:20
URL: http://josm.openstreetmap.de/svn/trunk
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last Changed Rev: 8892

Identification: JOSM/1.5 (8892 en) Windows 7 32-Bit
Memory Usage: 380 MB / 742 MB (177 MB allocated, but free)
Java version: 1.8.0_60, Oracle Corporation, Java HotSpot(TM) Client VM
VM arguments: [-Djava.security.manager, -Djava.security.policy=file:C:\Program Files\Java\jre1.8.0_60\lib\security\javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>\bin, -Djnlpx.origFilenameArg=C:\Program Files\josm-latest-bla.jnlp, -Djnlpx.remove=true, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.heapsize=256m,768m, -Djnlpx.splashport=63939, -Djnlpx.jvm=<java.home>\bin\javaw.exe, -Djnlpx.vmargs=LURqYXZhLnV0aWwuQXJyYXlzLnVzZUxlZ2FjeU1lcmdlU29ydD10cnVlAA==]
Dataset consistency test: No problems found

by Klumbumbus, 8 years ago

Attachment: split2.png added

comment:9 by simon04, 8 years ago

Resolution: fixed
Status: reopenedclosed

In 8897/josm:

fix #10730 - Way splitting: improve dialog layout and hide id of new ways

comment:10 by simon04, 8 years ago

Thank you for your testing and the feedback. Both issues should be addressed in r8897.

comment:11 by Don-vip, 8 years ago

In 8930/josm:

see #10730 - see #11992 - add regression unit test

comment:12 by Klumbumbus, 8 years ago

Resolution: fixed
Status: closedreopened
Summary: [Alpha patch] better use of existing object id while splittingBetter use of existing object id while splitting

I think there should be an option also for expert users to hide the dialog. I think in 99% of all cases it is fine when the longest part keeps the history. As a expert user to click on OK everytime you split a way is a bit anoying.

comment:13 by simon04, 8 years ago

In 8955/josm:

fix #11992 see #10730 - Joining Overlapping Areas results in RuntimeException

JoinAreasAction somehow requires that the first way chunk keeps the id of the split way.
SplitWayAction#Strategy allows to specify how to determine the way chunk which keeps the to id.

comment:14 by simon04, 8 years ago

Replying to Klumbumbus:

an option also for expert users to hide the dialog

Good point. I'd keep this for the next release to not introduce new i18n strings and further delay the release.

comment:15 by Klumbumbus, 8 years ago

What about a simple key in the advanced preferences for now (which does not create a i18n string, afaik)?

comment:16 by ijsb, 8 years ago

In addition to requiring an extra click for every split, this change also impacts route relations.

When splitting a way, the way with id 0 is inserted in parent relations before the existing id. This used to result in the correct sort order, but may now break the sort order without warning - depending on the section you pick when splitting a way. Would be nice if this could be fixed before the release referred to in comment:14.

comment:17 by simon04, 8 years ago

In 8963/josm:

see #10730 - Way splitting: hide id of new ways for non English languages

comment:18 by simon04, 8 years ago

In 8964/josm:

see #10730 - Way splitting: add preference key to disable segment selection dialog

Setting the preference key splitway.segment-selection-dialog to false
disables the segment selection dialog and selects the longest way chunk.

comment:19 by Don-vip, 8 years ago

Resolution: fixed
Status: reopenedclosed

comment:20 by Klumbumbus, 8 years ago

Don't you think the relation order issue (comment:16) should be fixed before stable release?

comment:21 by Don-vip, 8 years ago

I thought all was fixed. Simon what do you think?

comment:22 by Klumbumbus, 8 years ago

I think this is a serious regression. Example:
Download here: http://www.openstreetmap.org/way/203396009 and add to this way three waynodes. Split the way at the most eastern new waynode. See how the order is wrong in the associated bus route relations.

comment:23 by Don-vip, 8 years ago

Resolution: fixed
Status: closedreopened

comment:24 by simon04, 8 years ago

In 8965/josm:

see #10730 - Way splitting: fix and test (route) relation adaption

comment:25 by simon04, 8 years ago

While addressing comment:15, the issue from comment:16 has been reported. I'm sorry for not stating more clearly that the latter hasn't been resolved.

The issue from comment:16 is rather severe since it breaks route relations anytime unless the first split chunk reused the old way's history.

comment:26 by Don-vip, 8 years ago

My fault, no need to apologize :)

comment:27 by simon04, 8 years ago

Resolution: fixed
Status: reopenedclosed

All issues should be fixed – hopefully :)

comment:28 by simon04, 8 years ago

Ticket #8135 has been marked as a duplicate of this ticket.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.