From 12987caa78966e19a533d29862555ff24bf812f7 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Sun, 24 Nov 2024 06:56:40 -0500 Subject: [PATCH] fix tests --- .../planetiler/util/LoopLineMerger.java | 29 ++++---- .../planetiler/util/LoopLineMergerTest.java | 66 +++++++++++-------- 2 files changed, 58 insertions(+), 37 deletions(-) diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LoopLineMerger.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LoopLineMerger.java index 80eed03497..d7bc0336c3 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LoopLineMerger.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LoopLineMerger.java @@ -37,10 +37,10 @@ * Merging in Planetiler */ public class LoopLineMerger { - final List input = new ArrayList<>(); + private final List input = new ArrayList<>(); private final List output = new ArrayList<>(); - int nodes = 0; - int edges = 0; + private int nodes = 0; + private int edges = 0; private PrecisionModel precisionModel = new PrecisionModel(GeoUtils.TILE_PRECISION); private GeometryFactory factory = new GeometryFactory(precisionModel); private double minLength = 0.0; @@ -304,13 +304,17 @@ private void removeShortEdges() { } private void simplify() { + List toRemove = new ArrayList<>(); for (var node : output) { for (var edge : node.getEdges()) { if (edge.main) { - edge.simplify(); + if (!edge.simplify()) { + toRemove.add(edge); + } } } } + toRemove.forEach(Edge::remove); } private void removeDuplicatedEdges() { @@ -320,7 +324,7 @@ private void removeDuplicatedEdges() { Edge a = node.getEdges().get(i); for (var j = i + 1; j < node.getEdges().size(); ++j) { Edge b = node.getEdges().get(j); - if (a.coordinates.equals(b.coordinates)) { + if (b.to == a.to && a.coordinates.equals(b.coordinates)) { toRemove.add(b); } } @@ -361,6 +365,7 @@ public List getMergedLineStrings() { if (mergeStrokes) { strokeMerge(); + degreeTwoMerge(); } if (minLength > 0) { @@ -380,7 +385,7 @@ public List getMergedLineStrings() { return result; } - private double length(List edge) { + private static double length(List edge) { Coordinate last = null; double length = 0; for (Coordinate coord : edge) { @@ -464,7 +469,7 @@ private class Node { final List edge = new ArrayList<>(); Coordinate coordinate; - public Node(Coordinate coordinate) { + Node(Coordinate coordinate) { this.coordinate = coordinate; } @@ -490,7 +495,7 @@ public String toString() { return "Node{" + id + ": " + edge + '}'; } - public double distance(Node end) { + double distance(Node end) { return coordinate.distance(end.coordinate); } } @@ -514,7 +519,7 @@ private Edge(Node from, Node to, List coordinateSequence, double len edges++; } - public Edge(int id, Node from, Node to, double length, List coordinates, boolean main, Edge reversed) { + private Edge(int id, Node from, Node to, double length, List coordinates, boolean main, Edge reversed) { this.id = id; this.from = from; this.to = to; @@ -524,7 +529,7 @@ public Edge(int id, Node from, Node to, double length, List coordina this.reversed = reversed; } - public void remove() { + void remove() { if (!removed) { from.removeEdge(this); to.removeEdge(reversed); @@ -546,11 +551,13 @@ public void remove() { return length; } - public void simplify() { + /** Returns {@code false} if simplifying this edge collapsed it to {@code length=0}. */ + boolean simplify() { coordinates = DouglasPeuckerSimplifier.simplify(coordinates, tolerance, false); if (reversed != null) { reversed.coordinates = coordinates.reversed(); } + return coordinates.size() != 2 || !coordinates.getFirst().equals(coordinates.getLast()); } @Override diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/util/LoopLineMergerTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/util/LoopLineMergerTest.java index 59ab8ca57a..07c6886d01 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/util/LoopLineMergerTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/util/LoopLineMergerTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.params.provider.CsvSource; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.LineString; import org.locationtech.jts.io.ParseException; import org.locationtech.jts.io.WKBReader; import org.locationtech.jts.operation.linemerge.LineMerger; @@ -168,12 +169,12 @@ void testRemoveSelfClosingLoops() { .setLoopMinLength(-1); merger.add(newLineString( - 1, -10, - 1, 1, - 1, 2, - 0, 2, - 0, 1, - 1, 1, + 1, -10, + 1, 1, + 1, 2, + 0, 2, + 0, 1, + 1, 1, 10, 1)); assertEquals( List.of(newLineString(1, -10, 1, 1, 10, 1)), @@ -263,13 +264,14 @@ void testRemovesShortStubsTheNonStubsThatAreTooShort() { void testMergeCarriagewaysWithOneSplitShorterThanLoopMinLength() { var merger = new LoopLineMerger() .setMinLength(20) + .setMergeStrokes(true) .setLoopMinLength(20); merger.add(newLineString(0, 0, 10, 0, 20, 0, 30, 0)); merger.add(newLineString(30, 0, 20, 0, 15, 1, 10, 0, 0, 0)); assertEquals( - List.of(newLineString(30, 0, 20, 0, 10, 0, 0, 0)), + List.of(newLineString(0, 0, 10, 0, 20, 0, 30, 0)), merger.getMergedLineStrings() ); } @@ -278,6 +280,7 @@ void testMergeCarriagewaysWithOneSplitShorterThanLoopMinLength() { void testMergeCarriagewaysWithOneSplitLongerThanLoopMinLength() { var merger = new LoopLineMerger() .setMinLength(5) + .setMergeStrokes(true) .setLoopMinLength(5); merger.add(newLineString(0, 0, 10, 0, 20, 0, 30, 0)); @@ -285,7 +288,7 @@ void testMergeCarriagewaysWithOneSplitLongerThanLoopMinLength() { assertEquals( // ideally loop merging should connect long line strings and represent loops as separate segments off of the edges - List.of(newLineString(30, 0, 20, 0, 10, 0, 0, 0), newLineString(20, 0, 15, 1, 10, 0)), + List.of(newLineString(0, 0, 10, 0, 20, 0, 30, 0), newLineString(20, 0, 15, 1, 10, 0)), merger.getMergedLineStrings() ); } @@ -294,13 +297,14 @@ void testMergeCarriagewaysWithOneSplitLongerThanLoopMinLength() { void testMergeCarriagewaysWithTwoSplits() { var merger = new LoopLineMerger() .setMinLength(20) + .setMergeStrokes(true) .setLoopMinLength(20); merger.add(newLineString(0, 0, 10, 0, 20, 0, 30, 0, 40, 0)); merger.add(newLineString(40, 0, 30, 0, 25, 5, 20, 0, 15, 5, 10, 0, 0, 0)); assertEquals( - List.of(newLineString(40, 0, 30, 0, 20, 0, 10, 0, 0, 0)), + List.of(newLineString(0, 0, 10, 0, 20, 0, 30, 0, 40, 0)), merger.getMergedLineStrings() ); } @@ -354,23 +358,26 @@ void testRealWorldHarkingen() { @ParameterizedTest @CsvSource({ - "mergelines_1759_point_line.wkb.gz,0,4", - "mergelines_1759_point_line.wkb.gz,1,2", - - "mergelines_200433_lines.wkb.gz,0,35249", - "mergelines_200433_lines.wkb.gz,0.1,23034", - "mergelines_200433_lines.wkb.gz,1,1499", - - "mergelines_239823_lines.wkb.gz,0,19656", - "mergelines_239823_lines.wkb.gz,0.1,13851", - "mergelines_239823_lines.wkb.gz,1,1593", - - "i90.wkb.gz,0,95", - "i90.wkb.gz,1,65", - "i90.wkb.gz,20,4", - "i90.wkb.gz,30,1", + "mergelines_1759_point_line.wkb.gz,0,false,3", + "mergelines_1759_point_line.wkb.gz,1,false,2", + "mergelines_1759_point_line.wkb.gz,1,true,2", + + "mergelines_200433_lines.wkb.gz,0,false,9103", + "mergelines_200433_lines.wkb.gz,0.1,false,8834", + "mergelines_200433_lines.wkb.gz,1,false,878", + "mergelines_200433_lines.wkb.gz,1,true,527", + + "mergelines_239823_lines.wkb.gz,0,false,6188", + "mergelines_239823_lines.wkb.gz,0.1,false,5941", + "mergelines_239823_lines.wkb.gz,1,false,832", + "mergelines_239823_lines.wkb.gz,1,true,688", + + "i90.wkb.gz,0,false,17", + "i90.wkb.gz,1,false,18", + "i90.wkb.gz,20,false,4", + "i90.wkb.gz,30,false,1", }) - void testOnRealWorldData(String file, double minLengths, int expected) + void testOnRealWorldData(String file, double minLengths, boolean simplify, int expected) throws IOException, ParseException { Geometry geom = new WKBReader(GeoUtils.JTS_FACTORY).read( Gzip.gunzip(Files.readAllBytes(TestUtils.pathToResource("mergelines").resolve(file)))); @@ -379,6 +386,7 @@ void testOnRealWorldData(String file, double minLengths, int expected) merger.setLoopMinLength(minLengths); merger.setStubMinLength(minLengths); merger.setMergeStrokes(true); + merger.setTolerance(simplify ? 1 : -1); merger.add(geom); var merged = merger.getMergedLineStrings(); Set> lines = new HashSet<>(); @@ -386,11 +394,17 @@ void testOnRealWorldData(String file, double minLengths, int expected) for (var line : merged) { merger2.add(line); assertTrue(lines.add(Arrays.asList(line.getCoordinates())), "contained duplicate: " + line); - if (minLengths > 0) { + if (minLengths > 0 && !simplify) { // simplification can make an edge < min length assertTrue(line.getLength() >= minLengths, "line < " + minLengths + ": " + line); } } // ensure there are no more opportunities for simplification found by JTS: + List loop = List.copyOf(merged); + List jts = merger2.getMergedLineStrings().stream().map(LineString.class::cast).toList(); + List missing = jts.stream().filter(l -> !loop.contains(l)).toList(); + List extra = loop.stream().filter(l -> !jts.contains(l)).toList(); + assertEquals(List.of(), missing, "missing edges"); + assertEquals(List.of(), extra, "extra edges"); assertEquals(merged.size(), merger2.getMergedLineStrings().size()); assertEquals(expected, merged.size()); }