Index: trunk/src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java (revision 15343) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java (revision 15344) @@ -7,6 +7,6 @@ import java.awt.geom.Area; -import java.awt.geom.Line2D; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -16,4 +16,5 @@ import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.Set; @@ -25,4 +26,5 @@ import org.openstreetmap.josm.data.osm.OsmDataManager; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmUtils; import org.openstreetmap.josm.data.osm.QuadBuckets; import org.openstreetmap.josm.data.osm.Way; @@ -34,4 +36,6 @@ import org.openstreetmap.josm.gui.progress.ProgressMonitor; import org.openstreetmap.josm.spi.preferences.Config; +import org.openstreetmap.josm.tools.Geometry; +import org.openstreetmap.josm.tools.Logging; /** @@ -50,4 +54,13 @@ protected abstract boolean isCandidate(OsmPrimitive p); + /** + * Check if unconnected end node should be ignored. + * @param n the node + * @return true if node should be ignored + */ + protected boolean ignoreUnconnectedEndNode(Node n) { + return false; + } + @Override public boolean isPrimitiveUsable(OsmPrimitive p) { @@ -71,4 +84,13 @@ protected boolean isCandidate(OsmPrimitive p) { return p.hasKey(HIGHWAY); + } + + @Override + protected boolean ignoreUnconnectedEndNode(Node n) { + return n.hasTag(HIGHWAY, "turning_circle", "bus_stop") + || n.hasTag("amenity", "parking_entrance") + || n.isKeyTrue("noexit") + || n.hasKey("entrance", "barrier") + || n.getParentWays().stream().anyMatch(Test::isBuilding); } } @@ -88,5 +110,10 @@ @Override protected boolean isCandidate(OsmPrimitive p) { - return p.hasKey(RAILWAY) && !p.hasTag(RAILWAY, "abandoned"); + return p.hasTagDifferent(RAILWAY, "abandoned"); + } + + @Override + protected boolean ignoreUnconnectedEndNode(Node n) { + return n.hasTag(RAILWAY, "buffer_stop"); } } @@ -124,5 +151,5 @@ @Override protected boolean isCandidate(OsmPrimitive p) { - return p.hasKey("natural", "landuse") && !p.hasTag("natural", "tree_row", "cliff"); + return p.hasKey("landuse") || p.hasTagDifferent("natural", "tree_row", "cliff"); } } @@ -143,4 +170,9 @@ protected boolean isCandidate(OsmPrimitive p) { return p.hasTag("power", "line", "minor_line", "cable"); + } + + @Override + protected boolean ignoreUnconnectedEndNode(Node n) { + return n.hasTag("power", "terminal"); } } @@ -160,4 +192,5 @@ private double mindist; private double minmiddledist; + private double maxLen; // maximum length of allowed detour to reach the unconnected node private DataSet ds; @@ -189,4 +222,5 @@ super.startTest(monitor); waySegments = new ArrayList<>(); + searchNodes = new QuadBuckets<>(); waysToTest = new HashSet<>(); nodesToTest = new HashSet<>(); @@ -200,5 +234,5 @@ } - protected Map getWayEndNodesNearOtherHighway() { + protected Map getHighwayEndNodesNearOtherHighway() { Map map = new HashMap<>(); for (MyWaySegment s : waySegments) { @@ -207,15 +241,19 @@ return map; } - for (Node en : s.nearbyNodes(mindist)) { - if (en.hasTag(HIGHWAY, "turning_circle", "bus_stop") - || en.hasTag("amenity", "parking_entrance") - || en.hasTag(RAILWAY, "buffer_stop") - || en.isKeyTrue("noexit") - || en.hasKey("entrance", "barrier")) { - continue; - } - // to handle intersections of 't' shapes and similar - if (!en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) { - addIfNewOrCloser(map, en, s); + for (Node endnode : s.nearbyNodes(mindist)) { + Way parentWay = getWantedParentWay(endnode); + if (parentWay != null) { + if (!Objects.equals(OsmUtils.getLayer(s.w), OsmUtils.getLayer(parentWay))) { + continue; // ignore ways with different layer tag + } + + // to handle intersections of 't' shapes and similar + if (!s.isConnectedTo(endnode)) { + if (parentWay.hasTag(HIGHWAY, "platform") || s.w.hasTag(HIGHWAY, "platform") + || s.barrierBetween(endnode)) { + continue; + } + addIfNewOrCloser(map, endnode, s); + } } } @@ -226,4 +264,5 @@ protected Map getWayEndNodesNearOtherWay() { Map map = new HashMap<>(); + for (MyWaySegment s : waySegments) { if (isCanceled()) { @@ -232,7 +271,22 @@ } if (!s.concernsArea) { - for (Node en : s.nearbyNodes(mindist)) { - if (!en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) { - addIfNewOrCloser(map, en, s); + for (Node endnode : s.nearbyNodes(mindist)) { + if (!s.isConnectedTo(endnode)) { + if (s.w.hasTag("power")) { + boolean badConnection = false; + Way otherWay = getWantedParentWay(endnode); + if (otherWay != null) { + for (String key : Arrays.asList("voltage", "frequency")) { + String v1 = s.w.get(key); + String v2 = otherWay.get(key); + if (v1 != null && v2 != null && !v1.equals(v2)) { + badConnection = true; + } + } + } + if (badConnection) + continue; + } + addIfNewOrCloser(map, endnode, s); } } @@ -250,5 +304,5 @@ } for (Node en : s.nearbyNodes(minmiddledist)) { - if (!en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) { + if (!s.isConnectedTo(en)) { addIfNewOrCloser(map, en, s); } @@ -258,5 +312,22 @@ } + /** + * An unconnected node might have multiple parent ways, e.g. a highway and a landuse way. + * Make sure we get the one that was analysed before. + * @param endnode the node which is known to be an end node of the wanted way + * @return the wanted way + */ + private Way getWantedParentWay(Node endnode) { + for (Way w : endnode.getParentWays()) { + if (isCandidate(w)) + return w; + } + Logging.error("end node without matching parent way"); + return null; + } + private void addIfNewOrCloser(Map map, Node node, MyWaySegment ws) { + if (partialSelection && !nodesToTest.contains(node) && !waysToTest.contains(ws.w)) + return; MyWaySegment old = map.get(node); if (old != null) { @@ -275,6 +346,4 @@ Node node = error.getKey(); MyWaySegment ws = error.getValue(); - if (partialSelection && !nodesToTest.contains(node) && !waysToTest.contains(ws.w)) - continue; errors.add(TestError.builder(this, severity, code) .message(message) @@ -291,8 +360,5 @@ for (Way w : ds.getWays()) { - if (w.isUsable() && isCandidate(w) && w.getRealNodesCount() > 1 - // don't complain about highways ending near platforms - && !w.hasTag(HIGHWAY, "platform") && !w.hasTag(RAILWAY, "platform", "platform_edge") - ) { + if (w.isUsable() && isCandidate(w) && w.getRealNodesCount() > 1) { waySegments.addAll(getWaySegments(w)); addNode(w.firstNode(), endnodes); @@ -300,22 +366,24 @@ } } - searchNodes = new QuadBuckets<>(); - searchNodes.addAll(endnodes); - if (isHighwayTest) { - addErrors(Severity.WARNING, getWayEndNodesNearOtherHighway(), tr("Way end node near other highway")); - } else { - addErrors(Severity.WARNING, getWayEndNodesNearOtherWay(), tr("Way end node near other way")); - } - - /* the following two use a shorter distance */ + fillSearchNodes(endnodes); + if (!searchNodes.isEmpty()) { + maxLen = 4 * mindist; + if (isHighwayTest) { + addErrors(Severity.WARNING, getHighwayEndNodesNearOtherHighway(), tr("Way end node near other highway")); + } else { + addErrors(Severity.WARNING, getWayEndNodesNearOtherWay(), tr("Way end node near other way")); + } + } + + /* the following two should use a shorter distance */ boolean includeOther = isBeforeUpload ? ValidatorPrefHelper.PREF_OTHER_UPLOAD.get() : ValidatorPrefHelper.PREF_OTHER.get(); if (minmiddledist > 0.0 && includeOther) { - searchNodes.clear(); - searchNodes.addAll(middlenodes); + maxLen = 4 * minmiddledist; + fillSearchNodes(middlenodes); addErrors(Severity.OTHER, getWayNodesNearOtherWay(), tr("Way node near other way")); - searchNodes.clear(); - searchNodes.addAll(othernodes); + fillSearchNodes(othernodes); addErrors(Severity.OTHER, getWayNodesNearOtherWay(), tr("Connected way end node near other way")); } + waySegments = null; endnodes = null; @@ -328,4 +396,13 @@ } + private void fillSearchNodes(Collection nodes) { + searchNodes.clear(); + for (Node n : nodes) { + if (!ignoreUnconnectedEndNode(n) && n.getCoor().isIn(dsArea)) { + searchNodes.add(n); + } + } + } + private class MyWaySegment { public final Way w; @@ -341,11 +418,62 @@ } + /** + * Check if the given node is connected to this segment using a reasonable short way. + * @param startNode the node + * @return true if a reasonable connection was found + */ + boolean isConnectedTo(Node startNode) { + return isConnectedTo(startNode, null, new HashSet<>(), 0); + } + + /** + * Check if the given node is connected to this segment using a reasonable short way. + * @param node the given node + * @param startWay previously visited way or null if first + * @param visited set of visited nodes + * @param len length of the travelled route + * @return true if a reasonable connection was found + */ + boolean isConnectedTo(Node node, Way startWay, Set visited, double len) { + if (n1 == node || n2 == node) { + return true; + } + if (len > maxLen) { + return false; + } + if (visited != null) { + visited.add(node); + for (final Way way : node.getParentWays()) { + if (isCandidate(way)) { + List nextNodes = new ArrayList<>(); + int pos = way.getNodes().indexOf(node); + if (pos > 0) { + nextNodes.add(way.getNode(pos - 1)); + } + if (pos + 1 < way.getNodesCount()) { + nextNodes.add(way.getNode(pos + 1)); + } + for (Node next : nextNodes) { + final boolean containsN = visited.contains(next); + visited.add(next); + if (!containsN && isConnectedTo(next, way, visited, + len + node.getCoor().greatCircleDistance(next.getCoor()))) { + return true; + } + } + } + } + } + return false; + } + double getDist(Node n) { EastNorth coord = n.getEastNorth(); if (coord == null) return Double.NaN; - EastNorth en1 = n1.getEastNorth(); - EastNorth en2 = n2.getEastNorth(); - return Line2D.ptSegDist(en1.getX(), en1.getY(), en2.getX(), en2.getY(), coord.getX(), coord.getY()); + EastNorth closest = Geometry.closestPointToSegment(n1.getEastNorth(), n2.getEastNorth(), coord); + Node x = new Node(); + x.setEastNorth(closest); + return x.getCoor().greatCircleDistance(n.getCoor()); } @@ -353,6 +481,4 @@ boolean nearby(Node n, double dist) { if (w.containsNode(n)) - return false; - if (n.isKeyTrue("noexit")) return false; double d = getDist(n); @@ -391,5 +517,5 @@ List foundNodes = searchNodes.search(bounds); for (Node n : foundNodes) { - if (!nearby(n, dist) || !n.getCoor().isIn(dsArea)) { + if (!nearby(n, dist)) { continue; } @@ -404,4 +530,23 @@ return result == null ? Collections.emptyList() : result; } + + private boolean barrierBetween(Node endnode) { + EastNorth closest = Geometry.closestPointToSegment(n1.getEastNorth(), n2.getEastNorth(), endnode.getEastNorth()); + Node x = new Node(); + x.setEastNorth(closest); + BBox bbox = new BBox(endnode.getCoor(), x.getCoor()); + for (Way nearbyWay: ds.searchWays(bbox)) { + if (nearbyWay != w && nearbyWay.hasTag("barrier") && !endnode.getParentWays().contains(nearbyWay)) { + //make sure that the barrier is really between the two nodes, not just close to them + Way directWay = new Way(); + directWay.addNode(endnode); + directWay.addNode(x); + if (!Geometry.addIntersections(Arrays.asList(nearbyWay, directWay), true, new ArrayList<>()).isEmpty()) + return true; + } + } + return false; + } + } Index: trunk/src/org/openstreetmap/josm/tools/Geometry.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/Geometry.java (revision 15343) +++ trunk/src/org/openstreetmap/josm/tools/Geometry.java (revision 15344) @@ -169,9 +169,4 @@ if (intersection != null) { - if (test) { - intersectionNodes.add(seg2Node1); - return intersectionNodes; - } - Node newNode = new Node(ProjectionRegistry.getProjection().eastNorth2latlon(intersection)); Node intNode = newNode; @@ -196,4 +191,9 @@ } else { insertInSeg2 = true; + } + + if (test) { + intersectionNodes.add(intNode); + return intersectionNodes; }