Skip to content

Commit

Permalink
fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@msbarry
msbarry committed Nov 24, 2024
1 parent 62c869a commit 12987ca
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 37 deletions.
29 changes: 18 additions & 11 deletions planetiler-core/src/main/java/com/onthegomap/planetiler/util/LoopLineMerger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
* Merging in Planetiler</a>
*/
public class LoopLineMerger {
final List<LineString> input = new ArrayList<>();
private final List<LineString> input = new ArrayList<>();
private final List<Node> 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;
Expand Down Expand Up @@ -304,13 +304,17 @@ private void removeShortEdges() {
}

private void simplify() {
List<Edge> 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() {
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -361,6 +365,7 @@ public List<LineString> getMergedLineStrings() {

if (mergeStrokes) {
strokeMerge();
degreeTwoMerge();
}

if (minLength > 0) {
Expand All @@ -380,7 +385,7 @@ public List<LineString> getMergedLineStrings() {
return result;
}

private double length(List<Coordinate> edge) {
private static double length(List<Coordinate> edge) {
Coordinate last = null;
double length = 0;
for (Coordinate coord : edge) {
Expand Down Expand Up @@ -464,7 +469,7 @@ private class Node {
final List<Edge> edge = new ArrayList<>();
Coordinate coordinate;

public Node(Coordinate coordinate) {
Node(Coordinate coordinate) {
this.coordinate = coordinate;
}

Expand All @@ -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);
}
}
Expand All @@ -514,7 +519,7 @@ private Edge(Node from, Node to, List<Coordinate> coordinateSequence, double len
edges++;
}

public Edge(int id, Node from, Node to, double length, List<Coordinate> coordinates, boolean main, Edge reversed) {
private Edge(int id, Node from, Node to, double length, List<Coordinate> coordinates, boolean main, Edge reversed) {
this.id = id;
this.from = from;
this.to = to;
Expand All @@ -524,7 +529,7 @@ public Edge(int id, Node from, Node to, double length, List<Coordinate> coordina
this.reversed = reversed;
}

public void remove() {
void remove() {
if (!removed) {
from.removeEdge(this);
to.removeEdge(reversed);
Expand All @@ -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
Expand Down
66 changes: 40 additions & 26 deletions planetiler-core/src/test/java/com/onthegomap/planetiler/util/LoopLineMergerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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()
);
}
Expand All @@ -278,14 +280,15 @@ 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));
merger.add(newLineString(30, 0, 20, 0, 15, 1, 10, 0, 0, 0));

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()
);
}
Expand All @@ -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()
);
}
Expand Down Expand Up @@ -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))));
Expand All @@ -379,18 +386,25 @@ 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<List<Coordinate>> lines = new HashSet<>();
var merger2 = new LineMerger();
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<LineString> loop = List.copyOf(merged);
List<LineString> jts = merger2.getMergedLineStrings().stream().map(LineString.class::cast).toList();
List<LineString> missing = jts.stream().filter(l -> !loop.contains(l)).toList();
List<LineString> 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());
}
Expand Down

0 comments on commit 12987ca

Please sign in to comment.