Skip to content

Commit

Permalink
add detailed jts debugging info
Browse files Browse the repository at this point in the history
  • Loading branch information
msbarry committed Oct 30, 2023
1 parent 9f96002 commit 1b6618c
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import org.geotools.geometry.jts.WKTWriter2;
import org.locationtech.jts.algorithm.Area;
import org.locationtech.jts.geom.CoordinateSequence;
import org.locationtech.jts.geom.Envelope;
Expand All @@ -28,6 +29,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 +412,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 +426,27 @@ 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).addDetails(() -> {
var wktWriter = new WKTWriter2();
return """
Original polygons: %s
Buffer: %f
Buffered: %s
""".formatted(
wktWriter.write(GeoUtils.createGeometryCollection(polygonGroup)),
buffer,
wktWriter.write(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,7 @@
package com.onthegomap.planetiler.geo;

import com.onthegomap.planetiler.stats.Stats;
import java.util.function.Supplier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

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

private final String stat;
private final boolean nonFatal;
private Supplier<String> detailsSupplier;

Check warning on line 18 in planetiler-core/src/main/java/com/onthegomap/planetiler/geo/GeometryException.java

View workflow job for this annotation

GitHub Actions / Analyze with Sonar

CRITICAL CODE_SMELL

Make "detailsSupplier" transient or serializable. rule: java:S1948 (https://sonarcloud.io/organizations/onthegomap/rules?open=java%3AS1948&rule_key=java%3AS1948) issue url: https://sonarcloud.io/project/issues?pullRequest=703&open=AYuAhza-5PpBbDgOeDmr&id=onthegomap_planetiler

Check warning on line 18 in planetiler-core/src/main/java/com/onthegomap/planetiler/geo/GeometryException.java

View workflow job for this annotation

GitHub Actions / Analyze with Sonar

MINOR CODE_SMELL

Make this "detailsSupplier" field final. rule: java:S1165 (https://sonarcloud.io/organizations/onthegomap/rules?open=java%3AS1165&rule_key=java%3AS1165) issue url: https://sonarcloud.io/project/issues?pullRequest=703&open=AYuAhza-5PpBbDgOeDmq&id=onthegomap_planetiler

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

public GeometryException addDetails(Supplier<String> detailsSupplier) {
this.detailsSupplier = 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 +79,17 @@ 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 && detailsSupplier != null) {
stats.dataError(statPrefix + "_" + stat());
logMessage(logPrefix + ": " + getMessage() + "\n" + detailsSupplier.get());
} else {
log(stats, statPrefix, logPrefix);
}
}

/**
* An error that we expect to encounter often so should only be logged at {@code TRACE} level.
*/
Expand Down

0 comments on commit 1b6618c

Please sign in to comment.