Changeset 6564 in josm

Timestamp:

2013-12-29T20:31:47+01:00 (10 years ago)

Author:

simon04

Message:

fix 9492 - Tools>"Create Multipolygon": updates multipolygon if a multipolygon relation is selected

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/CreateMultipolygonAction.java (modified) (9 diffs)
• src/org/openstreetmap/josm/data/osm/Relation.java (modified) (5 diffs)
• test/data/create_multipolygon.osm (added)
• test/unit/org/openstreetmap/josm/actions/CreateMultipolygonActionTest.groovy (added)

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

@@ -10 +10 @@
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -33 +34 @@
 import org.openstreetmap.josm.gui.Notification;
 import org.openstreetmap.josm.gui.dialogs.relation.RelationEditor;
+import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.Shortcut;
+import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -56 +59 @@
      */
     public CreateMultipolygonAction() {
-        super(tr("Create multipolygon"), "multipoly_create", tr("Create multipolygon."),
+        super(tr("Create multipolygon"), "multipoly_create", tr("Create multipolygon"),
             Shortcut.registerShortcut("tools:multipoly", tr("Tool: {0}", tr("Create multipolygon")),
             KeyEvent.VK_A, Shortcut.ALT_CTRL), true);
@@ -76 +79 @@
         }
 
-        Collection<Way> selectedWays = Main.main.getCurrentDataSet().getSelectedWays();
+        final Collection<Way> selectedWays = Main.main.getCurrentDataSet().getSelectedWays();
+        final Collection<Relation> selectedRelations = Main.main.getCurrentDataSet().getSelectedRelations();
 
         if (selectedWays.size() < 1) {
@@ -89 +93 @@
         }
 
-        MultipolygonCreate polygon = this.analyzeWays(selectedWays);
-
-        if (polygon == null)
-            return;                   //could not make multipolygon.
-
-        final Relation relation = this.createRelation(polygon);
-
-        if (Main.pref.getBoolean("multipoly.show-relation-editor", false)) {
-            //Open relation edit window, if set up in preferences
-            RelationEditor editor = RelationEditor.getEditor(Main.main.getEditLayer(), relation, null);
-
-            editor.setModal(true);
-            editor.setVisible(true);
-
-            //TODO: cannot get the resulting relation from RelationEditor :(.
-            /*
-            if (relationCountBefore < relationCountAfter) {
-                //relation saved, clean up the tags
-                List<Command> list = this.removeTagsFromInnerWays(relation);
-                if (list.size() > 0)
-                {
-                    Main.main.undoRedo.add(new SequenceCommand(tr("Remove tags from multipolygon inner ways"), list));
-                }
-            }
-             */
-
+        final Pair<SequenceCommand, Relation> commandAndRelation = createMultipolygonCommand(selectedWays, selectedRelations);
+        final Command command = commandAndRelation.a;
+        final Relation relation = commandAndRelation.b;
+        if (command == null) {
+            return;
+        }
+        Main.main.undoRedo.add(command);
+
+        // Use 'SwingUtilities.invokeLater' to make sure the relationListDialog
+        // knows about the new relation before we try to select it.
+        // (Yes, we are already in event dispatch thread. But DatasetEventManager
+        // uses 'SwingUtilities.invokeLater' to fire events so we have to do
+        // the same.)
+        SwingUtilities.invokeLater(new Runnable() {
+            @Override
+            public void run() {
+                Main.map.relationListDialog.selectRelation(relation);
+                if (Main.pref.getBoolean("multipoly.show-relation-editor", false)) {
+                    //Open relation edit window, if set up in preferences
+                    RelationEditor editor = RelationEditor.getEditor(Main.main.getEditLayer(), relation, null);
+
+                    editor.setModal(true);
+                    editor.setVisible(true);
+                }
+            }
+        });
+
+    }
+
+    private static Relation getSelectedMultipolygonRelation(Collection<Relation> selectedRelations) {
+        return  selectedRelations.size() == 1 && "multipolygon".equals(selectedRelations.iterator().next().get("type"))
+                ? selectedRelations.iterator().next()
+                : null;
+    }
+
+    /**
+     * Returns a {@link Pair} of the old multipolygon {@link Relation} (or null) and the newly created/modified multipolygon {@link Relation}.
+     */
+    public static Pair<Relation, Relation> createMultipolygonRelation(Collection<Way> selectedWays, Collection<Relation> selectedRelations) {
+
+        final Relation selectedMultipolygonRelation = getSelectedMultipolygonRelation(selectedRelations);
+        if (selectedMultipolygonRelation != null) {
+            // add ways of existing relation to include them in polygon analysis
+            selectedWays = new HashSet<Way>(selectedWays);
+            selectedWays.addAll(Utils.filteredCollection(selectedMultipolygonRelation.getMemberPrimitives(), Way.class));
+        }
+
+        final MultipolygonCreate polygon = analyzeWays(selectedWays);
+        if (polygon == null) {
+            return null; //could not make multipolygon.
+        }
+
+        if (selectedMultipolygonRelation != null) {
+            return Pair.create(selectedMultipolygonRelation, createRelation(polygon, new Relation(selectedMultipolygonRelation)));
         } else {
-            //Just add the relation
-            List<Command> list = this.removeTagsFromWaysIfNeeded(relation);
+            return Pair.create(null, createRelation(polygon, new Relation()));
+        }
+    }
+
+    /**
+     * Returns a pair of a multipolygon creating/modifying {@link Command} as well as the multipolygon {@link Relation}.
+     */
+    public static Pair<SequenceCommand, Relation> createMultipolygonCommand(Collection<Way> selectedWays, Collection<Relation> selectedRelations) {
+
+        final Pair<Relation, Relation> rr = createMultipolygonRelation(selectedWays, selectedRelations);
+        if (rr == null) {
+            return null;
+        }
+        final Relation existingRelation = rr.a;
+        final Relation relation = rr.b;
+
+        final List<Command> list = removeTagsFromWaysIfNeeded(relation);
+        final String commandName;
+        if (existingRelation == null) {
             list.add(new AddCommand(relation));
-            Main.main.undoRedo.add(new SequenceCommand(tr("Create multipolygon"), list));
-            // Use 'SwingUtilities.invokeLater' to make sure the relationListDialog
-            // knows about the new relation before we try to select it.
-            // (Yes, we are already in event dispatch thread. But DatasetEventManager
-            // uses 'SwingUtilities.invokeLater' to fire events so we have to do
-            // the same.)
-            SwingUtilities.invokeLater(new Runnable() {
-                @Override
-                public void run() {
-                    Main.map.relationListDialog.selectRelation(relation);
-                }
-            });
-        }
-
-
+            commandName = tr("Create multipolygon");
+        } else {
+            list.add(new ChangeCommand(existingRelation, relation));
+            commandName = tr("Update multipolygon");
+        }
+        return Pair.create(new SequenceCommand(commandName, list), relation);
     }
 
@@ -152 +192 @@
     @Override protected void updateEnabledState(Collection < ? extends OsmPrimitive > selection) {
         setEnabled(selection != null && !selection.isEmpty());
+        putValue(NAME, getSelectedMultipolygonRelation(getCurrentDataSet().getSelectedRelations()) != null
+                ? tr("Update multipolygon")
+                : tr("Create multipolygon")
+        );
     }
 
@@ -159 +203 @@
      * @return <code>null</code>, if there was a problem with the ways.
      */
-    private MultipolygonCreate analyzeWays(Collection < Way > selectedWays) {
+    private static MultipolygonCreate analyzeWays(Collection < Way > selectedWays) {
 
         MultipolygonCreate pol = new MultipolygonCreate();
@@ -179 +223 @@
      * @return multipolygon relation
      */
-    private Relation createRelation(MultipolygonCreate pol) {
+    private static Relation createRelation(MultipolygonCreate pol, final Relation rel) {
         // Create new relation
-        Relation rel = new Relation();
         rel.put("type", "multipolygon");
         // Add ways to it
         for (JoinedPolygon jway:pol.outerWays) {
-            for (Way way:jway.ways) {
-                rel.addMember(new RelationMember("outer", way));
-            }
+            addMembers(jway, rel, "outer");
         }
 
         for (JoinedPolygon jway:pol.innerWays) {
-            for (Way way:jway.ways) {
-                rel.addMember(new RelationMember("inner", way));
-            }
+            addMembers(jway, rel, "inner");
         }
         return rel;
+    }
+
+    private static void addMembers(JoinedPolygon polygon, Relation rel, String role) {
+        final int count = rel.getMembersCount();
+        final HashSet<Way> ways = new HashSet<Way>(polygon.ways);
+        for (int i = 0; i < count; i++) {
+            final RelationMember m = rel.getMember(i);
+            if (ways.contains(m.getMember()) && !role.equals(m.getRole())) {
+                rel.setMember(i, new RelationMember(role, m.getMember()));
+            }
+        }
+        ways.removeAll(rel.getMemberPrimitives());
+        for (final Way way : ways) {
+            rel.addMember(new RelationMember(role, way));
+        }
     }
 
@@ -206 +260 @@
      * @return a list of commands to execute
      */
-    private List<Command> removeTagsFromWaysIfNeeded( Relation relation ) {
+    private static List<Command> removeTagsFromWaysIfNeeded( Relation relation ) {
         Map<String, String> values = new HashMap<String, String>();
 

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

@@ -5 +5 @@
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
@@ -14 +15 @@
 import org.openstreetmap.josm.data.osm.visitor.Visitor;
 import org.openstreetmap.josm.tools.CopyList;
+import org.openstreetmap.josm.tools.Predicate;
 import org.openstreetmap.josm.tools.Utils;
 
@@ -319 +321 @@
      */
     public void removeMembersFor(OsmPrimitive primitive) {
-        if (primitive == null)
-            return;
-
-        boolean locked = writeLock();
-        try {
-            List<RelationMember> todelete = new ArrayList<RelationMember>();
-            for (RelationMember member: members) {
-                if (member.getMember() == primitive) {
-                    todelete.add(member);
-                }
-            }
-            List<RelationMember> members = getMembers();
-            members.removeAll(todelete);
-            setMembers(members);
-        } finally {
-            writeUnlock(locked);
-        }
+        removeMembersFor(Collections.singleton(primitive));
     }
 
@@ -356 +342 @@
 
     /**
+     * Obtains all members with member.member == primitive
+     * @param primitives the primitives to check for
+     */
+    public Collection<RelationMember> getMembersFor(final Collection<? extends OsmPrimitive> primitives) {
+        return Utils.filter(getMembers(), new Predicate<RelationMember>() {
+            @Override
+            public boolean evaluate(RelationMember member) {
+                return primitives.contains(member.getMember());
+            }
+        });
+    }
+
+    /**
      * removes all members with member.member == primitive
      *
@@ -367 +366 @@
         boolean locked = writeLock();
         try {
-            List<RelationMember> todelete = new ArrayList<RelationMember>();
-            for (RelationMember member: members) {
-                if (primitives.contains(member.getMember())) {
-                    todelete.add(member);
-                }
-            }
             List<RelationMember> members = getMembers();
-            members.removeAll(todelete);
+            members.removeAll(getMembersFor(primitives));
             setMembers(members);
         } finally {