Changeset 11118 in josm

Timestamp:

2016-10-11T20:20:06+02:00 (8 years ago)

Author:

simon04

Message:

fix #13094 - SimplifyWayAction for first node of closed way (patch by Tyndare, modified)

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/SimplifyWayAction.java (modified) (5 diffs)
• test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java (modified) (4 diffs)

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

@@ -9 +9 @@
 import java.awt.event.KeyEvent;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -134 +135 @@
      */
     protected boolean isRequiredNode(Way way, Node node) {
-        boolean isRequired = Collections.frequency(way.getNodes(), node) > 1;
+        int frequency = Collections.frequency(way.getNodes(), node);
+        if ((way.getNode(0) == node) && (way.getNode(way.getNodesCount()-1) == node)) {
+            frequency = frequency - 1; // closed way closing node counted only once
+        }
+        boolean isRequired = frequency > 1;
         if (!isRequired) {
             List<OsmPrimitive> parents = new LinkedList<>();
@@ -187 +192 @@
             lower = i;
             i++;
+        }
+
+        if ((newNodes.size() > 3) && (newNodes.get(0) == newNodes.get(newNodes.size() - 1))) {
+            // Closed way, check if the first node could also be simplified ...
+            if (!isRequiredNode(w, newNodes.get(0))) {
+                final List<Node> l1 = Arrays.asList(newNodes.get(newNodes.size() - 2), newNodes.get(0), newNodes.get(1));
+                final List<Node> l2 = new ArrayList<>(3);
+                buildSimplifiedNodeList(l1, 0, 2, threshold, l2);
+                if (!l2.contains(newNodes.get(0))) {
+                    newNodes.remove(0);
+                    newNodes.set(newNodes.size() - 1, newNodes.get(0)); // close the way
+                }
+            }
         }
 
@@ -253 +271 @@
      * http://williams.best.vwh.net/avform.htm
      */
-    public static double dist(double lat1, double lon1, double lat2, double lon2) {
+    private static double dist(double lat1, double lon1, double lat2, double lon2) {
         return 2 * Math.asin(Math.sqrt(Math.pow(Math.sin((lat1 - lat2) / 2), 2) + Math.cos(lat1) * Math.cos(lat2)
                 * Math.pow(Math.sin((lon1 - lon2) / 2), 2)));
     }
 
-    public static double course(double lat1, double lon1, double lat2, double lon2) {
+    private static double course(double lat1, double lon1, double lat2, double lon2) {
         return Math.atan2(Math.sin(lon1 - lon2) * Math.cos(lat2), Math.cos(lat1) * Math.sin(lat2) - Math.sin(lat1)
                 * Math.cos(lat2) * Math.cos(lon1 - lon2))
@@ -264 +282 @@
     }
 
-    public static double xtd(double lat1, double lon1, double lat2, double lon2, double lat3, double lon3) {
+    private static double xtd(double lat1, double lon1, double lat2, double lon2, double lat3, double lon3) {
         double distAD = dist(lat1, lon1, lat3, lon3);
         double crsAD = course(lat1, lon1, lat3, lon3);

trunk/test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java

@@ -3 +3 @@
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.stream.Stream;
 
 import org.junit.BeforeClass;
@@ -9 +14 @@
 import org.openstreetmap.josm.JOSMFixture;
 import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.command.DeleteCommand;
+import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.DataSet;
@@ -14 +21 @@
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -97 +105 @@
         }
     }
+
+    /**
+     * Tests that also the first node may be simplified, see #13094.
+     */
+    @Test
+    public void testSimplifyFirstNode() {
+        final DataSet ds = new DataSet();
+        final Node n1 = new Node(new LatLon(47.26269614984, 11.34044231149));
+        final Node n2 = new Node(new LatLon(47.26274590831, 11.34053120859));
+        final Node n3 = new Node(new LatLon(47.26276562382, 11.34034715039));
+        final Node n4 = new Node(new LatLon(47.26264639132, 11.34035341438));
+        final Way w = new Way();
+        Stream.of(n1, n2, n3, n4, w).forEach(ds::addPrimitive);
+        Stream.of(n1, n2, n3, n4, n1).forEach(w::addNode);
+        final SequenceCommand command = action.simplifyWay(w);
+        assertNotNull(command);
+        assertEquals(2, command.getChildren().size());
+        final Collection<DeleteCommand> deleteCommands = Utils.filteredCollection(command.getChildren(), DeleteCommand.class);
+        assertEquals(1, deleteCommands.size());
+        assertEquals(Collections.singleton(n1), deleteCommands.iterator().next().getParticipatingPrimitives());
+    }
 }