Modify

Opened 13 years ago

Closed 8 years ago

Last modified 8 years ago

#6447 closed defect (fixed)

selection problem: Node in inactive instead of active layer moved.

Reported by: skyper Owned by: team
Priority: major Milestone: 17.05
Component: Core Version: latest
Keywords: select node layer Cc:

Description (last modified by xeen)

  1. select downloaded node and move it. (node still selected)
  2. download same node in new layer, select and move it.

Node in older inactive layer is moved instead of the one in the new active layer.

Using /usr/lib/jvm/java-6-openjdk/bin/java to execute josm.
Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2011-06-08 01:31:45
Last Changed Author: bastiK
Revision: 4127
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2011-06-07 23:29:54 +0200 (Tue, 07 Jun 2011)
Last Changed Rev: 4127

GET http://api.openstreetmap.org/api/capabilities... OK
Communications with http://api.openstreetmap.org/api established using protocol version 0.6.
Could not get presets icon ./icons/_neu.png
Could not get presets icon ./icons/vehicle_inspection.png
Could not get presets icon ./icons/buero.png
Could not get presets icon ./icons/employment_agency.png
Could not get presets icon ./icons/architect.png
Could not get presets icon ./icons/government.png
Could not get presets icon ./icons/research.png
Could not get presets icon ./icons/estate_agent.png
Could not get presets icon ./icons/it.png
Could not get presets icon ./icons/ngo.png
Could not get presets icon ./icons/quango.png
Could not get presets icon ./icons/company.png
Could not get presets icon ./icons/lawyer.png
Could not get presets icon ./icons/travel_agent.png
Could not get presets icon ./icons/accountant.png
Could not get presets icon ./icons/telecommunication.png
Could not get presets icon ./icons/insurance.png
Could not get presets icon ./icons/newspaper.png
Could not get presets icon ./icons/ice_cream.png
Could not get presets icon ./icons/dog_park.png
Could not get presets icon ./icons/hospice.png
Could not get presets icon ./icons/trade.png
Could not get presets icon ./icons/funeral_directors.png
Could not get presets icon ./icons/photo_studio.png
Could not get presets icon ./icons/second_hand.png
Could not get presets icon ./icons/glaziery.png
Could not get presets icon ./icons/farm.png
Could not get presets icon ./icons/hunting.png
Could not get presets icon ./icons/beauty.png
Could not get presets icon ./icons/art.png
Could not get presets icon ./icons/massage.png
Could not get presets icon ./icons/furnace.png
Could not get presets icon ./icons/pawnbroker.png
Could not get presets icon ./icons/interior_decoration.png
Could not get presets icon ./icons/tattoo.png
Could not get presets icon ./icons/pet.png
Could not get presets icon ./icons/aed.png
Could not get presets icon ./icons/blacksmith.png
Could not get presets icon ./icons/shoemaker.png
Could not get presets icon ./icons/orchard.png
Could not get presets icon ./icons/fire_hose.png
Could not get presets icon ./icons/fire_extinguisher.png
Could not get presets icon ./icons/fire_hydrant.png
Could not get presets icon ./icons/phone.png
Could not get presets icon ./icons/ambulance_station.png
Could not get presets icon ./icons/siren.png
Could not get presets icon ./icons/group_home.png
Could not get presets icon ./icons/ambulantorycare.png
Could not get presets icon ./icons/social_living.png
Could not get presets icon ./icons/sheltered_workshop.png
Could not get presets icon ./icons/foodbank.png
Could not get presets icon ./icons/outpatient_care.png
Could not get presets icon ./icons/social_shelter.png
Could not get presets icon ./icons/outreach.png
Could not get presets icon ./icons/group_home.png
Could not get presets icon ./icons/toboggan.png
Could not get presets icon ./icons/chess.png
Could not get presets icon ./icons/skiing.png
Could not get presets icon ./icons/water_ski.png
Could not get presets icon ./icons/aquarium.png
Could not get presets icon ./icons/obelisk.png
Could not get presets icon ./icons/storage_tank.png
loading plugin 'reltoolbox' (version 25751)
loading plugin 'undelete' (version 26073)
loading plugin 'alignways' (version 25199)
loading plugin 'Curves' (version 16.master-4f7c5a0)
loading plugin 'reverter' (version 26093)
loading plugin 'buildings_tools' (version 25905)
loading plugin 'ParallelWay' (version 8.master-7a9a787)
loading plugin 'waydownloader' (version 25190)
loading plugin 'multipoly-convert' (version 25192)
Silent shortcut conflict: 'tools:multipolyconv' moved by 'tools:mirror' to 'Alt+Umschalt+M'.
loading plugin 'OpeningHoursEditor' (version 26002)
loading plugin 'utilsplugin2' (version 26051)
Silent shortcut conflict: 'tools:intway' moved by 'tools:alignways' to 'Alt+Umschalt+I'.
loading plugin 'terracer' (version 26029)
Silent shortcut conflict: 'tools:Terracer' moved by 'tool:revert' to 'Alt+Umschalt+T'.
Silent shortcut conflict: 'tools:ReverseTerrace' moved by 'tools:tagbuffer' to 'Alt+Umschalt+R'.
RemoteControl::Accepting connections on port 8111
Silent shortcut conflict: 'subwindow:selection' moved by 'tools:Terracer' to 'Alt+C'.
Silent shortcut conflict: 'subwindow:relations' moved by 'tools:ReverseTerrace' to 'Alt+R'.
Silent shortcut conflict: 'subwindow:conflict' moved by 'subwindow:selection' to 'Alt+Umschalt+C'.
Silent shortcut conflict: 'subwindow:mappaint' moved by 'tools:multipolyconv' to 'Alt+D'.
Silent shortcut conflict: 'reltoolbox:changerole' moved by 'tools:tagbuffer' to 'H'.
Silent shortcut conflict: 'reltoolbox:find' moved by 'system:find' to 'Strg+Alt+F'.
Silent shortcut conflict: 'mapmode:parallel' moved by 'tools:splitobject' to 'K'.
GET http://api.openstreetmap.org/api/0.6/relations?relations=957374
GET http://api.openstreetmap.org/api/0.6/relation/957374/full
org.openstreetmap.josm.io.OsmApiException: ResponseCode=408, Error Header=<Request timed out>
	at org.openstreetmap.josm.io.OsmServerReader.getInputStreamRaw(OsmServerReader.java:117)
	at org.openstreetmap.josm.io.OsmServerReader.getInputStream(OsmServerReader.java:48)
	at org.openstreetmap.josm.io.OsmServerObjectReader.parseOsm(OsmServerObjectReader.java:91)
	at org.openstreetmap.josm.gui.io.DownloadPrimitivesTask.realRun(DownloadPrimitivesTask.java:153)
	at org.openstreetmap.josm.gui.PleaseWaitRunnable.doRealRun(PleaseWaitRunnable.java:83)
	at org.openstreetmap.josm.gui.PleaseWaitRunnable.run(PleaseWaitRunnable.java:129)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
	at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
	at java.util.concurrent.FutureTask.run(FutureTask.java:166)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
	at java.lang.Thread.run(Thread.java:636)
failed loading 19/273110/191078 http://tile.openstreetmap.org/19/273110/191078.png
failed loading 19/273111/191078 http://tile.openstreetmap.org/19/273111/191078.png
failed loading 19/273111/191079 http://tile.openstreetmap.org/19/273111/191079.png
failed loading 19/273110/191079 http://tile.openstreetmap.org/19/273110/191079.png
failed loading 19/273109/191079 http://tile.openstreetmap.org/19/273109/191079.png
failed loading 19/273109/191078 http://tile.openstreetmap.org/19/273109/191078.png
failed loading 19/273109/191077 http://tile.openstreetmap.org/19/273109/191077.png
failed loading 19/273110/191077 http://tile.openstreetmap.org/19/273110/191077.png
GET http://api.openstreetmap.org/api/0.6/map?bbox=7.531487299999999,43.7840925,7.5317073,43.7842474
Keystroke pressed H is already assigned to relcontext.RelContextDialog$EnterRoleAction@1790ce9, will be overridden by relcontext.RelContextDialog$EnterRoleAction@ce88d2
Keystroke pressed EQUALS is already assigned to relcontext.actions.AddRemoveMemberAction@1603bdc, will be overridden by relcontext.actions.AddRemoveMemberAction@1789a96
Keystroke shift ctrl pressed C is already assigned to relcontext.actions.CreateRelationAction@2bfa91, will be overridden by relcontext.actions.CreateRelationAction@129645a
Keystroke ctrl pressed B is already assigned to relcontext.actions.CreateMultipolygonAction@aa77d9, will be overridden by relcontext.actions.CreateMultipolygonAction@e038c4
Keystroke ctrl alt pressed F is already assigned to relcontext.actions.FindRelationAction@6c2308, will be overridden by relcontext.actions.FindRelationAction@1499616
Keystroke shift pressed C is already assigned to org.openstreetmap.josm.plugins.curves.CurveAction@16c1227, will be overridden by org.openstreetmap.josm.plugins.curves.CurveAction@422510
failed loading 19/273112/191077 http://tile.openstreetmap.org/19/273112/191077.png
failed loading 19/273113/191077 http://tile.openstreetmap.org/19/273113/191077.png
failed loading 19/273113/191078 http://tile.openstreetmap.org/19/273113/191078.png
failed loading 19/273112/191078 http://tile.openstreetmap.org/19/273112/191078.png
failed loading 19/273111/191077 http://tile.openstreetmap.org/19/273111/191077.png
failed loading 19/273111/191076 http://tile.openstreetmap.org/19/273111/191076.png
failed loading 19/273112/191076 http://tile.openstreetmap.org/19/273112/191076.png
failed loading 19/273113/191076 http://tile.openstreetmap.org/19/273113/191076.png
GET http://api.openstreetmap.org/api/0.6/map?bbox=7.530511,43.7840189,7.5307739,43.784169999999996
failed loading 20/546222/382155 http://tile.openstreetmap.org/20/546222/382155.png
failed loading 20/546223/382155 http://tile.openstreetmap.org/20/546223/382155.png
failed loading 20/546223/382156 http://tile.openstreetmap.org/20/546223/382156.png
failed loading 20/546221/382156 http://tile.openstreetmap.org/20/546221/382156.png
failed loading 20/546222/382156 http://tile.openstreetmap.org/20/546222/382156.png
failed loading 20/546221/382155 http://tile.openstreetmap.org/20/546221/382155.png
failed loading 20/546221/382154 http://tile.openstreetmap.org/20/546221/382154.png
failed loading 20/546222/382154 http://tile.openstreetmap.org/20/546222/382154.png
GET http://api.openstreetmap.org/api/0.6/map?bbox=7.5315195,43.7841196,7.531680499999999,43.7842358
Keystroke pressed H is already assigned to relcontext.RelContextDialog$EnterRoleAction@ce88d2, will be overridden by relcontext.RelContextDialog$EnterRoleAction@21f46a
Keystroke pressed EQUALS is already assigned to relcontext.actions.AddRemoveMemberAction@1789a96, will be overridden by relcontext.actions.AddRemoveMemberAction@13598c3
Keystroke shift ctrl pressed C is already assigned to relcontext.actions.CreateRelationAction@129645a, will be overridden by relcontext.actions.CreateRelationAction@169674
Keystroke ctrl pressed B is already assigned to relcontext.actions.CreateMultipolygonAction@e038c4, will be overridden by relcontext.actions.CreateMultipolygonAction@197d63b
Keystroke ctrl alt pressed F is already assigned to relcontext.actions.FindRelationAction@1499616, will be overridden by relcontext.actions.FindRelationAction@94e4f4
Keystroke shift pressed C is already assigned to org.openstreetmap.josm.plugins.curves.CurveAction@422510, will be overridden by org.openstreetmap.josm.plugins.curves.CurveAction@aa168c
GET http://api.openstreetmap.org/api/0.6/relation/957374/history
failed loading 20/546225/382155 http://tile.openstreetmap.org/20/546225/382155.png
failed loading 20/546225/382156 http://tile.openstreetmap.org/20/546225/382156.png
failed loading 20/546226/382156 http://tile.openstreetmap.org/20/546226/382156.png
failed loading 20/546226/382155 http://tile.openstreetmap.org/20/546226/382155.png
failed loading 20/546226/382154 http://tile.openstreetmap.org/20/546226/382154.png
failed loading 20/546225/382154 http://tile.openstreetmap.org/20/546225/382154.png
failed loading 20/546224/382154 http://tile.openstreetmap.org/20/546224/382154.png
failed loading 20/546224/382155 http://tile.openstreetmap.org/20/546224/382155.png
GET http://api.openstreetmap.org/api/0.6/map?bbox=7.5315169,43.7841166,7.5316830999999995,43.7842388
failed loading 20/546224/382156 http://tile.openstreetmap.org/20/546224/382156.png


Attachments (1)

ticket-6447-demofix.patch (1.5 KB ) - added by olejorgenb 13 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 by skyper, 13 years ago

Just to make it sure:

Warnung: Fehlende Einstellungsdatei "/home2/test/.josm/preferences". Datei mit Standardeinstellungen wird erstellt.
Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2011-06-08 01:31:45
Last Changed Author: bastiK
Revision: 4127
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2011-06-07 23:29:54 +0200 (Tue, 07 Jun 2011)
Last Changed Rev: 4127

Warning: no preference 'proxy.policy' found.
The proxy will not be used.
GET http://api.openstreetmap.org/api/capabilities... OK
Communications with http://api.openstreetmap.org/api established using protocol version 0.6.
GET http://api.openstreetmap.org/api/0.6/map?bbox=-3.1777954,49.0954522,-3.0789185,49.1601547
GET http://api.openstreetmap.org/api/0.6/map?bbox=-3.1793928,49.093883299999995,-3.0773211,49.1617215

But you see some other problems (not all concerning JOSM) in the first log.

Cheers

comment:2 by stoecker, 13 years ago

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

comment:3 by olejorgenb, 13 years ago

Attached a "demofix". (quick and dirty, needs some rework)

The bug originates from re-using the previous MoveCommand. The check relies on OsmPrimitive.equals (indirectly through Set.equals), which only consider the primitive's id*, not its dataset membership.

\* Probably what's usually wanted?

by olejorgenb, 13 years ago

Attachment: ticket-6447-demofix.patch added

comment:4 by olejorgenb, 13 years ago

Not really sure if I like the current MoveCommand reuse behavior, btw.

Obviously you don't want to create a new undo checkpoint on each mousedragged event. The current behavior is to also reuse the movecommand across multiple separate (but consecutive) moves. (ie. move a node twice, use undo -> node moves back to the first, not last position)

comment:5 by olejorgenb, 13 years ago

What to do:

  1. Only reuse the old movecommand during a continuous stream of mousedragged events. (assuming no one switches layer while dragging, this should work)
  2. A variant of the extended equals test. A bit ugly, but it's probably enough to use Set.equals and only check that the dataset of the first elements matches. (I assume all elements in those collection will have the same dataset)
  3. Listen to layer changes and use a flag to disable the reuse when a layer has been changed.
  4. Expose Command.getLayer and use the layer as a check

Any preference?

Last edited 13 years ago by olejorgenb (previous) (diff)

comment:6 by Larry0ua, 13 years ago

I looked through the bug, and it looks like existence of two nodes with the same IDs at different layers is absolutely surprise for JOSM...
They are compared by their IDs in all internal sets, maps, etc... That's quite unusual bug.. should we rewrite Node.compareTo method? the same about ways etc.. afraid even to think about relations :)

comment:7 by xeen, 13 years ago

Description: modified (diff)

This is a duplicate of #4991. I worked on a fix for the latter, but the result is similar to olejorgenb’s patch. I believe option *4 would be the cleanest solution, but only expose MoveCommand.getLayer.

Option *1 has the disadvantage of changing JOSM behaviour and it’s not clear it would improve the situation (I know applications where the undo is this fine-grained. It’s annoying to have to hit undo five times before your action is actually undone. If this is really is an issue, this should be solved outside of this bug)

Option *2: I agree all nodes are likely to belong to the same data set, but we’d have to assert that. This is true as well for option *4, at least unless there’s some kind of check added that returns <null> if the nodes are on multiple layers (or data sets).

Option *3: sounds overcomplicated, given that all information we need is already stored elsewhere.

In the current form it takes O(n²) comparisons. Only asserting all items in each collection have the same data set and that these data sets are the same would bring it down to O(2n). This is a lot better, but still feels wrong given that this is executed each mouse move while dragging. It probably doesn’t matter, because n is reasonably small.

Still, I’d vote to go with *4 and enforce the same data set policy in MoveCommand’s constructor (throw an exception on error). I doubt there are any callers in JOSM that pass multi-data-set primitives to the move command, so this should be fine.

Comments?

Last edited 13 years ago by xeen (previous) (diff)

comment:8 by xeen, 13 years ago

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

comment:9 by skyper, 13 years ago

Priority: normalmajor

comment:10 by skyper, 12 years ago

Still reproducable: #7864 (skyper)

r5345

comment:11 by michael2402, 8 years ago

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

comment:12 by stoecker, 8 years ago

Milestone: 16.07

We should care for it now. We have introduced a lot of separation now, so situation should be different to 5 years ago :-)

comment:13 by Don-vip, 8 years ago

Milestone: 16.0716.08

comment:14 by Don-vip, 8 years ago

Milestone: 16.0816.09

comment:15 by simon04, 8 years ago

Milestone: 16.0916.10

comment:16 by simon04, 8 years ago

Milestone: 16.1016.11

Milestone renamed

comment:17 by Don-vip, 8 years ago

Milestone: 16.1116.12

Milestone renamed

comment:18 by Don-vip, 8 years ago

Milestone: 16.12

comment:19 by michael2402, 8 years ago

In 12052/josm:

See #6447: Fix same issue for the move command (shift+arrow)

comment:20 by michael2402, 8 years ago

Resolution: fixed
Status: newclosed

In 12053/josm:

Fix #6447: Ensure that only move commands in same layer are combined.

comment:21 by michael2402, 8 years ago

Milestone: 17.05

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.