diff --git a/src/org/openstreetmap/josm/data/validation/tests/CycleDetector.java b/src/org/openstreetmap/josm/data/validation/tests/CycleDetector.java index 92f4ec7d7cb..450a3be5f6c 100644 --- a/src/org/openstreetmap/josm/data/validation/tests/CycleDetector.java +++ b/src/org/openstreetmap/josm/data/validation/tests/CycleDetector.java @@ -15,9 +15,11 @@ import java.util.Set; import java.util.stream.Collectors; +import org.openstreetmap.josm.data.osm.BBox; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.NodeGraph; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.QuadBuckets; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.osm.WaySegment; import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper; @@ -69,26 +71,39 @@ public void visit(Way w) { public void startTest(ProgressMonitor progressMonitor) { super.startTest(progressMonitor); directionalWaterways = Config.getPref().getList(PREFIX + ".directionalWaterways", - Arrays.asList("river", "stream", "tidal_channel", "drain", "ditch", "fish_pass", "fairway")); + Arrays.asList("river", "stream", "tidal_channel", "drain", "ditch", "fish_pass", "fairway")); } @Override public void endTest() { + final QuadBuckets quadBuckets = new QuadBuckets<>(); + quadBuckets.addAll(usableWaterways); + for (Collection graph : getGraphs()) { NodeGraph nodeGraph = NodeGraph.createDirectedGraphFromWays(graph); Tarjan tarjan = new Tarjan(nodeGraph); Collection> scc = tarjan.getSCC(); - Map> graphMap = tarjan.getGraphMap(); - for (Collection possibleCycle : scc) { + for (List possibleCycle : scc) { // there is a cycle in the graph if a strongly connected component has more than one node if (possibleCycle.size() > 1) { + // build bbox to locate the issue + BBox bBox = new BBox(); + possibleCycle.forEach(node -> bBox.addPrimitive(node, 0)); + // find ways within this bbox + List waysWithinErrorBbox = quadBuckets.search(bBox); + List toReport = waysWithinErrorBbox.stream() + .filter(w -> possibleCycle.stream() + .anyMatch(w.getNodes()::contains)) + .collect(Collectors.toList()); + + Map> graphMap = tarjan.getGraphMap(); errors.add( - TestError.builder(this, Severity.ERROR, CYCLE_DETECTED) - .message(trc("graph theory", "Cycle in directional waterway network")) - .primitives(possibleCycle) - .highlightWaySegments(createSegments(graphMap, possibleCycle)) - .build() + TestError.builder(this, Severity.ERROR, CYCLE_DETECTED) + .message(trc("graph theory", "Cycle in directional waterway network")) + .primitives(toReport) + .highlightWaySegments(createSegments(graphMap, possibleCycle)) + .build() ); } } @@ -203,9 +218,9 @@ private Collection buildGraph(Way way) { for (Node node : currentWay.getNodes()) { Collection referrers = node.referrers(Way.class) - .filter(this::isPrimitiveUsable) - .filter(candidate -> candidate != currentWay) - .collect(Collectors.toList()); + .filter(this::isPrimitiveUsable) + .filter(candidate -> candidate != currentWay) + .collect(Collectors.toList()); if (!referrers.isEmpty()) { for (Way referrer : referrers) {