Changeset 15344 in josm


Ignore:
Timestamp:
2019-09-10T10:04:27+02:00 (12 days ago)
Author:
GerdP
Message:

see #6102, #11778: Improve UNCONNECTED_WAYS test

  • avoid to fill QuadBuckets structure with nodes outside downloaded area
  • reduce dependence on projection, calculate distance as greatCircleDistance
  • check if barrier is between unconnected highways
  • don't ignore nodes which are only "connected" via a long detour (4 x mindist), this replaces the method Node.isConnectedTo() which counted hops in a rather unpredictable way.
  • fix error in Geometry.addIntersections() which didn't return the intersection node when parameter test was true
  • don't report ways with different layer=* values

Test now typically produces a lot more warnings, esp. for sidewalks and for service roads like service=parking_aisle. Not sure if the latter should be supressed...

Location:
trunk/src/org/openstreetmap/josm
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java

    r15336 r15344  
    77
    88import java.awt.geom.Area;
    9 import java.awt.geom.Line2D;
    109import java.util.ArrayList;
     10import java.util.Arrays;
    1111import java.util.Collection;
    1212import java.util.Collections;
     
    1616import java.util.Map;
    1717import java.util.Map.Entry;
     18import java.util.Objects;
    1819import java.util.Set;
    1920
     
    2526import org.openstreetmap.josm.data.osm.OsmDataManager;
    2627import org.openstreetmap.josm.data.osm.OsmPrimitive;
     28import org.openstreetmap.josm.data.osm.OsmUtils;
    2729import org.openstreetmap.josm.data.osm.QuadBuckets;
    2830import org.openstreetmap.josm.data.osm.Way;
     
    3436import org.openstreetmap.josm.gui.progress.ProgressMonitor;
    3537import org.openstreetmap.josm.spi.preferences.Config;
     38import org.openstreetmap.josm.tools.Geometry;
     39import org.openstreetmap.josm.tools.Logging;
    3640
    3741/**
     
    5054    protected abstract boolean isCandidate(OsmPrimitive p);
    5155
     56    /**
     57     * Check if unconnected end node should be ignored.
     58     * @param n the node
     59     * @return true if node should be ignored
     60     */
     61    protected boolean ignoreUnconnectedEndNode(Node n) {
     62        return false;
     63    }
     64
    5265    @Override
    5366    public boolean isPrimitiveUsable(OsmPrimitive p) {
     
    7184        protected boolean isCandidate(OsmPrimitive p) {
    7285            return p.hasKey(HIGHWAY);
     86        }
     87
     88        @Override
     89        protected boolean ignoreUnconnectedEndNode(Node n) {
     90            return n.hasTag(HIGHWAY, "turning_circle", "bus_stop")
     91                    || n.hasTag("amenity", "parking_entrance")
     92                    || n.isKeyTrue("noexit")
     93                    || n.hasKey("entrance", "barrier")
     94                    || n.getParentWays().stream().anyMatch(Test::isBuilding);
    7395        }
    7496    }
     
    88110        @Override
    89111        protected boolean isCandidate(OsmPrimitive p) {
    90             return p.hasKey(RAILWAY) && !p.hasTag(RAILWAY, "abandoned");
     112            return p.hasTagDifferent(RAILWAY, "abandoned");
     113        }
     114
     115        @Override
     116        protected boolean ignoreUnconnectedEndNode(Node n) {
     117            return n.hasTag(RAILWAY, "buffer_stop");
    91118        }
    92119    }
     
    124151        @Override
    125152        protected boolean isCandidate(OsmPrimitive p) {
    126             return p.hasKey("natural", "landuse") && !p.hasTag("natural", "tree_row", "cliff");
     153            return p.hasKey("landuse") || p.hasTagDifferent("natural", "tree_row", "cliff");
    127154        }
    128155    }
     
    143170        protected boolean isCandidate(OsmPrimitive p) {
    144171            return p.hasTag("power", "line", "minor_line", "cable");
     172        }
     173
     174        @Override
     175        protected boolean ignoreUnconnectedEndNode(Node n) {
     176            return n.hasTag("power", "terminal");
    145177        }
    146178    }
     
    160192    private double mindist;
    161193    private double minmiddledist;
     194    private double maxLen; // maximum length of allowed detour to reach the unconnected node
    162195    private DataSet ds;
    163196
     
    189222        super.startTest(monitor);
    190223        waySegments = new ArrayList<>();
     224        searchNodes = new QuadBuckets<>();
    191225        waysToTest = new HashSet<>();
    192226        nodesToTest = new HashSet<>();
     
    200234    }
    201235
    202     protected Map<Node, MyWaySegment> getWayEndNodesNearOtherHighway() {
     236    protected Map<Node, MyWaySegment> getHighwayEndNodesNearOtherHighway() {
    203237        Map<Node, MyWaySegment> map = new HashMap<>();
    204238        for (MyWaySegment s : waySegments) {
     
    207241                return map;
    208242            }
    209             for (Node en : s.nearbyNodes(mindist)) {
    210                 if (en.hasTag(HIGHWAY, "turning_circle", "bus_stop")
    211                         || en.hasTag("amenity", "parking_entrance")
    212                         || en.hasTag(RAILWAY, "buffer_stop")
    213                         || en.isKeyTrue("noexit")
    214                         || en.hasKey("entrance", "barrier")) {
    215                     continue;
    216                 }
    217                 // to handle intersections of 't' shapes and similar
    218                 if (!en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) {
    219                     addIfNewOrCloser(map, en, s);
     243            for (Node endnode : s.nearbyNodes(mindist)) {
     244                Way parentWay = getWantedParentWay(endnode);
     245                if (parentWay != null) {
     246                    if (!Objects.equals(OsmUtils.getLayer(s.w), OsmUtils.getLayer(parentWay))) {
     247                        continue; // ignore ways with different layer tag
     248                    }
     249
     250                    // to handle intersections of 't' shapes and similar
     251                    if (!s.isConnectedTo(endnode)) {
     252                        if (parentWay.hasTag(HIGHWAY, "platform") || s.w.hasTag(HIGHWAY, "platform")
     253                                || s.barrierBetween(endnode)) {
     254                            continue;
     255                        }
     256                        addIfNewOrCloser(map, endnode, s);
     257                    }
    220258                }
    221259            }
     
    226264    protected Map<Node, MyWaySegment> getWayEndNodesNearOtherWay() {
    227265        Map<Node, MyWaySegment> map = new HashMap<>();
     266
    228267        for (MyWaySegment s : waySegments) {
    229268            if (isCanceled()) {
     
    232271            }
    233272            if (!s.concernsArea) {
    234                 for (Node en : s.nearbyNodes(mindist)) {
    235                     if (!en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) {
    236                         addIfNewOrCloser(map, en, s);
     273                for (Node endnode : s.nearbyNodes(mindist)) {
     274                    if (!s.isConnectedTo(endnode)) {
     275                        if (s.w.hasTag("power")) {
     276                            boolean badConnection = false;
     277                            Way otherWay = getWantedParentWay(endnode);
     278                            if (otherWay != null) {
     279                                for (String key : Arrays.asList("voltage", "frequency")) {
     280                                    String v1 = s.w.get(key);
     281                                    String v2 = otherWay.get(key);
     282                                    if (v1 != null && v2 != null && !v1.equals(v2)) {
     283                                        badConnection = true;
     284                                    }
     285                                }
     286                            }
     287                            if (badConnection)
     288                                continue;
     289                        }
     290                        addIfNewOrCloser(map, endnode, s);
    237291                    }
    238292                }
     
    250304            }
    251305            for (Node en : s.nearbyNodes(minmiddledist)) {
    252                 if (!en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) {
     306                if (!s.isConnectedTo(en)) {
    253307                    addIfNewOrCloser(map, en, s);
    254308                }
     
    258312    }
    259313
     314    /**
     315     * An unconnected node might have multiple parent ways, e.g. a highway and a landuse way.
     316     * Make sure we get the one that was analysed before.
     317     * @param endnode the node which is known to be an end node of the wanted way
     318     * @return the wanted way
     319     */
     320    private Way getWantedParentWay(Node endnode) {
     321        for (Way w : endnode.getParentWays()) {
     322            if (isCandidate(w))
     323                return w;
     324        }
     325        Logging.error("end node without matching parent way");
     326        return null;
     327    }
     328
    260329    private void addIfNewOrCloser(Map<Node, MyWaySegment> map, Node node, MyWaySegment ws) {
     330        if (partialSelection && !nodesToTest.contains(node) && !waysToTest.contains(ws.w))
     331            return;
    261332        MyWaySegment old = map.get(node);
    262333        if (old != null) {
     
    275346            Node node = error.getKey();
    276347            MyWaySegment ws = error.getValue();
    277             if (partialSelection && !nodesToTest.contains(node) && !waysToTest.contains(ws.w))
    278                 continue;
    279348            errors.add(TestError.builder(this, severity, code)
    280349                    .message(message)
     
    291360
    292361        for (Way w : ds.getWays()) {
    293             if (w.isUsable() && isCandidate(w) && w.getRealNodesCount() > 1
    294                     // don't complain about highways ending near platforms
    295                     && !w.hasTag(HIGHWAY, "platform") && !w.hasTag(RAILWAY, "platform", "platform_edge")
    296                     ) {
     362            if (w.isUsable() && isCandidate(w) && w.getRealNodesCount() > 1) {
    297363                waySegments.addAll(getWaySegments(w));
    298364                addNode(w.firstNode(), endnodes);
     
    300366            }
    301367        }
    302         searchNodes = new QuadBuckets<>();
    303         searchNodes.addAll(endnodes);
    304         if (isHighwayTest) {
    305             addErrors(Severity.WARNING, getWayEndNodesNearOtherHighway(), tr("Way end node near other highway"));
    306         } else {
    307             addErrors(Severity.WARNING, getWayEndNodesNearOtherWay(), tr("Way end node near other way"));
    308         }
    309 
    310         /* the following two use a shorter distance */
     368        fillSearchNodes(endnodes);
     369        if (!searchNodes.isEmpty()) {
     370            maxLen = 4 * mindist;
     371            if (isHighwayTest) {
     372                addErrors(Severity.WARNING, getHighwayEndNodesNearOtherHighway(), tr("Way end node near other highway"));
     373            } else {
     374                addErrors(Severity.WARNING, getWayEndNodesNearOtherWay(), tr("Way end node near other way"));
     375            }
     376        }
     377
     378        /* the following two should use a shorter distance */
    311379        boolean includeOther = isBeforeUpload ? ValidatorPrefHelper.PREF_OTHER_UPLOAD.get() : ValidatorPrefHelper.PREF_OTHER.get();
    312380        if (minmiddledist > 0.0 && includeOther) {
    313             searchNodes.clear();
    314             searchNodes.addAll(middlenodes);
     381            maxLen = 4 * minmiddledist;
     382            fillSearchNodes(middlenodes);
    315383            addErrors(Severity.OTHER, getWayNodesNearOtherWay(), tr("Way node near other way"));
    316             searchNodes.clear();
    317             searchNodes.addAll(othernodes);
     384            fillSearchNodes(othernodes);
    318385            addErrors(Severity.OTHER, getWayNodesNearOtherWay(), tr("Connected way end node near other way"));
    319386        }
     387
    320388        waySegments = null;
    321389        endnodes = null;
     
    328396    }
    329397
     398    private void fillSearchNodes(Collection<Node> nodes) {
     399        searchNodes.clear();
     400        for (Node n : nodes) {
     401            if (!ignoreUnconnectedEndNode(n) && n.getCoor().isIn(dsArea)) {
     402                searchNodes.add(n);
     403            }
     404        }
     405    }
     406
    330407    private class MyWaySegment {
    331408        public final Way w;
     
    341418        }
    342419
     420        /**
     421         * Check if the given node is connected to this segment using a reasonable short way.
     422         * @param startNode the node
     423         * @return true if a reasonable connection was found
     424         */
     425        boolean isConnectedTo(Node startNode) {
     426            return isConnectedTo(startNode, null, new HashSet<>(), 0);
     427        }
     428
     429        /**
     430         * Check if the given node is connected to this segment using a reasonable short way.
     431         * @param node the given node
     432         * @param startWay previously visited way or null if first
     433         * @param visited set of visited nodes
     434         * @param len length of the travelled route
     435         * @return true if a reasonable connection was found
     436         */
     437        boolean isConnectedTo(Node node, Way startWay, Set<Node> visited, double len) {
     438            if (n1 == node || n2 == node) {
     439                return true;
     440            }
     441            if (len > maxLen) {
     442                return false;
     443            }
     444            if (visited != null) {
     445                visited.add(node);
     446                for (final Way way : node.getParentWays()) {
     447                    if (isCandidate(way)) {
     448                        List<Node> nextNodes = new ArrayList<>();
     449                        int pos = way.getNodes().indexOf(node);
     450                        if (pos > 0) {
     451                            nextNodes.add(way.getNode(pos - 1));
     452                        }
     453                        if (pos + 1 < way.getNodesCount()) {
     454                            nextNodes.add(way.getNode(pos + 1));
     455                        }
     456                        for (Node next : nextNodes) {
     457                            final boolean containsN = visited.contains(next);
     458                            visited.add(next);
     459                            if (!containsN && isConnectedTo(next, way, visited,
     460                                    len + node.getCoor().greatCircleDistance(next.getCoor()))) {
     461                                return true;
     462                            }
     463                        }
     464                    }
     465                }
     466            }
     467            return false;
     468        }
     469
    343470        double getDist(Node n) {
    344471            EastNorth coord = n.getEastNorth();
    345472            if (coord == null)
    346473                return Double.NaN;
    347             EastNorth en1 = n1.getEastNorth();
    348             EastNorth en2 = n2.getEastNorth();
    349             return Line2D.ptSegDist(en1.getX(), en1.getY(), en2.getX(), en2.getY(), coord.getX(), coord.getY());
     474            EastNorth closest = Geometry.closestPointToSegment(n1.getEastNorth(), n2.getEastNorth(), coord);
     475            Node x = new Node();
     476            x.setEastNorth(closest);
     477            return x.getCoor().greatCircleDistance(n.getCoor());
    350478
    351479        }
     
    353481        boolean nearby(Node n, double dist) {
    354482            if (w.containsNode(n))
    355                 return false;
    356             if (n.isKeyTrue("noexit"))
    357483                return false;
    358484            double d = getDist(n);
     
    391517            List<Node> foundNodes = searchNodes.search(bounds);
    392518            for (Node n : foundNodes) {
    393                 if (!nearby(n, dist) || !n.getCoor().isIn(dsArea)) {
     519                if (!nearby(n, dist)) {
    394520                    continue;
    395521                }
     
    404530            return result == null ? Collections.emptyList() : result;
    405531        }
     532
     533        private boolean barrierBetween(Node endnode) {
     534            EastNorth closest = Geometry.closestPointToSegment(n1.getEastNorth(), n2.getEastNorth(), endnode.getEastNorth());
     535            Node x = new Node();
     536            x.setEastNorth(closest);
     537            BBox bbox = new BBox(endnode.getCoor(), x.getCoor());
     538            for (Way nearbyWay: ds.searchWays(bbox)) {
     539                if (nearbyWay != w && nearbyWay.hasTag("barrier") && !endnode.getParentWays().contains(nearbyWay)) {
     540                    //make sure that the barrier is really between the two nodes, not just close to them
     541                    Way directWay = new Way();
     542                    directWay.addNode(endnode);
     543                    directWay.addNode(x);
     544                    if (!Geometry.addIntersections(Arrays.asList(nearbyWay, directWay), true, new ArrayList<>()).isEmpty())
     545                        return true;
     546                }
     547            }
     548            return false;
     549        }
     550
    406551    }
    407552
  • trunk/src/org/openstreetmap/josm/tools/Geometry.java

    r15160 r15344  
    169169
    170170                            if (intersection != null) {
    171                                 if (test) {
    172                                     intersectionNodes.add(seg2Node1);
    173                                     return intersectionNodes;
    174                                 }
    175 
    176171                                Node newNode = new Node(ProjectionRegistry.getProjection().eastNorth2latlon(intersection));
    177172                                Node intNode = newNode;
     
    196191                                } else {
    197192                                    insertInSeg2 = true;
     193                                }
     194
     195                                if (test) {
     196                                    intersectionNodes.add(intNode);
     197                                    return intersectionNodes;
    198198                                }
    199199
Note: See TracChangeset for help on using the changeset viewer.