Ticket #5257: 5257.patch

src/org/openstreetmap/josm/data/validation/tests/WronglyOrderedWays.java

@@ -3 +3 @@
 
 import static org.openstreetmap.josm.tools.I18n.tr;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Collections;
 
-import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.validation.Severity;
 import org.openstreetmap.josm.data.validation.Test;
 import org.openstreetmap.josm.data.validation.TestError;
-import org.openstreetmap.josm.gui.progress.ProgressMonitor;
+import org.openstreetmap.josm.tools.Geometry;
 
 /**
  * Check cyclic ways for errors
@@ -33 +31 @@
     }
 
     @Override
-    public void startTest(ProgressMonitor monitor) {
-        super.startTest(monitor);
-    }
-
-    @Override
-    public void endTest() {
-        super.endTest();
-    }
-
-    @Override
     public void visit(Way w) {
-        String errortype = "";
-        int type;
 
-        if (!w.isUsable())
+        if (!w.isUsable() || !w.isClosed())
             return;
-        if (w.getNodesCount() <= 0)
-            return;
 
         String natural = w.get("natural");
-        if (natural == null)
+        if (natural == null) {
             return;
-
-        if (natural.equals("coastline")) {
-            errortype = tr("Reversed coastline: land not on left side");
-            type= WRONGLY_ORDERED_COAST;
-        } else if (natural.equals("water")) {
-            errortype = tr("Reversed water: land not on left side");
-            type= WRONGLY_ORDERED_WATER;
-        } else if (natural.equals("land")) {
-            errortype = tr("Reversed land: land not on left side");
-            type= WRONGLY_ORDERED_LAND;
-        } else
+        } else if ("coastline".equals(natural) && Geometry.isClockwise(w)) {
+            reportError(w, tr("Reversed coastline: land not on left side"), WRONGLY_ORDERED_COAST);
+        } else if ("water".equals(natural) && !Geometry.isClockwise(w)) {
+            reportError(w, tr("Reversed water: land not on left side"), WRONGLY_ORDERED_WATER);
+        } else if ("land".equals(natural) && Geometry.isClockwise(w)) {
+            reportError(w, tr("Reversed land: land not on left side"), WRONGLY_ORDERED_LAND);
+        } else {
             return;
+        }
 
-        /**
-         * Test the directionality of the way
-         *
-         * Assuming a closed non-looping way, compute twice the area
-         * of the polygon using the formula 2*a = sum (Xn * Yn+1 - Xn+1 * Yn)
-         * If the area is negative the way is ordered in a clockwise direction
-         *
-         */
-        if (w.getNode(0) == w.getNode(w.getNodesCount()-1)) {
-            double area2 = 0;
+    }
 
-            for (int node = 1; node < w.getNodesCount(); node++) {
-                area2 += (w.getNode(node-1).getCoor().lon() * w.getNode(node).getCoor().lat()
-                - w.getNode(node).getCoor().lon() * w.getNode(node-1).getCoor().lat());
-            }
-
-            if (((natural.equals("coastline") || natural.equals("land")) && area2 < 0.)
-                    || (natural.equals("water") && area2 > 0.)) {
-                List<OsmPrimitive> primitives = new ArrayList<OsmPrimitive>();
-                primitives.add(w);
-                errors.add( new TestError(this, Severity.OTHER, errortype, type, primitives) );
-            }
-        }
+    private void reportError(Way w, String msg, int type) {
+        errors.add(new TestError(this, Severity.OTHER, msg, type, Collections.singletonList(w)));
     }
 }

src/org/openstreetmap/josm/tools/Geometry.java

@@ -14 +14 @@
 import org.openstreetmap.josm.command.ChangeCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.data.coor.EastNorth;
+import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.BBox;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.NodePositionComparator;
@@ -220 +221 @@
      * @return true if first vector is clockwise before second vector.
      */
     public static boolean angleIsClockwise(Node commonNode, Node firstNode, Node secondNode) {
-        double dy1 = (firstNode.getEastNorth().getY() - commonNode.getEastNorth().getY());
-        double dy2 = (secondNode.getEastNorth().getY() - commonNode.getEastNorth().getY());
-        double dx1 = (firstNode.getEastNorth().getX() - commonNode.getEastNorth().getX());
-        double dx2 = (secondNode.getEastNorth().getX() - commonNode.getEastNorth().getX());
-
-        return dy1 * dx2 - dx1 * dy2 > 0;
+        return angleIsClockwise(commonNode.getEastNorth(), firstNode.getEastNorth(), secondNode.getEastNorth());
     }
 
     /**
@@ -526 +522 @@
         return 6367000 * c;
     }
 
+    /**
+     * Determines whether a way is oriented clockwise.
+     *
+     * Internals: Assuming a closed non-looping way, compute twice the area
+     * of the polygon using the formula {@code 2 * area = sum (X[n] * Y[n+1] - X[n+1] * Y[n])}.
+     * If the area is negative the way is ordered in a clockwise direction.
+     *
+     * @param w the way to be checked.
+     * @return true if and only if way is oriented clockwise.
+     * @throws IllegalArgumentException if way is not closed (see {@see Way#isClosed}).
+     * @see http://paulbourke.net/geometry/polyarea/
+     */
+    public static boolean isClockwise(Way w) {
+        if (!w.isClosed()) {
+            throw new IllegalArgumentException("Way must be closed to check orientation.");
+        }
 
+        double area2 = 0.;
+        int nodesCount = w.getNodesCount();
+
+        for (int node = 1; node <= /*sic! consider last-first as well*/ nodesCount; node++) {
+            LatLon coorPrev = w.getNode(node - 1).getCoor();
+            LatLon coorCurr = w.getNode(node % nodesCount).getCoor();
+            area2 += coorPrev.lon() * coorCurr.lat();
+            area2 -= coorCurr.lon() * coorPrev.lat();
+        }
+        return area2 < 0;
+    }
 }