Changeset 2521 in josm

Timestamp:

Nov 27, 2009 9:46:49 PM (4 years ago)

Author:

jttt

Message:

Fixed #3704 Relation memberships are not handled at all when a way is splitted or deleted by deleting a segment, rewritten DeleteAction a bit (delete commands are no longer build in updateCursor())

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/SplitWayAction.java (modified) (7 diffs)
• actions/mapmode/DeleteAction.java (modified) (11 diffs)
• command/DeleteCommand.java (modified) (5 diffs)
• data/osm/DatasetConsistencyTest.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/actions/SplitWayAction.java

@@ -28 +28 @@
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
+import org.openstreetmap.josm.data.osm.PrimitiveId;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
@@ -47 +48 @@
     private Way selectedWay;
     private List<Node> selectedNodes;
+
+    public static class SplitWayResult {
+        private final Command command;
+        private final List<? extends PrimitiveId> newSelection;
+
+        public SplitWayResult(Command command, List<? extends PrimitiveId> newSelection) {
+            this.command = command;
+            this.newSelection = newSelection;
+        }
+
+        public Command getCommand() {
+            return command;
+        }
+
+        public List<? extends PrimitiveId> getNewSelection() {
+            return newSelection;
+        }
+    }
 
     /**
@@ -267 +286 @@
         //Main.debug("way id: " + selectedWay.id);
 
+        SplitWayResult result = splitWay(selectedWay, wayChunks);
+        Main.main.undoRedo.add(result.getCommand());
+        getCurrentDataSet().setSelected(result.getNewSelection());
+    }
+
+    public static SplitWayResult splitWay(Way way, List<List<Node>> wayChunks) {
         // build a list of commands, and also a new selection list
         Collection<Command> commandList = new ArrayList<Command>(wayChunks.size());
-        Collection<Way> newSelection = new ArrayList<Way>(wayChunks.size());
+        List<Way> newSelection = new ArrayList<Way>(wayChunks.size());
 
         Iterator<List<Node>> chunkIt = wayChunks.iterator();
 
         // First, change the original way
-        Way changedWay = new Way(selectedWay);
+        Way changedWay = new Way(way);
         changedWay.setNodes(chunkIt.next());
-        commandList.add(new ChangeCommand(selectedWay, changedWay));
-        newSelection.add(selectedWay);
+        commandList.add(new ChangeCommand(way, changedWay));
+        newSelection.add(way);
 
         Collection<Way> newWays = new ArrayList<Way>();
@@ -283 +308 @@
         while (chunkIt.hasNext()) {
             Way wayToAdd = new Way();
-            wayToAdd.setKeys(selectedWay.getKeys());
+            wayToAdd.setKeys(way.getKeys());
             newWays.add(wayToAdd);
             wayToAdd.setNodes(chunkIt.next());
@@ -295 +320 @@
         // now copy all relations to new way also
 
-        for (Relation r : OsmPrimitive.getFilteredList(selectedWay.getReferrers(), Relation.class)) {
+        for (Relation r : OsmPrimitive.getFilteredList(way.getReferrers(), Relation.class)) {
             if (!r.isUsable()) {
                 continue;
@@ -308 +333 @@
             for (RelationMember rm : r.getMembers()) {
                 if (rm.isWay()) {
-                    if (rm.getMember() == selectedWay) {
+                    if (rm.getMember() == way) {
                         if (!("route".equals(type)) && !("multipolygon".equals(type))) {
                             warnme = true;
@@ -355 +380 @@
         }
 
-        Main.main.undoRedo.add(
-                new SequenceCommand(tr("Split way {0} into {1} parts",
-                        selectedWay.getDisplayName(DefaultNameFormatter.getInstance()),
-                        wayChunks.size()),
-                        commandList));
-        getCurrentDataSet().setSelected(newSelection);
+
+        return new SplitWayResult(new SequenceCommand(tr("Split way {0} into {1} parts",
+                way.getDisplayName(DefaultNameFormatter.getInstance()),
+                wayChunks.size()),
+                commandList), newSelection);
     }
 

trunk/src/org/openstreetmap/josm/actions/mapmode/DeleteAction.java

@@ -13 +13 @@
 import java.awt.event.KeyEvent;
 import java.awt.event.MouseEvent;
-import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 
 import org.openstreetmap.josm.Main;
@@ -21 +19 @@
 import org.openstreetmap.josm.command.DeleteCommand;
 import org.openstreetmap.josm.data.osm.Node;
-import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Relation;
-import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.osm.WaySegment;
 import org.openstreetmap.josm.gui.MapFrame;
@@ -63 +59 @@
     private MouseEvent oldEvent = null;
 
-    private enum Cursors {
-        none,
-        node,
-        segment,
-        way_node_only,
-        way_normal,
-        way_only;
-
-        private Cursor c = null;
-        // This loads and caches the cursor for each
+    private enum DeleteMode {
+        none("delete"),
+        segment("delete_segment"),
+        node("delete_node"),
+        node_with_references("delete_node"),
+        way("delete_way_only"),
+        way_with_references("delete_way_normal"),
+        way_with_nodes("delete_way_node_only");
+
+        private final Cursor c;
+
+        private DeleteMode(String cursorName) {
+            c = ImageProvider.getCursor("normal", cursorName);
+        }
+
         public Cursor cursor() {
-            if(c == null) {
-                String nm = "delete_" + this.name().toLowerCase();
-                // "None" has no special icon
-                nm = nm.equals("delete_none") ? "delete" : nm;
-                this.c = ImageProvider.getCursor("normal", nm);
-            }
             return c;
         }
     }
-    private Cursors currCursor = Cursors.none;
+    private DeleteMode currentMode = DeleteMode.none;
+
+    private static class DeleteParameters {
+        DeleteMode mode;
+        Node nearestNode;
+        WaySegment nearestSegment;
+    }
 
     /**
@@ -110 +111 @@
         try {
             Toolkit.getDefaultToolkit().addAWTEventListener(this, AWTEvent.KEY_EVENT_MASK);
-        } catch (SecurityException ex) {}
-
-        currCursor = Cursors.none;
+        } catch (SecurityException ex) {
+            System.out.println(ex);
+        }
+
+        currentMode = DeleteMode.none;
     }
 
@@ -121 +124 @@
         try {
             Toolkit.getDefaultToolkit().removeAWTEventListener(this);
-        } catch (SecurityException ex) {}
+        } catch (SecurityException ex) {
+            System.out.println(ex);
+        }
     }
 
@@ -172 +177 @@
      *
      * @param MouseEvent
-     * @parm int modifiers
+     * @param int modifiers
      */
     private void updateCursor(MouseEvent e, int modifiers) {
-        if (!Main.isDisplayingMapView()) {
-            return;
-        }
+        if (!Main.isDisplayingMapView())
+            return;
         if(!Main.map.mapView.isActiveLayerVisible() || e == null)
             return;
@@ -184 +188 @@
         //cleanOldHighlights();
 
-        Command c = buildDeleteCommands(e, modifiers, true);
-        if(c == null) {
-            setCursor(Cursors.none);
-            return;
-        }
-
-        Collection<OsmPrimitive> prims = new HashSet<OsmPrimitive>();
-        Collection<OsmPrimitive> mods = new HashSet<OsmPrimitive>();
-        c.fillModifiedData(mods, prims, prims);
-
-        if(prims.size() == 0 && mods.size() == 0) {
-            // Default-Cursor
-            setCursor(Cursors.none);
-            return;
-        }
-
-        // There are no deleted parts if solely a way segment is deleted
-        // This is no the case when actually deleting only a segment but that
-        // segment happens to be the whole way. This is an acceptable error
-        // though
-        if(prims.size() == 0) {
-            setCursor(Cursors.segment);
-        } else if(prims.size() == 1 && prims.toArray()[0] instanceof Node) {
-            setCursor(Cursors.node);
-        } else if(prims.size() == 1 && prims.toArray()[0] instanceof Way) {
-            setCursor(Cursors.way_only);
-        } else {
-            // Decide between non-accel click where "useless" nodes are deleted
-            // and ctrl-click where nodes and ways are deleted
-            boolean ctrl = (modifiers & ActionEvent.CTRL_MASK) != 0;
-            if(ctrl) {
-                setCursor(Cursors.way_node_only);
-            } else {
-                setCursor(Cursors.way_normal);
-            }
-
-        }
+        DeleteParameters parameters = getDeleteParameters(e, modifiers);
+        setCursor(parameters.mode);
 
         // Needs to implement WaySegment highlight first
@@ -306 +275 @@
     }
 
+    private DeleteParameters getDeleteParameters(MouseEvent e, int modifiers) {
+        // Note: CTRL is the only modifier that is checked in MouseMove, don't
+        // forget updating it there
+        boolean ctrl = (modifiers & ActionEvent.CTRL_MASK) != 0;
+        boolean shift = (modifiers & ActionEvent.SHIFT_MASK) != 0;
+        boolean alt = (modifiers & (ActionEvent.ALT_MASK|InputEvent.ALT_GRAPH_MASK)) != 0;
+
+        DeleteParameters result = new DeleteParameters();
+
+        result.nearestNode = Main.map.mapView.getNearestNode(e.getPoint());
+        if (result.nearestNode == null) {
+            result.nearestSegment = Main.map.mapView.getNearestWaySegment(e.getPoint());
+            if (result.nearestSegment != null) {
+                if (shift) {
+                    result.mode = DeleteMode.segment;
+                } else if (ctrl) {
+                    result.mode = DeleteMode.way_with_references;
+                } else {
+                    result.mode = alt?DeleteMode.way:DeleteMode.way_with_nodes;
+                }
+            } else {
+                result.mode = DeleteMode.none;
+            }
+        } else if (ctrl) {
+            result.mode = DeleteMode.node_with_references;
+        } else {
+            result.mode = DeleteMode.node;
+        }
+
+        return result;
+    }
+
     /**
      * This function takes any mouse event argument and builds the list of elements
@@ -316 +317 @@
      */
     private Command buildDeleteCommands(MouseEvent e, int modifiers, boolean silent) {
-        // Note: CTRL is the only modifier that is checked in MouseMove, don't
-        // forget updating it there
-        boolean ctrl = (modifiers & ActionEvent.CTRL_MASK) != 0;
-        boolean shift = (modifiers & ActionEvent.SHIFT_MASK) != 0;
-        boolean alt = (modifiers & (ActionEvent.ALT_MASK|InputEvent.ALT_GRAPH_MASK)) != 0;
-
-        OsmPrimitive sel = Main.map.mapView.getNearestNode(e.getPoint());
-        Command c = null;
-        if (sel == null) {
-            WaySegment ws = Main.map.mapView.getNearestWaySegment(e.getPoint());
-            if (ws != null) {
-                if (shift) {
-                    c = DeleteCommand.deleteWaySegment(getEditLayer(),ws);
-                } else if (ctrl) {
-                    c = DeleteCommand.deleteWithReferences(getEditLayer(),Collections.singleton((OsmPrimitive)ws.way),true);
-                } else {
-                    c = DeleteCommand.delete(getEditLayer(),Collections.singleton((OsmPrimitive)ws.way), !alt, silent);
-                }
-            }
-        } else if (ctrl) {
-            c = DeleteCommand.deleteWithReferences(getEditLayer(),Collections.singleton(sel));
-        } else {
-            c = DeleteCommand.delete(getEditLayer(),Collections.singleton(sel), !alt, silent);
-        }
-
-        return c;
+        DeleteParameters parameters = getDeleteParameters(e, modifiers);
+        switch (parameters.mode) {
+        case node:
+            return DeleteCommand.delete(getEditLayer(),Collections.singleton(parameters.nearestNode), false, silent);
+        case node_with_references:
+            return DeleteCommand.deleteWithReferences(getEditLayer(),Collections.singleton(parameters.nearestNode));
+        case segment:
+            return DeleteCommand.deleteWaySegment(getEditLayer(), parameters.nearestSegment);
+        case way:
+            return DeleteCommand.delete(getEditLayer(), Collections.singleton(parameters.nearestSegment.way), false, silent);
+        case way_with_nodes:
+            return DeleteCommand.delete(getEditLayer(), Collections.singleton(parameters.nearestSegment.way), true, silent);
+        case way_with_references:
+            return DeleteCommand.deleteWithReferences(getEditLayer(),Collections.singleton(parameters.nearestSegment.way),true);
+        default:
+            return null;
+        }
     }
 
@@ -351 +343 @@
      * @param c
      */
-    private void setCursor(final Cursors c) {
-        if(currCursor.equals(c) || (!drawTargetCursor && currCursor.equals(Cursors.none)))
+    private void setCursor(final DeleteMode c) {
+        if(currentMode.equals(c) || (!drawTargetCursor && currentMode.equals(DeleteMode.none)))
             return;
         try {
@@ -366 +358 @@
                 }
             });
-            currCursor = c;
+            currentMode = c;
         } catch(Exception e) {}
     }

trunk/src/org/openstreetmap/josm/command/DeleteCommand.java

@@ -23 +23 @@
 
 import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.actions.SplitWayAction;
 import org.openstreetmap.josm.data.osm.BackreferencedDataSet;
 import org.openstreetmap.josm.data.osm.Node;
@@ -253 +254 @@
     protected static Collection<Node> computeNodesToDelete(BackreferencedDataSet backreferences, OsmDataLayer layer, Collection<OsmPrimitive> primitivesToDelete) {
         Collection<Node> nodesToDelete = new HashSet<Node>();
-        //CollectBackReferencesVisitor v = new CollectBackReferencesVisitor(layer.data, false);
         for (Way way : OsmPrimitive.getFilteredList(primitivesToDelete, Way.class)) {
             for (Node n : way.getNodes()) {
@@ -259 +259 @@
                     continue;
                 }
-                //v.initialize();
-                //n.visit(v);
                 Collection<OsmPrimitive> referringPrimitives = backreferences.getParents(n);
                 referringPrimitives.removeAll(primitivesToDelete);
@@ -385 +383 @@
     public static Command deleteWaySegment(OsmDataLayer layer, WaySegment ws) {
         if (ws.way.getNodesCount() < 3)
-            return new DeleteCommand(layer, Collections.singleton(ws.way));
+            return delete(layer, Collections.singleton(ws.way));
 
         if (ws.way.firstNode() == ws.way.lastNode()) {
@@ -416 +414 @@
             return new ChangeCommand(ws.way, wnew);
         } else {
-            Collection<Command> cmds = new LinkedList<Command>();
-
-            wnew.setNodes(n1);
-            cmds.add(new ChangeCommand(ws.way, wnew));
-
-            Way wnew2 = new Way();
-            wnew2.setKeys(wnew.getKeys());
-            wnew2.setNodes(n2);
-            cmds.add(new AddCommand(wnew2));
-
-            // FIXME: relation memberships are not handled
-
-            return new SequenceCommand(tr("Split way segment"), cmds);
+            List<List<Node>> chunks = new ArrayList<List<Node>>(2);
+            chunks.add(n1);
+            chunks.add(n2);
+            return SplitWayAction.splitWay(ws.way, chunks).getCommand();
         }
     }

trunk/src/org/openstreetmap/josm/data/osm/DatasetConsistencyTest.java

@@ -26 +26 @@
     private void checkReferrers() {
         for (Way way:dataSet.getWays()) {
-            for (Node n:way.getNodes()) {
-                if (!n.getReferrers().contains(way)) {
-                    writer.println(String.format("%s is part of %s but is not in referrers", n, way));
+            if (!way.isDeleted()) {
+                for (Node n:way.getNodes()) {
+                    if (!n.getReferrers().contains(way)) {
+                        writer.println(String.format("%s is part of %s but is not in referrers", n, way));
+                    }
                 }
             }
@@ -34 +36 @@
 
         for (Relation relation:dataSet.getRelations()) {
-            for (RelationMember m:relation.getMembers()) {
-                if (!m.getMember().getReferrers().contains(relation)) {
-                    writer.println(String.format("%s is part of %s but is not in referrers", m.getMember(), relation));
+            if (!relation.isDeleted()) {
+                for (RelationMember m:relation.getMembers()) {
+                    if (!m.getMember().getReferrers().contains(relation)) {
+                        writer.println(String.format("%s is part of %s but is not in referrers", m.getMember(), relation));
+                    }
                 }
             }