Changeset 30872 in osm

Timestamp:

2014-12-21T13:55:57+01:00 (10 years ago)

Author:

donvip

Message:

[josm_terracer] fix #josm6552 - terracer unnecessarily deletes objects in favour of new ones

Location:

applications/editors/josm/plugins/terracer/src/terracer

Files:

• HouseNumberInputDialog.java (modified) (8 diffs)
• HouseNumberInputHandler.java (modified) (5 diffs)
• TerracerAction.java (modified) (15 diffs)

applications/editors/josm/plugins/terracer/src/terracer/HouseNumberInputDialog.java

@@ -55 +55 @@
     final static String DEFAULT_SEGMENTS = "plugins.terracer.segments";
     final static String HANDLE_RELATION = "plugins.terracer.handle_relation";
-    final static String DELETE_OUTLINE = "plugins.terracer.delete_outline";
+    final static String KEEP_OUTLINE = "plugins.terracer.keep_outline";
     final static String INTERPOLATION = "plugins.terracer.interpolation";
 
@@ -83 +83 @@
     Choice interpolation;
     JCheckBox handleRelationCheckBox;
-    JCheckBox deleteOutlineCheckBox;
+    JCheckBox keepOutlineCheckBox;
 
     HouseNumberInputHandler inputHandler;
@@ -182 +182 @@
 
             handleRelationCheckBox = new JCheckBox(txt, relationExists ? Main.pref.getBoolean(HANDLE_RELATION, true) : false);
-            deleteOutlineCheckBox = new JCheckBox(tr("delete outline way"), Main.pref.getBoolean(DELETE_OUTLINE, true));
+            keepOutlineCheckBox = new JCheckBox(tr("keep outline way"), Main.pref.getBoolean(KEEP_OUTLINE, false));
 
             inputPanel = new JPanel();
@@ -209 +209 @@
             inputPanel.add(getBuilding(), GBC.eol().insets(5,3,0,0));
             inputPanel.add(handleRelationCheckBox, GBC.eol().insets(3,3,0,0));
-            inputPanel.add(deleteOutlineCheckBox, GBC.eol().insets(3,3,0,0));
-            
+            inputPanel.add(keepOutlineCheckBox, GBC.eol().insets(3,3,0,0));
+
             if (numbers.isVisible()) {
                 loLabel.setVisible(false);
@@ -261 +261 @@
         return hi;
     }
-    
+
     /**
      * This method initializes numbers
@@ -270 +270 @@
         if (numbers == null) {
             numbers = new JTextField();
-            
+
             Iterator<Node> it = housenumbers.iterator();
             StringBuilder s = new StringBuilder(256);
@@ -282 +282 @@
                 numbers.setVisible(false);
             }
-            
+
             numbers.setText(s.toString());
             numbers.setEditable(false);
@@ -306 +306 @@
         return streetComboBox;
     }
-    
+
     /**
      * This method initializes building

applications/editors/josm/plugins/terracer/src/terracer/HouseNumberInputHandler.java

@@ -304 +304 @@
                             streetName(),
                             doHandleRelation(),
-                            doDeleteOutline(), buildingType());
+                            doKeepOutline(), buildingType());
                     } catch (UserCancelException ex) {
                         // Ignore
@@ -380 +380 @@
         return getItemText(dialog.streetComboBox);
     }
-    
+
     /**
      * Gets the building type.
@@ -389 +389 @@
         return getItemText(dialog.buildingComboBox);
     }
-    
+
     private static String getItemText(AutoCompletingComboBox box) {
         Object selected = box.getSelectedItem();
@@ -430 +430 @@
 
     /**
-     * Whether the user likes to delete the outline way.
-     */
-    public boolean doDeleteOutline() {
-        return dialog.deleteOutlineCheckBox.isSelected();
+     * Whether the user likes to keep the outline way.
+     */
+    public boolean doKeepOutline() {
+        return dialog.keepOutlineCheckBox.isSelected();
     }
 
@@ -454 +454 @@
     public void saveValues() {
         Main.pref.put(HouseNumberInputDialog.HANDLE_RELATION, doHandleRelation());
-        Main.pref.put(HouseNumberInputDialog.DELETE_OUTLINE, doDeleteOutline());
+        Main.pref.put(HouseNumberInputDialog.KEEP_OUTLINE, doKeepOutline());
         Main.pref.put(HouseNumberInputDialog.INTERPOLATION, stepSize().toString());
     }

applications/editors/josm/plugins/terracer/src/terracer/TerracerAction.java

@@ -69 +69 @@
 
     Collection<Command> commands;
-    
+
     private Collection<OsmPrimitive> primitives;
     private TagCollection tagsInConflict;
@@ -96 +96 @@
         return result;
     }
-    
+
     private static final class InvalidUserInputException extends Exception {
         InvalidUserInputException(String message) {
@@ -182 +182 @@
             if (outline == null || !outline.isClosed() || outline.getNodesCount() < 5)
                 throw new InvalidUserInputException("wrong or missing outline");
-        
+
         } catch (InvalidUserInputException ex) {
             Main.warn("Terracer: "+ex.getMessage());
@@ -202 +202 @@
             candidates.add(street);
         }
-        
+
         Set<Relation> associatedStreets = findAssociatedStreets(candidates);
-        
+
         if (!associatedStreets.isEmpty()) {
             associatedStreet = associatedStreets.iterator().next();
@@ -212 +212 @@
             }
         }
-        
+
         if (streetname == null && associatedStreet != null && associatedStreet.hasKey("name")) {
             streetname = associatedStreet.get("name");
@@ -297 +297 @@
      * @param associatedStreet
      * @param segments The number of segments to generate
-     * @param From Starting housenumber
-     * @param To Ending housenumber
+     * @param start Starting housenumber
+     * @param end Ending housenumber
      * @param step The step width to use
      * @param housenumbers List of housenumbers to use. From and To are ignored
@@ -306 +306 @@
      * @param handleRelations If the user likes to add a relation or extend an
      *        existing relation
-     * @param deleteOutline If the outline way should be deleted when done
+     * @param keepOutline If the outline way should be kept
      * @param buildingValue The value for {@code building} key to add
-     * @throws UserCancelException 
-     */
-    public void terraceBuilding(final Way outline,
-                Node init,
-                Way street,
-                Relation associatedStreet,
-                Integer segments,
-                String From,
-                String To,
-                int step,
-                ArrayList<Node> housenumbers,
-                String streetName,
-                boolean handleRelations,
-                boolean deleteOutline, String buildingValue) throws UserCancelException {
+     * @throws UserCancelException
+     */
+    public void terraceBuilding(final Way outline, Node init, Way street, Relation associatedStreet, Integer segments,
+                String start, String end, int step, List<Node> housenumbers, String streetName, boolean handleRelations,
+                boolean keepOutline, String buildingValue) throws UserCancelException {
         final int nb;
         Integer to = null, from = null;
         if (housenumbers == null || housenumbers.isEmpty()) {
-            to = getNumber(To);
-            from = getNumber(From);
+            to = getNumber(end);
+            from = getNumber(start);
             if (to != null && from != null) {
                 nb = 1 + (to.intValue() - from.intValue()) / step;
@@ -336 +327 @@
                         "Could not determine segments from parameters, this is a bug. "
                                 + "Parameters were: segments " + segments
-                                + " from " + from + " to " + to + " step "
-                                + step);
+                                + " from " + from + " to " + to + " step " + step);
             }
         } else {
@@ -346 +336 @@
         Pair<Way, Way> interp = findFrontAndBack(outline);
 
-        boolean swap = false;
-        if (init != null) {
-            if (interp.a.lastNode().equals(init) || interp.b.lastNode().equals(init)) {
-                swap = true;
-            }
-        }
+        final boolean swap = init != null && (interp.a.lastNode().equals(init) || interp.b.lastNode().equals(init));
 
         final double frontLength = wayLength(interp.a);
@@ -357 +342 @@
 
         // new nodes array to hold all intermediate nodes
-        // This set will contain at least 4 existing nodes from the original outline (those, which coordinates match coordinates of outline nodes)
-        Node[][] new_nodes = new Node[2][nb + 1];
+        // This set will contain at least 4 existing nodes from the original outline
+        // (those, which coordinates match coordinates of outline nodes)
+        Node[][] newNodes = new Node[2][nb + 1];
         // This list will contain nodes of the outline that are used in new lines.
         // These nodes will not be deleted with the outline (if deleting was prompted).
-        ArrayList<Node> reused_nodes = new ArrayList<>();
+        List<Node> reusedNodes = new ArrayList<>();
 
         this.commands = new LinkedList<>();
@@ -367 +353 @@
 
         if (nb > 1) {
+            // add required new nodes and build list of nodes to reuse
             for (int i = 0; i <= nb; ++i) {
-                int i_dir = swap ? nb - i : i;
-                new_nodes[0][i] = interpolateAlong(interp.a, frontLength * i_dir / nb);
-                new_nodes[1][i] = interpolateAlong(interp.b, backLength * i_dir / nb);
-                if (!outline.containsNode(new_nodes[0][i]))
-                    this.commands.add(new AddCommand(new_nodes[0][i]));
+                int iDir = swap ? nb - i : i;
+                newNodes[0][i] = interpolateAlong(interp.a, frontLength * iDir / nb);
+                newNodes[1][i] = interpolateAlong(interp.b, backLength * iDir / nb);
+                if (!outline.containsNode(newNodes[0][i]))
+                    this.commands.add(new AddCommand(newNodes[0][i]));
                 else
-                    reused_nodes.add(new_nodes[0][i]);
-                if (!outline.containsNode(new_nodes[1][i]))
-                    this.commands.add(new AddCommand(new_nodes[1][i]));
+                    reusedNodes.add(newNodes[0][i]);
+                if (!outline.containsNode(newNodes[1][i]))
+                    this.commands.add(new AddCommand(newNodes[1][i]));
                 else
-                    reused_nodes.add(new_nodes[1][i]);
+                    reusedNodes.add(newNodes[1][i]);
             }
 
             // assemble new quadrilateral, closed ways
             for (int i = 0; i < nb; ++i) {
-                Way terr = new Way();
-                terr.addNode(new_nodes[0][i]);
-                terr.addNode(new_nodes[0][i + 1]);
-                terr.addNode(new_nodes[1][i + 1]);
-                terr.addNode(new_nodes[1][i]);
-                terr.addNode(new_nodes[0][i]);
-
-                // add the tags of the outline to each building (e.g. source=*)
-                TagCollection.from(outline).applyTo(terr);
-                ways.add(addressBuilding(terr, street, streetName, associatedStreet, housenumbers, i, 
+                final Way terr;
+                if (i > 0 || keepOutline) {
+                    terr = new Way();
+                    // add the tags of the outline to each building (e.g. source=*)
+                    TagCollection.from(outline).applyTo(terr);
+                } else {
+                    terr = new Way(outline);
+                    terr.setNodes(null);
+                }
+
+                terr.addNode(newNodes[0][i]);
+                terr.addNode(newNodes[0][i + 1]);
+                terr.addNode(newNodes[1][i + 1]);
+                terr.addNode(newNodes[1][i]);
+                terr.addNode(newNodes[0][i]);
+
+                ways.add(addressBuilding(terr, street, streetName, associatedStreet, housenumbers, i,
                         from != null ? Integer.toString(from + i * step) : null, buildingValue));
-                
-                this.commands.add(new AddCommand(terr));
-            }
-
-            if (deleteOutline) {
+
+                if (i > 0 || keepOutline) {
+                    this.commands.add(new AddCommand(terr));
+                } else {
+                    this.commands.add(new ChangeCommand(outline, terr));
+                }
+            }
+
+            if (!keepOutline) {
                 // Delete outline nodes having no tags and referrers but the outline itself
                 List<Node> nodes = outline.getNodes();
                 ArrayList<Node> nodesToDelete = new ArrayList<>();
                 for (Node n : nodes)
-                    if (!n.hasKeys() && n.getReferrers().size() == 1 && !reused_nodes.contains(n))
+                    if (!n.hasKeys() && n.getReferrers().size() == 1 && !reusedNodes.contains(n))
                         nodesToDelete.add(n);
-                if (nodesToDelete.size() > 0)
+                if (!nodesToDelete.isEmpty())
                     this.commands.add(DeleteCommand.delete(Main.main.getEditLayer(), nodesToDelete));
-                // Delete the way itself without nodes
-                this.commands.add(DeleteCommand.delete(Main.main.getEditLayer(), Collections.singleton(outline), false, true));
-
             }
         } else {
             // Single building, just add the address details
-            ways.add(addressBuilding(outline, street, streetName, associatedStreet, housenumbers, 0, From, buildingValue));
-        }
-
-        // Remove the address nodes since their tags have been incorporated into
-        // the terraces.
+            ways.add(addressBuilding(outline, street, streetName, associatedStreet, housenumbers, 0, start, buildingValue));
+        }
+
+        // Remove the address nodes since their tags have been incorporated into the terraces.
         // Or should removing them also be an option?
         if (!housenumbers.isEmpty()) {
@@ -426 +420 @@
         if (handleRelations) { // create a new relation or merge with existing
             if (associatedStreet == null) {  // create a new relation
-                associatedStreet = new Relation();
-                associatedStreet.put("type", "associatedStreet");
-                if (street != null) { // a street was part of the selection
-                    associatedStreet.put("name", street.get("name"));
-                    associatedStreet.addMember(new RelationMember("street", street));
-                } else {
-                    associatedStreet.put("name", streetName);
-                }
-                for (Way w : ways) {
-                    associatedStreet.addMember(new RelationMember("house", w));
-                }
-                this.commands.add(new AddCommand(associatedStreet));
+                addNewAssociatedStreetRelation(street, streetName, ways);
             } else { // relation exists already - add new members
-                Relation newAssociatedStreet = new Relation(associatedStreet);
-                // remove housenumbers as they have been deleted
-                newAssociatedStreet.removeMembersFor(housenumbers);
-                for (Way w : ways) {
-                    newAssociatedStreet.addMember(new RelationMember("house", w));
-                }
-                if (deleteOutline) {
-                    newAssociatedStreet.removeMembersFor(outline);
-                }
-                this.commands.add(new ChangeCommand(associatedStreet, newAssociatedStreet));
-            }
-        }
-
-        Main.main.undoRedo.add(new SequenceCommand(tr("Terrace"), commands) {
+                updateAssociatedStreetRelation(associatedStreet, housenumbers, ways);
+            }
+        }
+
+        Main.main.undoRedo.add(createTerracingCommand(outline));
+        if (nb <= 1 && street != null) {
+            // Select the way (for quick selection of a new house (with the same way))
+            Main.main.getCurrentDataSet().setSelected(street);
+        } else {
+            // Select the new building outlines (for quick reversing)
+            Main.main.getCurrentDataSet().setSelected(ways);
+        }
+    }
+
+    private void updateAssociatedStreetRelation(Relation associatedStreet, List<Node> housenumbers, Collection<Way> ways) {
+        Relation newAssociatedStreet = new Relation(associatedStreet);
+        // remove housenumbers as they have been deleted
+        newAssociatedStreet.removeMembersFor(housenumbers);
+        for (Way w : ways) {
+            newAssociatedStreet.addMember(new RelationMember("house", w));
+        }
+        /*if (!keepOutline) {
+            newAssociatedStreet.removeMembersFor(outline);
+        }*/
+        this.commands.add(new ChangeCommand(associatedStreet, newAssociatedStreet));
+    }
+
+    private void addNewAssociatedStreetRelation(Way street, String streetName, Collection<Way> ways) {
+        Relation associatedStreet = new Relation();
+        associatedStreet.put("type", "associatedStreet");
+        if (street != null) { // a street was part of the selection
+            associatedStreet.put("name", street.get("name"));
+            associatedStreet.addMember(new RelationMember("street", street));
+        } else {
+            associatedStreet.put("name", streetName);
+        }
+        for (Way w : ways) {
+            associatedStreet.addMember(new RelationMember("house", w));
+        }
+        this.commands.add(new AddCommand(associatedStreet));
+    }
+
+    private Command createTerracingCommand(final Way outline) {
+        return new SequenceCommand(tr("Terrace"), commands) {
             @Override
             public boolean executeCommand() {
@@ -481 +494 @@
                 return result;
             }
-        });
-        if (nb <= 1 && street != null) {
-            // Select the way (for quick selection of a new house (with the same way))
-            Main.main.getCurrentDataSet().setSelected(street);
-        } else {
-            // Select the new building outlines (for quick reversing)
-            Main.main.getCurrentDataSet().setSelected(ways);
-        }
-    }
-    
+        };
+    }
+
     /**
      * Adds address details to a single building
@@ -500 +506 @@
      * @param buildingValue The value for {@code building} key to add
      * @return {@code outline}
-     * @throws UserCancelException 
-     */
-    private Way addressBuilding(Way outline, Way street, String streetName, Relation associatedStreet, ArrayList<Node> housenumbers, int i, String defaultNumber, String buildingValue) throws UserCancelException {
+     * @throws UserCancelException
+     */
+    private Way addressBuilding(Way outline, Way street, String streetName, Relation associatedStreet,
+            List<Node> housenumbers, int i, String defaultNumber, String buildingValue) throws UserCancelException {
         Node houseNum = (housenumbers != null && i >= 0 && i < housenumbers.size()) ? housenumbers.get(i) : null;
         boolean buildingAdded = false;
@@ -508 +515 @@
         if (houseNum != null) {
             primitives = Arrays.asList(new OsmPrimitive[]{houseNum, outline});
-            
+
             TagCollection tagsToCopy = TagCollection.unionOfAllPrimitives(primitives).getTagsFor(houseNum.keySet());
             tagsInConflict = tagsToCopy.getTagsFor(tagsToCopy.getKeysWithMultipleValues());
             tagsToCopy = tagsToCopy.minus(tagsInConflict).minus(TagCollection.from(outline));
-            
+
             for (Tag tag : tagsToCopy) {
                 this.commands.add(new ChangePropertyCommand(outline, tag.getKey(), tag.getValue()));
             }
-            
+
             buildingAdded = houseNum.hasKey("building");
             numberAdded = houseNum.hasKey("addr:housenumber");