Skip to content

Commit

Permalink
add some tests and fix loop merging
Browse files Browse the repository at this point in the history
  • Loading branch information
@msbarry
msbarry committed Nov 8, 2024
1 parent 8128713 commit 2fa8dc5
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 160 deletions.
7 changes: 3 additions & 4 deletions planetiler-core/src/main/java/com/onthegomap/planetiler/FeatureMerge.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.HashMap;
import java.util.function.Function;
import org.locationtech.jts.algorithm.Area;
import org.locationtech.jts.geom.CoordinateSequence;
Expand All @@ -36,7 +35,6 @@
import org.locationtech.jts.operation.buffer.BufferOp;
import org.locationtech.jts.operation.buffer.BufferParameters;
import org.locationtech.jts.operation.linemerge.LineMerger;
// import org.locationtech.jts.operation.linemerge.LineMerger;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -164,7 +162,8 @@ public static List<VectorTile.Feature> mergeLineStrings(List<VectorTile.Feature>
* except with a dynamic length limit computed by {@code lengthLimitCalculator} for the attributes of each group.
*/
public static List<VectorTile.Feature> mergeLineStrings(List<VectorTile.Feature> features,
Function<Map<String, Object>, Double> lengthLimitCalculator, double tolerance, double buffer, boolean resimplify, double loopMinLength) {
Function<Map<String, Object>, Double> lengthLimitCalculator, double tolerance, double buffer, boolean resimplify,
double loopMinLength) {
List<VectorTile.Feature> result = new ArrayList<>(features.size());
var groupedByAttrs = groupByAttrs(features, result, GeometryType.LINE);
for (List<VectorTile.Feature> groupedFeatures : groupedByAttrs) {
Expand All @@ -181,7 +180,7 @@ public static List<VectorTile.Feature> mergeLineStrings(List<VectorTile.Feature>
} else {
LoopLineMerger merger = new LoopLineMerger()
.setMinLength(lengthLimit)
.setLoopMinLength(loopMinLength);
.setLoopMinLength(lengthLimit);
for (VectorTile.Feature feature : groupedFeatures) {
try {
merger.add(feature.geometry().decode());
Expand Down
12 changes: 8 additions & 4 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 @@ -18,10 +18,11 @@

class RecursionDepthException extends Exception {
public RecursionDepthException(String message) {
super(message);
super(message);
}
}


public class LoopLineMerger {
private final List<LineString> input = new ArrayList<>();
private final List<Node> output = new ArrayList<>();
Expand Down Expand Up @@ -89,7 +90,9 @@ private void merge() {
a.to.removeEdge(a.reversed);
b.to.removeEdge(b.reversed);
a.to.addEdge(c);
b.to.addEdge(c.reversed);
if (a.to != b.to) {
b.to.addEdge(c.reversed);
}
}
}
}
Expand Down Expand Up @@ -121,7 +124,7 @@ private void removeLoops() {
// MAX_RECURSION_DEPTH was reached. On other edges of the graph the same
// will happen. Abort merging loops...
return;
}
}
}
}
}
Expand Down Expand Up @@ -173,7 +176,8 @@ List<List<Edge>> findAllPaths(Node start, Node end, double maxLength) throws Rec
private void removeShortStubEdges() {
for (var node : output) {
for (var edge : List.copyOf(node.getEdges())) {
if (edge.length < minLength && (edge.from.getEdges().size() == 1 || edge.to.getEdges().size() == 1 || edge.from == edge.to)) {
if (edge.length < minLength &&
(edge.from.getEdges().size() == 1 || edge.to.getEdges().size() == 1 || edge.from == edge.to)) {
edge.remove();
}
}
Expand Down
202 changes: 50 additions & 152 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 @@ -23,86 +23,6 @@

class LoopLineMergerTest {

// @Test
// void testSplit() {
// var merger = new LoopLineMerger();
//
// // splits linestrings into linear segments
// assertEquals(
// List.of(
// newLineString(10, 10, 20, 20),
// newLineString(20, 20, 30, 30)
// ),
// merger.split(newLineString(10, 10, 20, 20, 30, 30))
// );
//
// // does not keep zero length linestrings
// assertEquals(0, merger.split(newLineString(10, 10, 10, 10)).size());
//
// // rounds coordinates to 1/16 grid
// merger = new LoopLineMerger();
// assertEquals(
// List.of(
// newLineString(10.0625, 10, 20, 20),
// newLineString(20, 20, 30, 30)
// ),
// merger.split(newLineString(10.0624390, 10, 20, 20, 30, 30))
// );
//
// // rounds coordinates to 0.25 grid
// merger = new LoopLineMerger()
// .setPrecisionModel(new PrecisionModel(-0.25));
//
// assertEquals(
// List.of(
// newLineString(10.25, 10, 20, 20),
// newLineString(20, 20, 30, 30)
// ),
// merger.split(newLineString(10.2509803497, 10, 20, 20, 30, 30))
// );
// }

// @Test
// void testConcat() {
// // concatenates forward A followed by forward B
// assertEquals(
// newLineString(10, 10, 20, 20, 30, 30),
// LoopLineMerger.concat(
// newLineString(10, 10, 20, 20),
// newLineString(20, 20, 30, 30)
// )
// );
//
// // concatenates forward B followed by forward A
// assertEquals(
// newLineString(10, 10, 20, 20, 30, 30),
// LoopLineMerger.concat(
// newLineString(20, 20, 30, 30),
// newLineString(10, 10, 20, 20)
// )
// );
//
// // concatenates A and B with same start point to backward A forward B
// assertEquals(
// newLineString(10, 10, 20, 20, 30, 30),
// LoopLineMerger.concat(
// newLineString(20, 20, 10, 10),
// newLineString(20, 20, 30, 30)
// )
// );
//
// // concatenates A and B with same end point to forward A backward B
// assertEquals(
// newLineString(10, 10, 20, 20, 30, 30),
// LoopLineMerger.concat(
// newLineString(10, 10, 20, 20),
// newLineString(30, 30, 20, 20)
// )
// );
//
// // TODO: handle and test the case when A/B do not share end/start points
// }

@Test
void testMergeTouchingLinestrings() {
var merger = new LoopLineMerger()
Expand Down Expand Up @@ -290,74 +210,52 @@ void testRemovesShortStubsTheNonStubsThatAreTooShort() {
);
}

// @Test
// void testHasPointAppearingMoreThanTwice() {
//
// // can revisit starting node once
// assertEquals(
// false,
// LoopLineMerger.hasPointAppearingMoreThanTwice(
// List.of(
// newLineString(10, 0, 20, 0),
// newLineString(20, 0, 20, 10),
// newLineString(20, 10, 10, 0)
// )
// )
// );
//
// // cannot revisit starting node and continue on
// assertEquals(
// true,
// LoopLineMerger.hasPointAppearingMoreThanTwice(
// List.of(
// newLineString(10, 0, 20, 0),
// newLineString(20, 0, 20, 10),
// newLineString(20, 10, 10, 0),
// newLineString(10, 0, 10, 10)
// )
// )
// );
// }
//
// @Test
// void testFindAllPaths() {
// // finds all paths and orders them by length
// var merger = new LoopLineMerger();
// /**
// * 10 20 30 10 o-----o |\ | | \ | 20 o--o--o
// */
// merger.add(newLineString(10, 10, 30, 10));
// merger.add(newLineString(10, 10, 10, 20));
// merger.add(newLineString(10, 10, 20, 20));
// merger.add(newLineString(10, 20, 20, 20));
// merger.add(newLineString(20, 20, 30, 20));
// merger.add(newLineString(30, 20, 30, 10));
//
// var allPaths = merger.findAllPaths(newPoint(10, 10), newPoint(20, 20), 1000);
//
// assertEquals(3, allPaths.size());
// assertEquals(
// List.of(newLineString(10, 10, 20, 20)),
// allPaths.get(0)
// );
// assertEquals(
// List.of(
// newLineString(10, 10, 10, 20),
// newLineString(10, 20, 20, 20)
// ),
// allPaths.get(1)
// );
// assertEquals(
// List.of(
// newLineString(10, 10, 30, 10),
// newLineString(30, 10, 30, 20),
// newLineString(30, 20, 20, 20)
// ),
// allPaths.get(2)
// );
// }

// @Disabled
@Test
void testMergeCarriagewaysWithOneSplitShorterThanLoopMinLength() {
var merger = new LoopLineMerger()
.setMinLength(20)
.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)),
merger.getMergedLineStrings()
);
}

@Test
void testMergeCarriagewaysWithOneSplitLongerThanLoopMinLength() {
var merger = new LoopLineMerger()
.setMinLength(5)
.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)),
merger.getMergedLineStrings()
);
}

@Test
void testMergeCarriagewaysWithTwoSplits() {
var merger = new LoopLineMerger()
.setMinLength(20)
.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)),
merger.getMergedLineStrings()
);
}

@ParameterizedTest
@CsvSource({
"mergelines_1759_point_line.wkb.gz,0,4",
Expand All @@ -371,16 +269,16 @@ void testRemovesShortStubsTheNonStubsThatAreTooShort() {
"mergelines_239823_lines.wkb.gz,0.1,14196",
"mergelines_239823_lines.wkb.gz,1,1673",

"i90.wkb.gz,0,1",
"i90.wkb.gz,1,1",
"i90.wkb.gz,20,1",
"i90.wkb.gz,0,95",
"i90.wkb.gz,1,65",
"i90.wkb.gz,20,4",
"i90.wkb.gz,30,1",
})
void testOnRealWorldData(String file, double minLengths, int expected)
throws IOException, ParseException {
Geometry geom = new WKBReader(GeoUtils.JTS_FACTORY).read(
Gzip.gunzip(Files.readAllBytes(TestUtils.pathToResource("mergelines").resolve(file))));
var merger = new LoopLineMerger();
// merger.setPrecisionModel(new PrecisionModel(16));
merger.setMinLength(minLengths);
merger.setLoopMinLength(minLengths);
merger.add(geom);
Expand Down

0 comments on commit 2fa8dc5

Please sign in to comment.