Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add detailed jts debugging info #703

Merged
merged 6 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.locationtech.jts.geom.LinearRing;
import org.locationtech.jts.geom.Polygon;
import org.locationtech.jts.geom.Polygonal;
import org.locationtech.jts.geom.TopologyException;
import org.locationtech.jts.index.strtree.STRtree;
import org.locationtech.jts.operation.buffer.BufferOp;
import org.locationtech.jts.operation.buffer.BufferParameters;
Expand Down Expand Up @@ -410,7 +411,7 @@ public static Collection<List<VectorTile.Feature>> groupByAttrs(
* Merges nearby polygons by expanding each individual polygon by {@code buffer}, unioning them, and contracting the
* result.
*/
private static Geometry bufferUnionUnbuffer(double buffer, List<Geometry> polygonGroup) {
private static Geometry bufferUnionUnbuffer(double buffer, List<Geometry> polygonGroup) throws GeometryException {
/*
* A simpler alternative that might initially appear faster would be:
*
Expand All @@ -424,11 +425,19 @@ private static Geometry bufferUnionUnbuffer(double buffer, List<Geometry> polygo
* The following approach is slower most of the time, but faster on average because it does
* not choke on dense nearby polygons:
*/
for (int i = 0; i < polygonGroup.size(); i++) {
polygonGroup.set(i, buffer(buffer, polygonGroup.get(i)));
List<Geometry> buffered = new ArrayList<>(polygonGroup.size());
for (Geometry geometry : polygonGroup) {
buffered.add(buffer(buffer, geometry));
}
Geometry merged = GeoUtils.createGeometryCollection(buffered);
try {
merged = union(merged);
} catch (TopologyException e) {
throw new GeometryException("buffer_union_failure", "Error unioning buffered polygons", e)
.addGeometryDetails("original", GeoUtils.createGeometryCollection(polygonGroup))
.addDetails(() -> "buffer: " + buffer)
.addGeometryDetails("buffered", GeoUtils.createGeometryCollection(buffered));
}
Geometry merged = GeoUtils.createGeometryCollection(polygonGroup);
merged = union(merged);
merged = unbuffer(buffer, merged);
return merged;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ private void postProcessAndAddLayerFeatures(VectorTile encoder, String layer,
// log failures, only throwing when it's a fatal error
if (e instanceof GeometryException geoe) {
geoe.log(stats, "postprocess_layer",
"Caught error postprocessing features for " + layer + " layer on " + tileCoord);
"Caught error postprocessing features for " + layer + " layer on " + tileCoord, config.logJtsExceptions());
} else if (e instanceof Error err) {
LOGGER.error("Caught fatal error postprocessing features {} {}", layer, tileCoord, e);
throw err;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ public record PlanetilerConfig(
String debugUrlPattern,
Path tmpDir,
Path tileWeights,
double maxPointBuffer
double maxPointBuffer,
boolean logJtsExceptions
) {

public static final int MIN_MINZOOM = 0;
Expand Down Expand Up @@ -208,7 +209,8 @@ public static PlanetilerConfig from(Arguments arguments) {
"Max tile pixels to include points outside tile bounds. Set to a lower value to reduce tile size for " +
"clients that handle label collisions across tiles (most web and native clients). NOTE: Do not reduce if you need to support " +
"raster tile rendering",
Double.POSITIVE_INFINITY)
Double.POSITIVE_INFINITY),
arguments.getBoolean("log_jts_exceptions", "Emit verbose details to debug JTS geometry errors", false)
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
package com.onthegomap.planetiler.geo;

import com.onthegomap.planetiler.stats.Stats;
import java.util.ArrayList;
import java.util.Base64;
import java.util.function.Supplier;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.io.WKBWriter;
import org.locationtech.jts.io.WKTWriter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -14,6 +20,7 @@ public class GeometryException extends Exception {

private final String stat;
private final boolean nonFatal;
private final ArrayList<Supplier<String>> detailsSuppliers = new ArrayList<>();

/**
* Constructs a new exception with a detailed error message caused by {@code cause}.
Expand Down Expand Up @@ -51,6 +58,11 @@ public GeometryException(String stat, String message, boolean nonFatal) {
this.nonFatal = nonFatal;
}

public GeometryException addDetails(Supplier<String> detailsSupplier) {
this.detailsSuppliers.add(detailsSupplier);
return this;
}

/** Returns the unique code for this error condition to use for counting the number of occurrences in stats. */
public String stat() {
return stat;
Expand All @@ -72,6 +84,38 @@ void logMessage(String log) {
assert nonFatal : log; // make unit tests fail if fatal
}


/** Logs the error but if {@code logDetails} is true, then also prints detailed debugging info. */
public void log(Stats stats, String statPrefix, String logPrefix, boolean logDetails) {
if (logDetails) {
stats.dataError(statPrefix + "_" + stat());
StringBuilder log = new StringBuilder(logPrefix + ": " + getMessage());
for (var details : detailsSuppliers) {
log.append("\n").append(details.get());
}
var str = log.toString();
LOGGER.warn(str, this.getCause() == null ? this : this.getCause());
assert nonFatal : log.toString(); // make unit tests fail if fatal
} else {
log(stats, statPrefix, logPrefix);
}
}

public GeometryException addGeometryDetails(String original, Geometry geometryCollection) {
return addDetails(() -> {
var wktWriter = new WKTWriter();
var wkbWriter = new WKBWriter();
var base64 = Base64.getEncoder();
return """
%s (wkt): %s
%s (wkb): %s
""".formatted(
original, wktWriter.write(geometryCollection),
original, base64.encodeToString(wkbWriter.write(geometryCollection))
).strip();
});
}

/**
* An error that we expect to encounter often so should only be logged at {@code TRACE} level.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import com.onthegomap.planetiler.stats.Stats;
import java.util.List;
import org.geotools.geometry.jts.WKTReader2;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
Expand All @@ -18,6 +17,7 @@
import org.locationtech.jts.geom.Point;
import org.locationtech.jts.geom.util.AffineTransformation;
import org.locationtech.jts.io.ParseException;
import org.locationtech.jts.io.WKTReader;

class GeoUtilsTest {

Expand Down Expand Up @@ -367,7 +367,7 @@ void testCombineNested() {

@Test
void testSnapAndFixIssue511() throws ParseException, GeometryException {
var result = GeoUtils.snapAndFixPolygon(new WKTReader2().read(
var result = GeoUtils.snapAndFixPolygon(new WKTReader().read(
"""
MULTIPOLYGON (((198.83750000000003 46.07500000000004, 199.0625 46.375, 199.4375 46.0625, 199.5 46.43750000000001, 199.5625 46, 199.3125 45.5, 198.8912037037037 46.101851851851876, 198.83750000000003 46.07500000000004)), ((198.43750000000003 46.49999999999999, 198.5625 46.43750000000001, 198.6875 46.25, 198.1875 46.25, 198.43750000000003 46.49999999999999)), ((198.6875 46.25, 198.81249999999997 46.062500000000014, 198.6875 46.00000000000002, 198.6875 46.25)), ((196.55199579831933 46.29359243697479, 196.52255639097743 46.941259398496236, 196.5225563909774 46.941259398496236, 196.49999999999997 47.43750000000001, 196.875 47.125, 197 47.5625, 197.47880544905414 46.97729334004497, 197.51505401161464 46.998359569801956, 197.25 47.6875, 198.0625 47.6875, 198.5 46.625, 198.34375 46.546875, 198.34375000000003 46.54687499999999, 197.875 46.3125, 197.875 46.25, 197.875 46.0625, 197.82894736842107 46.20065789473683, 197.25 46.56250000000001, 197.3125 46.125, 196.9375 46.1875, 196.9375 46.21527777777778, 196.73250000000002 46.26083333333334, 196.5625 46.0625, 196.55199579831933 46.29359243697479)), ((196.35213414634146 45.8170731707317, 197.3402027027027 45.93108108108108, 197.875 45.99278846153846, 197.875 45.93750000000002, 197.93749999999997 45.99999999999999, 197.9375 46, 197.90625 45.96874999999999, 197.90625 45.96875, 196.75000000000006 44.81250000000007, 197.1875 45.4375, 196.3125 45.8125, 196.35213414634146 45.8170731707317)), ((195.875 46.124999999999986, 195.8125 46.5625, 196.5 46.31250000000001, 195.9375 46.4375, 195.875 46.124999999999986)), ((196.49999999999997 46.93749999999999, 196.125 46.875, 196.3125 47.125, 196.49999999999997 46.93749999999999)))
"""),
Expand All @@ -377,7 +377,7 @@ void testSnapAndFixIssue511() throws ParseException, GeometryException {

@Test
void testSnapAndFixIssue546() throws GeometryException, ParseException {
var orig = new WKTReader2().read(
var orig = new WKTReader().read(
"""
POLYGON(
(
Expand All @@ -404,7 +404,7 @@ void testSnapAndFixIssue546() throws GeometryException, ParseException {

@Test
void testSnapAndFixIssue546_2() throws GeometryException, ParseException {
var orig = new WKTReader2().read(
var orig = new WKTReader().read(
"""
POLYGON(
(
Expand All @@ -423,7 +423,7 @@ void testSnapAndFixIssue546_2() throws GeometryException, ParseException {

@Test
void testSnapAndFixIssue546_3() throws GeometryException, ParseException {
var orig = new WKTReader2().read(
var orig = new WKTReader().read(
"""
POLYGON(
(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
import java.util.TreeSet;
import java.util.stream.Stream;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.geotools.geometry.jts.WKTReader2;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.io.ParseException;
import org.locationtech.jts.io.WKTReader;
import org.snakeyaml.engine.v2.exceptions.YamlEngineException;

/** Verifies that a profile maps input elements map to expected output vector tile features. */
Expand Down Expand Up @@ -164,7 +164,7 @@ private static Geometry parseGeometry(String geometry) {
default -> geometry;
};
try {
return new WKTReader2().read(wkt);
return new WKTReader().read(wkt);
} catch (ParseException e) {
throw new IllegalArgumentException("""
Bad geometry: "%s", must be "point" "line" "polygon" or a valid WKT string.
Expand Down
Loading