Changeset 14779 in josm for trunk

Timestamp:

2019-02-10T16:03:37+01:00 (6 years ago)

Author:

GerdP

Message:

see #16803: revert r14777,r14775, and r14772: too many side effects, patch needs more review

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/validation/tests/Highways.java (modified) (6 diffs)
• test/unit/org/openstreetmap/josm/data/validation/tests/HighwaysTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/data/validation/tests/Highways.java

@@ -9 +9 @@
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.openstreetmap.josm.command.ChangePropertyCommand;
@@ -23 +23 @@
 import org.openstreetmap.josm.data.validation.Test;
 import org.openstreetmap.josm.data.validation.TestError;
-import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Utils;
@@ -43 +42 @@
     protected static final String SOURCE_MAXSPEED = "source:maxspeed";
 
-    /** threshold value for angles between two highway segments. */
-    private static final int MIN_ANGLE_NOT_SHARP = 60;
-
-    // CHECKSTYLE.OFF: SingleSpaceSeparator
-    private static final Set<String> LINK_TO_HIGHWAYS = new HashSet<>(Arrays.asList(
-            "motorway",  "motorway_link",
-            "trunk",     "trunk_link",
-            "primary",   "primary_link",
-            "secondary", "secondary_link",
-            "tertiary",  "tertiary_link"
-            ));
-
     /**
      * Classified highways in order of importance
      */
+    // CHECKSTYLE.OFF: SingleSpaceSeparator
     private static final List<String> CLASSIFIED_HIGHWAYS = Arrays.asList(
             "motorway",  "motorway_link",
@@ -69 +57 @@
     // CHECKSTYLE.ON: SingleSpaceSeparator
 
-
     private static final Set<String> KNOWN_SOURCE_MAXSPEED_CONTEXTS = new HashSet<>(Arrays.asList(
             "urban", "rural", "zone", "zone20", "zone:20", "zone30", "zone:30", "zone40",
@@ -112 +99 @@
             if (w.isClosed() && w.hasTag(HIGHWAY, CLASSIFIED_HIGHWAYS) && w.hasTag("junction", "roundabout")
                     && IN_DOWNLOADED_AREA_STRICT.test(w)) {
-                // TODO: find out how to handle split roundabouts (see #12841)
+                // TODO: find out how to handle splitted roundabouts (see #12841)
                 testWrongRoundabout(w);
             }
@@ -178 +165 @@
     public static boolean isHighwayLinkOkay(final Way way) {
         final String highway = way.get(HIGHWAY);
-        if (highway == null || !highway.endsWith("_link")) {
+        if (highway == null || !highway.endsWith("_link")
+                || !IN_DOWNLOADED_AREA.test(way.getNode(0)) || !IN_DOWNLOADED_AREA.test(way.getNode(way.getNodesCount()-1))) {
             return true;
         }
 
-        // check if connected to a high class road where the link must match the higher class
-        String highClass = null;
-        for (int i = 0; i < way.getNodesCount(); i++) {
-            Node n = way.getNode(i);
-            if (!IN_DOWNLOADED_AREA.test(n))
-                return true;
-            Set<Way> otherWays = new HashSet<>();
-            otherWays.addAll(Utils.filteredCollection(n.getReferrers(), Way.class));
-            if (otherWays.size() == 1)
-                continue;
-            Iterator<Way> iter = otherWays.iterator();
-            while (iter.hasNext()) {
-                Way w = iter.next();
-                final String hw2 = w.get(HIGHWAY);
-                if (way == w || w.getNodesCount() < 2 || !w.isUsable() || hw2 == null)
-                    iter.remove();
-                else {
-                    if ("motorway".equals(hw2)) {
-                        highClass = "motorway";
-                        break;
-                    } else if ("trunk".equals(hw2))
-                        highClass = "trunk";
-                }
-            }
-        }
-
-        if (highClass != null && !highway.equals(highClass + "_link")) {
-            return false;
-        }
-
-        for (int i = 0; i < way.getNodesCount(); i++) {
-            Node n = way.getNode(i);
-            Set<Way> otherWays = new HashSet<>();
-            otherWays.addAll(Utils.filteredCollection(n.getReferrers(), Way.class));
-            if (otherWays.size() == 1)
-                continue;
-            otherWays.removeIf(w -> w == way || !w.hasTag("highway") || !highway.startsWith(w.get(HIGHWAY)) || !LINK_TO_HIGHWAYS.contains(w.get(HIGHWAY)));
-            if (otherWays.isEmpty())
-                continue;
-
-            //TODO: ignore ways which are not allowed because of turn restrictions, oneway attributes or access rules?
-            HashSet<Way> sameTag = new HashSet<>();
-            for (Way ow : otherWays) {
-                if (highway.equals(ow.get(HIGHWAY)))
-                    sameTag.add(ow);
-                else
-                    return true;
-            }
-            // we have way(s) with the same _link tag, ignore those with a sharp angle
-            final int pos = i;
-            sameTag.removeIf(w -> isSharpAngle(way, pos, w));
-            if (!sameTag.isEmpty())
-                return true;
-        }
-        return false;
-
-    }
-
-    /**
-     * Check if the two given connected ways form a sharp angle.
-     * @param way 1st way
-     * @param nodePos node position of connecting node in 1st way
-     * @param otherWay the 2nd way
-     * @return true if angle is sharp or way cannot be travelled because of oneway attributes
-     */
-    private static boolean isSharpAngle(Way way, int nodePos, Way otherWay) {
-        Node n = way.getNode(nodePos);
-        int oneway = way.isOneway();
-        if (oneway == 0 && "roundabout".equals(way.get("junction"))) {
-            oneway = 1;
-        }
-
-        if (oneway != 1) {
-            Node prev = getPrevNode(way, nodePos);
-            if (prev != null && !onlySharpAngle(n, prev, otherWay))
-                return false;
-        }
-        if (oneway != -1) {
-            Node next = getNextNode(way, nodePos);
-            if (next != null && !onlySharpAngle(n, next, otherWay))
-                return false;
-        }
-        return true;
-    }
-
-    private static Node getNextNode(Way way, int nodePos) {
-        if (nodePos + 1 >= way.getNodesCount()) {
-            if (way.isClosed())
-                return way.getNode(1);
-            return null;
+        final Set<OsmPrimitive> referrers = new HashSet<>();
+
+        if (way.isClosed()) {
+            // for closed way we need to check all adjacent ways
+            for (Node n: way.getNodes()) {
+                referrers.addAll(n.getReferrers());
+            }
         } else {
-            return way.getNode(nodePos + 1);
-        }
-    }
-
-    private static Node getPrevNode(Way way, int nodePos) {
-        if (nodePos == 0) {
-            if (way.isClosed())
-                return way.getNode(way.getNodesCount() - 2);
-            return null;
-        } else {
-            return way.getNode(nodePos - 1);
-        }
-    }
-
-    private static boolean onlySharpAngle(Node common, Node from, Way toWay) {
-        int oneway = toWay.isOneway();
-        if (oneway == 0 && "roundabout".equals(toWay.get("junction"))) {
-            oneway = 1;
-        }
-
-        for (int i = 0; i < toWay.getNodesCount(); i++) {
-            if (common == toWay.getNode(i)) {
-
-                if (oneway != 1) {
-                    Node to = getNextNode(toWay, i);
-                    if (to != null && !isSharpAngle(from, common, to))
-                        return false;
-                }
-                if (oneway != -1) {
-                    Node to = getPrevNode(toWay, i);
-                    if (to != null && !isSharpAngle(from, common, to))
-                        return false;
-                }
-                break;
-            }
-        }
-        return true;
-    }
-
-    /**
-     * Returns true if angle of a corner defined with 3 point coordinates is &lt; MIN_ANGLE_NOT_SHARP
-     *
-     * @param n1 first node
-     * @param n2 Common node
-     * @param n3 third node
-     * @return true if angle is below value given in MIN_ANGLE_NOT_SHARP
-     */
-
-    private static boolean isSharpAngle(Node n1, Node n2, Node n3) {
-        double angle = Geometry.getNormalizedAngleInDegrees(
-                Geometry.getCornerAngle(n1.getEastNorth(), n2.getEastNorth(), n3.getEastNorth()));
-        return angle < MIN_ANGLE_NOT_SHARP;
+            referrers.addAll(way.firstNode().getReferrers());
+            referrers.addAll(way.lastNode().getReferrers());
+        }
+
+        // Find ways of same class (exact class of class_link)
+        List<Way> sameClass = Utils.filteredCollection(referrers, Way.class).stream().filter(
+                otherWay -> !way.equals(otherWay) && otherWay.hasTag(HIGHWAY, highway, highway.replaceAll("_link$", "")))
+                .collect(Collectors.toList());
+        if (sameClass.size() > 1) {
+            // It is possible to have a class_link between 2 segments of same class
+            // in roundabout designs that physically separate a specific turn from the main roundabout
+            // But if we have more than a single adjacent class, and one of them is a roundabout, that's an error
+            for (Way w : sameClass) {
+                if (w.hasTag("junction", "roundabout")) {
+                    return false;
+                }
+            }
+        }
+        // Link roads should always at least one adjacent segment of same class
+        return !sameClass.isEmpty();
     }
 

trunk/test/unit/org/openstreetmap/josm/data/validation/tests/HighwaysTest.java

@@ -117 +117 @@
                 fail(test.getErrors().get(0).getMessage());
             }
+            Way w1 = ways.stream().filter(w -> 28508494 == w.getId()).findFirst().get();
+            Way w2 = ways.stream().filter(w -> 28508493 == w.getId()).findFirst().get();
+            test.visit(w1);
+            test.visit(w2);
+            assertEquals(2, test.getErrors().size());
         }
     }