Ticket #5179: osm-join-areas.patch

src/org/openstreetmap/josm/actions/JoinAreasAction.java

@@ -6 +6 @@
 import static org.openstreetmap.josm.tools.I18n.trn;
 
 import java.awt.GridBagLayout;
-import java.awt.Polygon;
 import java.awt.event.ActionEvent;
 import java.awt.event.KeyEvent;
 import java.awt.geom.Area;
@@ -185 +184 @@
         if(!same) {
             int i = 0;
             if(checkForTagConflicts(a, b)) return true; // User aborted, so don't warn again
+
+            //join each area with itself, fixing self-crossings.
             if(joinAreas(a, a)) {
                 ++i;
             }
@@ -210 +211 @@
 
         Collection<Way> allWays = splitWaysOnNodes(a, b, nodes);
 
-        // Find all nodes and inner ways save them to a list
-        Collection<Node> allNodes = getNodesFromWays(allWays);
-        Collection<Way> innerWays = findInnerWays(allWays, allNodes);
+        // Find inner ways save them to a list
+        Collection<Way> outerWays = findOuterWays(allWays);
+        Collection<Way> innerWays = findInnerWays(allWays, outerWays);
 
         // Join outer ways
-        Way outerWay = joinOuterWays(allWays, innerWays);
+        Way outerWay = joinOuterWays(outerWays);
         if (outerWay == null)
             return true;
 
@@ -538 +539 @@
         return allNodes;
     }
 
+
     /**
-     * Finds all inner ways for a given list of Ways and Nodes from a multigon by constructing a polygon
-     * for each way, looking for inner nodes that are not part of this way. If a node is found, all ways
-     * containing this node are added to the list
+     * Gets all inner ways given all ways and outer ways.
+     * @param multigonWays
+     * @param outerWays
+     * @return list of inner ways.
+     */
+    private Collection<Way> findInnerWays(Collection<Way> multigonWays,Collection<Way> outerWays) {
+        ArrayList<Way> innerWays = new ArrayList<Way>();
+        for(Way way: multigonWays) {
+            if (!outerWays.contains(way)) {
+                innerWays.add(way);
+            }
+        }
+
+        return innerWays;
+    }
+
+
+    /**
+     * Finds all ways for a given list of Ways that form the outer hull.
+     * This works by starting with one node and traversing the multigon clockwise, always picking the leftmost path.
+     * Prerequisites - the ways must not intersect and have common end nodes where they meet.
      * @param Collection<Way> A list of (splitted) ways that form a multigon
-     * @param Collection<Node> A list of nodes that belong to the multigon
-     * @return Collection<Way> A list of ways that are positioned inside the outer borders of the multigon
+     * @return Collection<Way> A list of ways that form the outer boundary of the multigon.
      */
-    private Collection<Way> findInnerWays(Collection<Way> multigonWays, Collection<Node> multigonNodes) {
-        Collection<Way> innerWays = new ArrayList<Way>();
-        for(Way w: multigonWays) {
-            Polygon poly = new Polygon();
-            for(Node n: (w).getNodes()) {
-                poly.addPoint(latlonToXY(n.getCoor().lat()), latlonToXY(n.getCoor().lon()));
+    public static Collection<Way> findOuterWays(Collection<Way> multigonWays) {
+
+        //find the node with minimum lat - it's guaranteed to be outer. (What about the south pole?)
+        Way bestWay = null;
+        Node topNode = null;
+        int topIndex = 0;
+        double minLat = Double.POSITIVE_INFINITY;
+
+        for(Way way: multigonWays) {
+            for (int pos = 0; pos < way.getNodesCount(); pos ++){
+                Node node = way.getNode(pos);
+
+                if (node.getCoor().lat() < minLat){
+                    minLat = node.getCoor().lat();
+                    bestWay = way;
+                    topNode = node;
+                    topIndex = pos;
+                }
             }
+        }
 
-            for(Node n: multigonNodes) {
-                if(!(w).containsNode(n) && poly.contains(latlonToXY(n.getCoor().lat()), latlonToXY(n.getCoor().lon()))) {
-                    getWaysByNode(innerWays, multigonWays, n);
+        //get two final nodes from best way to mark as starting point and orientation.
+        Node headNode = null;
+        Node prevNode = null;
+
+        if (topNode.equals(bestWay.firstNode()) || topNode.equals(bestWay.lastNode()))
+        {
+            //node is in split point
+            headNode = topNode;
+            //make a fake node that is downwards from head node (smaller latitude). It will be a division point between paths.
+            prevNode = new Node(new LatLon(headNode.getCoor().lat() - 1000, headNode.getCoor().lon()));
+        }
+        else
+        {
+            //node is inside way - pick the clockwise going end.
+            Node prev = bestWay.getNode(topIndex - 1);
+            Node next = bestWay.getNode(topIndex + 1);
+
+            if (angleIsClockwise(prev, topNode, next)){
+                headNode = bestWay.lastNode();
+                prevNode = bestWay.getNode(bestWay.getNodesCount() - 2);
+            }
+            else
+            {
+                headNode = bestWay.firstNode();
+                prevNode = bestWay.getNode(1);
+            }
+        }
+
+        ArrayList<Way> outerWays = new ArrayList<Way>();
+
+        //iterate till full circle is reached
+        while (true){
+
+            bestWay = null;
+            Node bestWayNextNode = null;
+            boolean bestWayReverse = false;
+
+            for (Way way: multigonWays)
+            {
+                boolean wayReverse;
+                Node nextNode;
+
+                if (way.firstNode().equals(headNode)){
+                    nextNode = way.getNode(1);
+                    wayReverse = false;
                 }
+                else if (way.lastNode().equals(headNode))
+                {
+                    nextNode = way.getNode(way.getNodesCount() - 2);
+                    wayReverse = true;
+                }
+                else
+                {
+                    //this way not adjacent to headNode
+                    continue;
+                }
+
+                if (nextNode.equals(prevNode))
+                {
+                    //this is the path we came from - ignore it.
+                }
+                else if (bestWay == null || !isToTheRightSideOfLine(prevNode, headNode, bestWayNextNode, nextNode))
+                {
+                    //the new way is better
+                    bestWay = way;
+                    bestWayReverse = wayReverse;
+                    bestWayNextNode = nextNode;
+                }
             }
+
+            if (bestWay == null)
+                //this should not happen. Internal error here.
+                return null;
+            else if (outerWays.contains(bestWay)){
+                //full circle reached, terminate.
+                break;
+            }
+            else
+            {
+                //add to outer ways, repeat.
+                outerWays.add(bestWay);
+                headNode = bestWayReverse ? bestWay.firstNode() : bestWay.lastNode();
+                prevNode = bestWayReverse ? bestWay.getNode(2) : bestWay.getNode(bestWay.getNodesCount() - 2);
+            }
         }
 
-        return innerWays;
+        return outerWays;
     }
 
-    // Polygon only supports int coordinates, so convert them
-    private int latlonToXY(double val) {
-        return (int)Math.round(val*1000000);
+    /**
+     * Tests if given point is to the right side of path consisting of 3 points.
+     * @param lineP1 first point in path
+     * @param lineP2 second point in path
+     * @param lineP3 third point in path
+     * @param testPoint
+     * @return true if to the right side, false otherwise
+     */
+    public static boolean isToTheRightSideOfLine(Node lineP1, Node lineP2, Node lineP3, Node testPoint)
+    {
+        boolean pathBendToRight = angleIsClockwise(lineP1, lineP2, lineP3);
+        boolean rightOfSeg1 = angleIsClockwise(lineP1, lineP2, testPoint);
+        boolean rightOfSeg2 = angleIsClockwise(lineP2, lineP3, testPoint);
+
+        if (pathBendToRight)
+            return rightOfSeg1 && rightOfSeg2;
+        else
+            return !(!rightOfSeg1 && !rightOfSeg2);
     }
 
     /**
-     * Finds all ways that contain the given node.
-     * @param Collection<Way> A list to which matching ways will be added
-     * @param Collection<Way> A list of ways to check
-     * @param Node The node the ways should be checked against
+     * This method tests if secondNode is clockwise to first node.
+     * @param commonNode starting point for both vectors
+     * @param firstNode first vector end node
+     * @param secondNode second vector end node
+     * @return true if first vector is clockwise before second vector.
      */
-    private void getWaysByNode(Collection<Way> innerWays, Collection<Way> w, Node n) {
-        for(Way way : w) {
-            if(!(way).containsNode(n)) {
+    public static boolean angleIsClockwise(Node commonNode, Node firstNode, Node secondNode)
+    {
+        double dla1 = (firstNode.getCoor().lat() - commonNode.getCoor().lat());
+        double dla2 = (secondNode.getCoor().lat() - commonNode.getCoor().lat());
+        double dlo1 = (firstNode.getCoor().lon() - commonNode.getCoor().lon());
+        double dlo2 = (secondNode.getCoor().lon() - commonNode.getCoor().lon());
+
+        return dla1 * dlo2 - dlo1 * dla2 > 0;
+    }
+
+
+    /**
+     * Tests if point is inside a polygon. The polygon can be self-intersecting. In such case the contains function works in xor-like manner.
+     * @param polygonNodes list of nodes from polygon path.
+     * @param point the point to test
+     * @return true if the point is inside polygon.
+     * FIXME: this should probably be moved to tools..
+     */
+    public static boolean nodeInsidePolygon(ArrayList<Node> polygonNodes, Node point)
+    {
+        if (polygonNodes.size() < 3)
+            return false;
+
+        boolean inside = false;
+        Node p1, p2;
+
+        //iterate each side of the polygon, start with the last segment
+        Node oldPoint = polygonNodes.get(polygonNodes.size() - 1);
+
+        for(Node newPoint: polygonNodes)
+        {
+            //skip duplicate points
+            if (newPoint.equals(oldPoint)) {
                 continue;
             }
-            if(!innerWays.contains(way)) {
-                innerWays.add(way); // Will need this later for multigons
+
+            //order points so p1.lat <= p2.lat;
+            if (newPoint.getCoor().lat() > oldPoint.getCoor().lat())
+            {
+                p1 = oldPoint;
+                p2 = newPoint;
             }
+            else
+            {
+                p1 = newPoint;
+                p2 = oldPoint;
+            }
+
+            //test if the line is crossed and if so invert the inside flag.
+            if ((newPoint.getCoor().lat() < point.getCoor().lat()) == (point.getCoor().lat() <= oldPoint.getCoor().lat())
+                    && (point.getCoor().lon() - p1.getCoor().lon()) * (p2.getCoor().lat() - p1.getCoor().lat())
+                    < (p2.getCoor().lon() - p1.getCoor().lon()) * (point.getCoor().lat() - p1.getCoor().lat()))
+            {
+                inside = !inside;
+            }
+
+            oldPoint = newPoint;
         }
+
+        return inside;
     }
 
+
+
+
     /**
-     * Joins the two outer ways and deletes all short ways that can't be part of a multipolygon anyway
-     * @param Collection<OsmPrimitive> The list of all ways that belong to that multigon
-     * @param Collection<Way> The list of inner ways that belong to that multigon
+     * Joins the outer ways and deletes all short ways that can't be part of a multipolygon anyway.
+     * @param Collection<Way> The list of outer ways that belong to that multigon.
      * @return Way The newly created outer way
      */
-    private Way joinOuterWays(Collection<Way> multigonWays, Collection<Way> innerWays) {
+    private Way joinOuterWays(Collection<Way> outerWays) {
         ArrayList<Way> join = new ArrayList<Way>();
-        for(Way w: multigonWays) {
-            // Skip inner ways
-            if(innerWays.contains(w)) {
-                continue;
-            }
+        for(Way w: outerWays) {
 
             if(w.getNodesCount() <= 2) {
                 cmds.add(new DeleteCommand(w));
@@ -980 +1156 @@
     protected void updateEnabledState(Collection<? extends OsmPrimitive> selection) {
         setEnabled(selection != null && !selection.isEmpty());
     }
-}
+
+}
+ No newline at end of file

test/unit/actions/JoinAreasActionTest.java

@@ +1 @@
+// License: GPL. For details, see LICENSE file.
+package actions;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openstreetmap.josm.actions.JoinAreasAction;
+import org.openstreetmap.josm.data.coor.LatLon;
+import org.openstreetmap.josm.data.osm.Node;
+
+
+public class JoinAreasActionTest {
+
+    private Node makeNode(double lat, double lon)
+    {
+        Node node = new Node(new LatLon(lat, lon));
+        return node;
+    }
+
+    @Test
+    public void testAngleIsClockwise()
+    {
+        Assert.assertTrue(JoinAreasAction.angleIsClockwise(makeNode(0,0), makeNode(1,1), makeNode(0,1)));
+        Assert.assertTrue(JoinAreasAction.angleIsClockwise(makeNode(1,1), makeNode(0,1), makeNode(0,0)));
+        Assert.assertTrue(!JoinAreasAction.angleIsClockwise(makeNode(1,1), makeNode(0,1), makeNode(1,0)));
+    }
+
+    @Test
+    public void testisToTheRightSideOfLine()
+    {
+        Assert.assertTrue(JoinAreasAction.isToTheRightSideOfLine(makeNode(0,0), makeNode(1,1), makeNode(0,1), makeNode(0, 0.5)));
+        Assert.assertTrue(!JoinAreasAction.isToTheRightSideOfLine(makeNode(0,0), makeNode(1,1), makeNode(0,1), makeNode(1, 0)));
+        Assert.assertTrue(!JoinAreasAction.isToTheRightSideOfLine(makeNode(1,1), makeNode(0,1), makeNode(1,0), makeNode(0,0)));
+        Assert.assertTrue(JoinAreasAction.isToTheRightSideOfLine(makeNode(1,1), makeNode(0,1), makeNode(1,0), makeNode(2, 0)));
+    }
+
+}