Changeset 2113 in josm for trunk/src/org/openstreetmap/josm/actions

Timestamp:

2009-09-13T12:33:18+02:00 (15 years ago)

Author:

Gubaer

Message:

fixed #3469: Merge nodes produce bad results

File:

• trunk/src/org/openstreetmap/josm/actions/MergeNodesAction.java (modified) (3 diffs)

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

@@ -10 +10 @@
 import java.util.HashSet;
 import java.util.LinkedList;
+import java.util.List;
 import java.util.Set;
 
@@ -150 +151 @@
 
     /**
-     * Merges the nodes in <code>node</code> onto one of the nodes. Uses the dataset
-     * managed by <code>layer</code> as reference. <code>backreferences</code> is precomputed
-     * collection of all parent/child references in the dataset.
-     *
-     * @param layer layer the reference data layer. Must not be null.
-     * @param backreferences if null, backreferneces are first computed from layer.data; otherwise
-     *    backreferences.getSource() == layer.data must hold
-     * @param nodes the collection of nodes. Ignored if null.
-     * @param targetNode the target node the collection of nodes is merged to. Must not be null.
-     * @throw IllegalArgumentException thrown if layer is null
-     * @throw IllegalArgumentException thrown if  backreferences.getSource() != layer.data
-     */
-    public static Command mergeNodes(OsmDataLayer layer, BackreferencedDataSet backreferences, Collection<Node> nodes, Node targetNode) {
-        if (layer == null)
-            throw new IllegalArgumentException(tr("parameter ''{0}'' must not be null", "nodes"));
-        if (targetNode == null)
-            throw new IllegalArgumentException(tr("parameter ''{0}'' must not be null", "targetNode"));
-        if (nodes == null)
-            return null;
-        if (backreferences == null) {
-            backreferences = new BackreferencedDataSet(layer.data);
-            backreferences.build();
-        }
-
-        Set<RelationToChildReference> relationToNodeReferences = backreferences.getRelationToChildReferences(nodes);
-
-        // build the tag collection
-        //
-        TagCollection nodeTags = TagCollection.unionOfAllPrimitives(nodes);
-        completeTagCollectionWithMissingTags(nodeTags, nodes);
-        TagCollection nodeTagsToEdit = new TagCollection(nodeTags);
-        completeTagCollectionForEditing(nodeTagsToEdit);
-
-        // launch a conflict resolution dialog, if necessary
-        //
-        CombinePrimitiveResolverDialog dialog = CombinePrimitiveResolverDialog.getInstance();
-        dialog.getTagConflictResolverModel().populate(nodeTagsToEdit);
-        dialog.getRelationMemberConflictResolverModel().populate(relationToNodeReferences);
-        dialog.setTargetPrimitive(targetNode);
-        dialog.prepareDefaultDecisions();
-        if (! nodeTags.isApplicableToPrimitive() || relationToNodeReferences.size() > 1) {
-            dialog.setVisible(true);
-            if (dialog.isCancelled())
-                return null;
-        }
-        LinkedList<Command> cmds = new LinkedList<Command>();
-
-        // the nodes we will have to delete
-        //
-        Collection<OsmPrimitive> nodesToDelete = new HashSet<OsmPrimitive>(nodes);
-        nodesToDelete.remove(targetNode);
-
-        // change the ways referring to at least one of the merge nodes
-        //
-        Collection<Way> waysToDelete= new HashSet<Way>();
-        for (Way w : OsmPrimitive.getFilteredList(backreferences.getParents(nodesToDelete), Way.class)) {
-            // OK - this way contains one or more nodes to change
+     * Fixes the parent ways referring to one of the nodes.
+     * 
+     * Replies null, if the ways could not be fixed, i.e. because a way would have to be delted
+     * which is referred to by a relation.
+     * 
+     * @param backreferences the backreference data set
+     * @param nodesToDelete the collection of nodes to be deleted
+     * @param targetNode the target node the other nodes are merged to
+     * @return a list of command; null, the ways could not be fixed
+     */
+    protected static List<Command> fixParentWays(BackreferencedDataSet backreferences, Collection<OsmPrimitive> nodesToDelete, Node targetNode) {
+        List<Command> cmds = new ArrayList<Command>();
+        Set<Way> waysToDelete = new HashSet<Way>();
+
+        for (Way w: OsmPrimitive.getFilteredList(backreferences.getParents(nodesToDelete), Way.class)) {
             ArrayList<Node> newNodes = new ArrayList<Node>(w.getNodesCount());
             for (Node n: w.getNodes()) {
-                if (! nodesToDelete.contains(n)) {
+                if (! nodesToDelete.contains(n) && n != targetNode) {
                     newNodes.add(n);
+                } else if (newNodes.isEmpty()) {
+                    newNodes.add(targetNode);
+                } else if (newNodes.get(newNodes.size()-1) != targetNode) {
+                    // make sure we collapse a sequence of deleted nodes
+                    // to exactly one occurrence of the merged target node
+                    //
+                    newNodes.add(targetNode);
                 } else {
-                    newNodes.add(targetNode);
+                    // drop the node
                 }
             }
@@ -236 +202 @@
             }
         }
+        return cmds;
+    }
+
+    /**
+     * Merges the nodes in <code>node</code> onto one of the nodes. Uses the dataset
+     * managed by <code>layer</code> as reference. <code>backreferences</code> is precomputed
+     * collection of all parent/child references in the dataset.
+     *
+     * @param layer layer the reference data layer. Must not be null.
+     * @param backreferences if null, backreferneces are first computed from layer.data; otherwise
+     *    backreferences.getSource() == layer.data must hold
+     * @param nodes the collection of nodes. Ignored if null.
+     * @param targetNode the target node the collection of nodes is merged to. Must not be null.
+     * @throw IllegalArgumentException thrown if layer is null
+     * @throw IllegalArgumentException thrown if  backreferences.getSource() != layer.data
+     */
+    public static Command mergeNodes(OsmDataLayer layer, BackreferencedDataSet backreferences, Collection<Node> nodes, Node targetNode) {
+        if (layer == null)
+            throw new IllegalArgumentException(tr("parameter ''{0}'' must not be null", "nodes"));
+        if (targetNode == null)
+            throw new IllegalArgumentException(tr("parameter ''{0}'' must not be null", "targetNode"));
+        if (nodes == null)
+            return null;
+        if (backreferences == null) {
+            backreferences = new BackreferencedDataSet(layer.data);
+            backreferences.build();
+        }
+
+        Set<RelationToChildReference> relationToNodeReferences = backreferences.getRelationToChildReferences(nodes);
+
+        // build the tag collection
+        //
+        TagCollection nodeTags = TagCollection.unionOfAllPrimitives(nodes);
+        completeTagCollectionWithMissingTags(nodeTags, nodes);
+        TagCollection nodeTagsToEdit = new TagCollection(nodeTags);
+        completeTagCollectionForEditing(nodeTagsToEdit);
+
+        // launch a conflict resolution dialog, if necessary
+        //
+        CombinePrimitiveResolverDialog dialog = CombinePrimitiveResolverDialog.getInstance();
+        dialog.getTagConflictResolverModel().populate(nodeTagsToEdit);
+        dialog.getRelationMemberConflictResolverModel().populate(relationToNodeReferences);
+        dialog.setTargetPrimitive(targetNode);
+        dialog.prepareDefaultDecisions();
+        if (! nodeTags.isApplicableToPrimitive() || relationToNodeReferences.size() > 1) {
+            dialog.setVisible(true);
+            if (dialog.isCancelled())
+                return null;
+        }
+        LinkedList<Command> cmds = new LinkedList<Command>();
+
+        // the nodes we will have to delete
+        //
+        Collection<OsmPrimitive> nodesToDelete = new HashSet<OsmPrimitive>(nodes);
+        nodesToDelete.remove(targetNode);
+
+        // change the ways referring to at least one of the merge nodes
+        //
+        Collection<Way> waysToDelete= new HashSet<Way>();
+        List<Command> wayFixCommands = fixParentWays(
+                backreferences,
+                nodesToDelete,
+                targetNode
+        );
+        if (wayFixCommands == null)
+            return null;
+        else {
+            cmds.addAll(wayFixCommands);
+        }
 
         // build the commands