Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/Highways.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/Highways.java (revision 14778) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/Highways.java (revision 14779) @@ -9,9 +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,5 +23,4 @@ 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,19 +42,8 @@ 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 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 CLASSIFIED_HIGHWAYS = Arrays.asList( "motorway", "motorway_link", @@ -69,5 +57,4 @@ // CHECKSTYLE.ON: SingleSpaceSeparator - private static final Set KNOWN_SOURCE_MAXSPEED_CONTEXTS = new HashSet<>(Arrays.asList( "urban", "rural", "zone", "zone20", "zone:20", "zone30", "zone:30", "zone40", @@ -112,5 +99,5 @@ 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,151 +165,37 @@ 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 otherWays = new HashSet<>(); - otherWays.addAll(Utils.filteredCollection(n.getReferrers(), Way.class)); - if (otherWays.size() == 1) - continue; - Iterator 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 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 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 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 < 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 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(); } Index: /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/HighwaysTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/HighwaysTest.java (revision 14778) +++ /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/HighwaysTest.java (revision 14779) @@ -117,4 +117,9 @@ 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()); } }