Index: src/org/openstreetmap/josm/data/validation/tests/Highways.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/Highways.java (revision 14530) +++ src/org/openstreetmap/josm/data/validation/tests/Highways.java (working copy) @@ -8,11 +8,11 @@ import java.util.Arrays; 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; import org.openstreetmap.josm.data.osm.Node; @@ -22,6 +22,7 @@ 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.tools.Geometry; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; @@ -41,10 +42,18 @@ protected static final String SOURCE_MAXSPEED = "source:maxspeed"; + // 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", "trunk", "trunk_link", @@ -56,6 +65,7 @@ "living_street"); // 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", "nsl_single", "nsl_dual", "motorway", "trunk", "living_street", "bicycle_road")); @@ -98,7 +108,7 @@ if (w.isUsable()) { if (w.isClosed() && w.hasTag(HIGHWAY, CLASSIFIED_HIGHWAYS) && w.hasTag("junction", "roundabout") && IN_DOWNLOADED_AREA_STRICT.test(w)) { - // TODO: find out how to handle splitted roundabouts (see #12841) + // TODO: find out how to handle split roundabouts (see #12841) testWrongRoundabout(w); } if (w.hasKey(SOURCE_MAXSPEED)) { @@ -164,41 +174,159 @@ */ public static boolean isHighwayLinkOkay(final Way way) { final String highway = way.get(HIGHWAY); - if (highway == null || !highway.endsWith("_link") - || !IN_DOWNLOADED_AREA.test(way.getNode(0)) || !IN_DOWNLOADED_AREA.test(way.getNode(way.getNodesCount()-1))) { + if (highway == null || !highway.endsWith("_link")) { return true; } - final Set referrers = new HashSet<>(); + // 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 (way.isClosed()) { - // for closed way we need to check all adjacent ways - for (Node n: way.getNodes()) { - referrers.addAll(n.getReferrers()); + 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 || !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) { + if ("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; } else { - referrers.addAll(way.firstNode().getReferrers()); - referrers.addAll(way.lastNode().getReferrers()); + return way.getNode(nodePos + 1); } + } - // 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; + 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) { + if ("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; } } - // Link roads should always at least one adjacent segment of same class - return !sameClass.isEmpty(); + return true; } + /** + * Returns true if angle of a corner defined with 3 point coordinates is < 60. + * + * @param n1 first node + * @param n2 Common node + * @param n3 third node + * @return true if angle is below 60. + */ + + private static boolean isSharpAngle(Node n1, Node n2, Node n3) { + double angle = Geometry.getNormalizedAngleInDegrees( + Geometry.getCornerAngle(n1.getEastNorth(), n2.getEastNorth(), n3.getEastNorth())); + return angle < 60; + } + private void testHighwayLink(final Way way) { if (!isHighwayLinkOkay(way)) { errors.add(TestError.builder(this, Severity.WARNING, SOURCE_WRONG_LINK) Index: test/unit/org/openstreetmap/josm/data/validation/tests/HighwaysTest.java =================================================================== --- test/unit/org/openstreetmap/josm/data/validation/tests/HighwaysTest.java (revision 14530) +++ test/unit/org/openstreetmap/josm/data/validation/tests/HighwaysTest.java (working copy) @@ -116,11 +116,6 @@ if (!test.getErrors().isEmpty()) { 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()); } } }