From b848f69e2ae8b9040272c9b30b383baf10488103 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Fri, 12 Jan 2024 06:32:13 -0500 Subject: [PATCH 1/5] make OSM polygon construction deterministic --- .../planetiler/collection/Hppc.java | 5 + .../planetiler/pmtiles/WriteablePmtiles.java | 5 +- .../reader/osm/OsmMultipolygon.java | 31 ++- .../planetiler/reader/osm/OsmReader.java | 2 +- .../planetiler/util/CompareArchives.java | 206 +++++++++++++----- .../planetiler/util/LayerAttrStats.java | 8 +- .../planetiler/util/CompareArchivesTest.java | 3 +- 7 files changed, 177 insertions(+), 83 deletions(-) diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/collection/Hppc.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/collection/Hppc.java index 497f1dc66c..2ab9421d31 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/collection/Hppc.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/collection/Hppc.java @@ -7,6 +7,7 @@ import com.carrotsearch.hppc.LongLongHashMap; import com.carrotsearch.hppc.LongObjectHashMap; import com.carrotsearch.hppc.ObjectIntHashMap; +import com.carrotsearch.hppc.SortedIterationLongObjectHashMap; /** * Static factory method for High Performance Primitive Collections. @@ -40,4 +41,8 @@ public static LongIntHashMap newLongIntHashMap() { public static LongByteMap newLongByteHashMap() { return new LongByteHashMap(10, 0.75); } + + public static SortedIterationLongObjectHashMap sortedView(LongObjectHashMap input) { + return new SortedIterationLongObjectHashMap<>(input, Long::compare); + } } diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/pmtiles/WriteablePmtiles.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/pmtiles/WriteablePmtiles.java index 63f42ec3bc..795a50f03b 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/pmtiles/WriteablePmtiles.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/pmtiles/WriteablePmtiles.java @@ -24,10 +24,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.LinkedHashMap; import java.util.List; import java.util.Objects; import java.util.OptionalLong; +import java.util.TreeMap; import java.util.function.LongSupplier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -147,7 +147,8 @@ public void finish(TileArchiveMetadata tileArchiveMetadata) { } try { Directories directories = makeDirectories(entries); - var otherMetadata = new LinkedHashMap<>(tileArchiveMetadata.toMap()); + // use treemap to ensure consistent ouput between runs + var otherMetadata = new TreeMap<>(tileArchiveMetadata.toMap()); // exclude keys included in top-level header otherMetadata.remove(TileArchiveMetadata.CENTER_KEY); diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java index 31ca60a356..16a110a41d 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java @@ -14,15 +14,14 @@ package com.onthegomap.planetiler.reader.osm; import com.carrotsearch.hppc.LongArrayList; -import com.carrotsearch.hppc.LongObjectMap; +import com.carrotsearch.hppc.LongObjectHashMap; import com.carrotsearch.hppc.ObjectIntMap; -import com.carrotsearch.hppc.cursors.LongObjectCursor; import com.onthegomap.planetiler.collection.Hppc; import com.onthegomap.planetiler.geo.GeoUtils; import com.onthegomap.planetiler.geo.GeometryException; import java.util.ArrayList; import java.util.Comparator; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import org.locationtech.jts.geom.Coordinate; @@ -41,8 +40,7 @@ * Multipolygon way members have an "inner" and "outer" role, but they can be incorrectly specified, so instead * determine the nesting order and alternate outer/inner/outer/inner... from the outermost ring inwards. *

- * This class is ported to Java from https://github.com/omniscale/imposm3/blob/master/geom/multipolygon.go and - * https://github.com/omniscale/imposm3/blob/master/geom/ring.go + * This class is ported to Java from ... and ... */ public class OsmMultipolygon { /* @@ -62,7 +60,8 @@ private static class Ring { private final Polygon geom; private final double area; private Ring containedBy = null; - private final Set holes = new HashSet<>(); + // use linked hash set to ensure stable output + private final Set holes = new LinkedHashSet<>(); private Ring(Polygon geom) { this.geom = geom; @@ -163,7 +162,7 @@ private static Geometry doBuild( boolean fix ) throws GeometryException { try { - if (rings.size() == 0) { + if (rings.isEmpty()) { throw new GeometryException.Verbose("osm_invalid_multipolygon_empty", "error building multipolygon " + osmId + ": no rings to process"); } @@ -175,7 +174,7 @@ private static Geometry doBuild( } polygons.sort(BY_AREA_DESCENDING); Set shells = groupParentChildShells(polygons); - if (shells.size() == 0) { + if (shells.isEmpty()) { throw new GeometryException.Verbose("osm_invalid_multipolygon_not_closed", "error building multipolygon " + osmId + ": multipolygon not closed"); } else if (shells.size() == 1) { @@ -227,7 +226,8 @@ private static void addPolygonRings(List polygons, Geometry geom) { } private static Set groupParentChildShells(List polygons) { - Set shells = new HashSet<>(); + // use linked hash sate to ensure the same input always produces the same output + Set shells = new LinkedHashSet<>(); int numPolygons = polygons.size(); if (numPolygons == 0) { return shells; @@ -313,7 +313,7 @@ static LongArrayList prependToSkipLast(LongArrayList orig, LongArrayList toPrepe } static List connectPolygonSegments(List outer) { - LongObjectMap endpointIndex = Hppc.newLongObjectHashMap(outer.size() * 2); + LongObjectHashMap endpointIndex = Hppc.newLongObjectHashMap(outer.size() * 2); List completeRings = new ArrayList<>(outer.size()); for (LongArrayList ids : outer) { @@ -366,12 +366,11 @@ static List connectPolygonSegments(List outer) { } } - for (LongObjectCursor cursor : endpointIndex) { - LongArrayList value = cursor.value; - if (value.size() >= 4) { - if (value.get(0) == value.get(value.size() - 1) || cursor.key == value.get(0)) { - completeRings.add(value); - } + // iterate in sorted order to ensure the same input always produces the same output + for (var entry : Hppc.sortedView(endpointIndex)) { + LongArrayList value = entry.value; + if (value.size() >= 4 && (value.get(0) == value.get(value.size() - 1) || entry.key == value.get(0))) { + completeRings.add(value); } } return completeRings; diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmReader.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmReader.java index dbd3a97fbe..7a9789feea 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmReader.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmReader.java @@ -592,7 +592,7 @@ default CoordinateSequence getWayGeometry(LongArrayList nodeIds) { * @param role "role" of the relation member * @param relation user-provided data about the relation from pass1 */ - public record RelationMember (String role, T relation) {} + public record RelationMember(String role, T relation) {} /** Raw relation membership data that gets encoded/decoded into a long. */ private record RelationMembership(String role, long relationId) {} diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/CompareArchives.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/CompareArchives.java index e84384d7fc..7a99c6b933 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/CompareArchives.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/CompareArchives.java @@ -1,11 +1,17 @@ package com.onthegomap.planetiler.util; +import com.google.common.primitives.Ints; +import com.onthegomap.planetiler.VectorTile; import com.onthegomap.planetiler.archive.Tile; import com.onthegomap.planetiler.archive.TileArchiveConfig; import com.onthegomap.planetiler.archive.TileArchives; import com.onthegomap.planetiler.archive.TileCompression; import com.onthegomap.planetiler.config.Arguments; import com.onthegomap.planetiler.config.PlanetilerConfig; +import com.onthegomap.planetiler.geo.GeometryException; +import com.onthegomap.planetiler.geo.GeometryType; +import com.onthegomap.planetiler.geo.TileCoord; +import com.onthegomap.planetiler.pmtiles.ReadablePmtiles; import com.onthegomap.planetiler.stats.ProgressLoggers; import com.onthegomap.planetiler.worker.WorkerPipeline; import java.io.IOException; @@ -18,6 +24,9 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Supplier; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.MultiPolygon; +import org.locationtech.jts.geom.Polygon; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import vector_tile.VectorTileProto; @@ -38,15 +47,17 @@ public class CompareArchives { private final Map> diffsByLayer = new ConcurrentHashMap<>(); private final TileArchiveConfig input1; private final TileArchiveConfig input2; + private final boolean verbose; - private CompareArchives(TileArchiveConfig archiveConfig1, TileArchiveConfig archiveConfig2) { + private CompareArchives(TileArchiveConfig archiveConfig1, TileArchiveConfig archiveConfig2, boolean verbose) { + this.verbose = verbose; this.input1 = archiveConfig1; this.input2 = archiveConfig2; } public static Result compare(TileArchiveConfig archiveConfig1, TileArchiveConfig archiveConfig2, - PlanetilerConfig config) { - return new CompareArchives(archiveConfig1, archiveConfig2).getResult(config); + PlanetilerConfig config, boolean verbose) { + return new CompareArchives(archiveConfig1, archiveConfig2, verbose).getResult(config); } public static void main(String[] args) { @@ -58,12 +69,13 @@ public static void main(String[] args) { String inputString1 = args[args.length - 2]; String inputString2 = args[args.length - 1]; var arguments = Arguments.fromArgsOrConfigFile(Arrays.copyOf(args, args.length - 2)); + var verbose = arguments.getBoolean("verbose", "log each tile diff", false); var config = PlanetilerConfig.from(arguments); var input1 = TileArchiveConfig.from(inputString1); var input2 = TileArchiveConfig.from(inputString2); try { - var result = compare(input1, input2, config); + var result = compare(input1, input2, config, verbose); var format = Format.defaultInstance(); if (LOGGER.isInfoEnabled()) { @@ -88,12 +100,11 @@ public static void main(String[] args) { } private Result getResult(PlanetilerConfig config) { + final TileCompression compression2; + final TileCompression compression1; if (!input1.format().equals(input2.format())) { - throw new IllegalArgumentException( - "input1 and input2 must have the same format, got " + input1.format() + " and " + - input2.format()); + LOGGER.warn("archive1 and archive2 have different formats, got {} and {}", input1.format(), input2.format()); } - final TileCompression compression; try ( var reader1 = TileArchives.newReader(input1, config); var reader2 = TileArchives.newReader(input2, config); @@ -107,12 +118,23 @@ private Result getResult(PlanetilerConfig config) { archive2: {} """, reader1.metadata(), reader2.metadata()); } - compression = metadata1 == null ? TileCompression.UNKNOWN : metadata1.tileCompression(); - TileCompression compression2 = metadata2 == null ? TileCompression.UNKNOWN : metadata2.tileCompression(); - if (compression != compression2) { - throw new IllegalArgumentException( - "input1 and input2 must have the same compression, got " + compression + " and " + - compression2); + if (reader1 instanceof ReadablePmtiles pmt1 && reader2 instanceof ReadablePmtiles pmt2) { + var header1 = pmt1.getHeader(); + var header2 = pmt2.getHeader(); + if (!Objects.equals(header1, header2)) { + LOGGER.warn(""" + archive1 and archive2 have different pmtiles headers + archive1: {} + archive2: {} + """, header1, header2); + } + } + compression1 = metadata1 == null ? TileCompression.UNKNOWN : metadata1.tileCompression(); + compression2 = metadata2 == null ? TileCompression.UNKNOWN : metadata2.tileCompression(); + if (compression1 != compression2) { + LOGGER.warn( + "input1 and input2 must have the same compression, got {} and {} - will compare decompressed tile contents instead", + compression1, compression2); } } catch (IOException e) { throw new UncheckedIOException(e); @@ -160,22 +182,39 @@ record Diff(Tile a, Tile b) {} }) .addBuffer("diffs", 50_000, 1_000) .sinkTo("process", config.featureProcessThreads(), prev -> { + boolean sameCompression = compression1 == compression2; for (var diff : prev) { var a = diff.a(); var b = diff.b(); total.incrementAndGet(); if (a == null) { - recordTileDiff("archive 1 missing tile"); + recordTileDiff(b.coord(), "archive 1 missing tile"); diffs.incrementAndGet(); } else if (b == null) { - recordTileDiff("archive 2 missing tile"); - diffs.incrementAndGet(); - } else if (!Arrays.equals(a.bytes(), b.bytes())) { - recordTileDiff("different contents"); + recordTileDiff(a.coord(), "archive 2 missing tile"); diffs.incrementAndGet(); - var proto1 = decode(a.bytes(), compression); - var proto2 = decode(b.bytes(), compression); - compareTiles(proto1, proto2); + } else if (sameCompression) { + if (!Arrays.equals(a.bytes(), b.bytes())) { + recordTileDiff(a.coord(), "different contents"); + diffs.incrementAndGet(); + compareTiles( + a.coord(), + decode(decompress(a.bytes(), compression1)), + decode(decompress(b.bytes(), compression2)) + ); + } + } else { // different compression + var decompressed1 = decompress(a.bytes(), compression1); + var decompressed2 = decompress(b.bytes(), compression2); + if (!Arrays.equals(decompressed1, decompressed2)) { + recordTileDiff(a.coord(), "different decompressed contents"); + diffs.incrementAndGet(); + compareTiles( + a.coord(), + decode(decompressed1), + decode(decompressed2) + ); + } } } }); @@ -189,104 +228,157 @@ record Diff(Tile a, Tile b) {} return new Result(total.get(), diffs.get(), diffTypes, diffsByLayer); } - private void compareTiles(VectorTileProto.Tile proto1, VectorTileProto.Tile proto2) { - compareLayerNames(proto1, proto2); + private void compareTiles(TileCoord coord, VectorTileProto.Tile proto1, VectorTileProto.Tile proto2) { + compareLayerNames(coord, proto1, proto2); for (int i = 0; i < proto1.getLayersCount() && i < proto2.getLayersCount(); i++) { var layer1 = proto1.getLayers(i); var layer2 = proto2.getLayers(i); - compareLayer(layer1, layer2); + compareLayer(coord, layer1, layer2); } } - private void compareLayer(VectorTileProto.Tile.Layer layer1, VectorTileProto.Tile.Layer layer2) { + private void compareLayer(TileCoord coord, VectorTileProto.Tile.Layer layer1, VectorTileProto.Tile.Layer layer2) { String name = layer1.getName(); - compareValues(name, "version", layer1.getVersion(), layer2.getVersion()); - compareValues(name, "extent", layer1.getExtent(), layer2.getExtent()); - compareList(name, "keys list", layer1.getKeysList(), layer2.getKeysList()); - compareList(name, "values list", layer1.getValuesList(), layer2.getValuesList()); - if (compareValues(name, "features count", layer1.getFeaturesCount(), layer2.getFeaturesCount())) { + compareValues(coord, name, "version", layer1.getVersion(), layer2.getVersion()); + compareValues(coord, name, "extent", layer1.getExtent(), layer2.getExtent()); + compareList(coord, name, "keys list", layer1.getKeysList(), layer2.getKeysList()); + compareList(coord, name, "values list", layer1.getValuesList(), layer2.getValuesList()); + if (compareValues(coord, name, "features count", layer1.getFeaturesCount(), layer2.getFeaturesCount())) { var ids1 = layer1.getFeaturesList().stream().map(f -> f.getId()).toList(); var ids2 = layer2.getFeaturesList().stream().map(f -> f.getId()).toList(); - if (compareValues(name, "feature ids", Set.of(ids1), Set.of(ids2)) && - compareValues(name, "feature order", ids1, ids2)) { + if (compareValues(coord, name, "feature ids", Set.of(ids1), Set.of(ids2)) && + compareValues(coord, name, "feature order", ids1, ids2)) { for (int i = 0; i < layer1.getFeaturesCount() && i < layer2.getFeaturesCount(); i++) { var feature1 = layer1.getFeatures(i); var feature2 = layer2.getFeatures(i); - compareFeature(name, feature1, feature2); + compareFeature(coord, name, feature1, feature2); } } } } - private void compareFeature(String layer, VectorTileProto.Tile.Feature feature1, + private void compareFeature(TileCoord coord, String layer, VectorTileProto.Tile.Feature feature1, + VectorTileProto.Tile.Feature feature2) { + compareValues(coord, layer, "feature id", feature1.getId(), feature2.getId()); + compareGeometry(coord, layer, feature1, feature2); + compareValues(coord, layer, "feature tags", feature1.getTagsCount(), feature2.getTagsCount()); + } + + private void compareGeometry(TileCoord coord, String layer, VectorTileProto.Tile.Feature feature1, VectorTileProto.Tile.Feature feature2) { - compareValues(layer, "feature id", feature1.getId(), feature2.getId()); - compareValues(layer, "feature type", feature1.getType(), feature2.getType()); - compareValues(layer, "feature geometry", feature1.getGeometryList(), feature2.getGeometryList()); - compareValues(layer, "feature tags", feature1.getTagsCount(), feature2.getTagsCount()); + if (compareValues(coord, layer, "feature type", feature1.getType(), feature2.getType())) { + var geomType = feature1.getType(); + if (!compareValues(coord, layer, "feature " + geomType.toString().toLowerCase() + " geometry commands", + feature1.getGeometryList(), feature2.getGeometryList())) { + var geom1 = + new VectorTile.VectorGeometry(Ints.toArray(feature1.getGeometryList()), GeometryType.valueOf(geomType), 0); + var geom2 = + new VectorTile.VectorGeometry(Ints.toArray(feature2.getGeometryList()), GeometryType.valueOf(geomType), 0); + try { + compareGeometry(coord, layer, geom1.decode(), geom2.decode()); + } catch (GeometryException e) { + LOGGER.error("Error decoding geometry", e); + } + } + } } - private void compareLayerNames(VectorTileProto.Tile proto1, VectorTileProto.Tile proto2) { + private void compareGeometry(TileCoord coord, String layer, Geometry geom1, Geometry geom2) { + String geometryType = geom1.getGeometryType(); + compareValues(coord, layer, "feature JTS geometry type", geom1.getGeometryType(), geom2.getGeometryType()); + compareValues(coord, layer, "feature num geometries", geom1.getNumGeometries(), geom2.getNumGeometries()); + if (geom1 instanceof MultiPolygon) { + for (int i = 0; i < geom1.getNumGeometries(); i++) { + comparePolygon(coord, layer, geometryType, (Polygon) geom1.getGeometryN(i), (Polygon) geom2.getGeometryN(i)); + } + } else if (geom1 instanceof Polygon p1 && geom2 instanceof Polygon p2) { + comparePolygon(coord, layer, geometryType, p1, p2); + } + } + + private void comparePolygon(TileCoord coord, String layer, String geomType, Polygon p1, Polygon p2) { + compareValues(coord, layer, geomType + " exterior ring geometry", p1.getExteriorRing(), p2.getExteriorRing()); + if (compareValues(coord, layer, geomType + " num interior rings", p1.getNumInteriorRing(), + p2.getNumInteriorRing())) { + for (int i = 0; i < p1.getNumInteriorRing(); i++) { + compareValues(coord, layer, geomType + " interior ring geometry", p1.getInteriorRingN(i), + p2.getInteriorRingN(i)); + } + } + } + + private void compareLayerNames(TileCoord coord, VectorTileProto.Tile proto1, VectorTileProto.Tile proto2) { var layers1 = proto1.getLayersList().stream().map(d -> d.getName()).toList(); var layers2 = proto2.getLayersList().stream().map(d -> d.getName()).toList(); - compareListDetailed("tile layers", layers1, layers2); + compareListDetailed(coord, "tile layers", layers1, layers2); } - private boolean compareList(String layer, String name, List value1, List value2) { - return compareValues(layer, name + " unique values", Set.copyOf(value1), Set.copyOf(value2)) && - compareValues(layer, name + " order", value1, value2); + private boolean compareList(TileCoord coord, String layer, String name, List value1, List value2) { + return compareValues(coord, layer, name + " unique values", Set.copyOf(value1), Set.copyOf(value2)) && + compareValues(coord, layer, name + " order", value1, value2); } - private void compareListDetailed(String name, List value1, List value2) { + private void compareListDetailed(TileCoord coord, String name, List value1, List value2) { if (!Objects.equals(value1, value2)) { boolean missing = false; for (var layer : value1) { if (!value2.contains(layer)) { - recordTileDiff(name + " 2 missing " + layer); + recordTileDiff(coord, name + " 2 missing " + layer); missing = true; } } for (var layer : value2) { if (!value1.contains(layer)) { - recordTileDiff(name + " 1 missing " + layer); + recordTileDiff(coord, name + " 1 missing " + layer); missing = true; } } if (!missing) { - recordTileDiff(name + " different order"); + recordTileDiff(coord, name + " different order"); } } } - private boolean compareValues(String layer, String name, T value1, T value2) { + private boolean compareValues(TileCoord coord, String layer, String name, T value1, T value2) { if (!Objects.equals(value1, value2)) { - recordLayerDiff(layer, name); + recordLayerDiff(coord, layer, name); return false; } return true; } - private VectorTileProto.Tile decode(byte[] bytes, TileCompression tileCompression) throws IOException { - byte[] decompressed = switch (tileCompression) { + private byte[] decompress(byte[] bytes, TileCompression tileCompression) throws IOException { + return switch (tileCompression) { case GZIP -> Gzip.gunzip(bytes); case NONE -> bytes; case UNKNOWN -> throw new IllegalArgumentException("Unknown compression"); }; - return VectorTileProto.Tile.parseFrom(decompressed); } - private void recordLayerDiff(String layer, String issue) { + private VectorTileProto.Tile decode(byte[] decompressedTile) throws IOException { + return VectorTileProto.Tile.parseFrom(decompressedTile); + } + + private void recordLayerDiff(TileCoord coord, String layer, String issue) { var layerDiffs = diffsByLayer.get(layer); if (layerDiffs == null) { layerDiffs = diffsByLayer.computeIfAbsent(layer, k -> new ConcurrentHashMap<>()); } layerDiffs.merge(issue, 1L, Long::sum); + if (verbose) { + LOGGER.debug("{} layer {} {}", coord, layer, issue); + } } - private void recordTileDiff(String issue) { + private void recordTileDiff(TileCoord coord, String issue) { diffTypes.merge(issue, 1L, Long::sum); + if (verbose) { + LOGGER.debug("{} {}", coord, issue); + } } - public record Result(long total, long tileDiffs, Map diffTypes, - Map> diffsByLayer) {} + public record Result( + long total, long tileDiffs, Map diffTypes, + Map> diffsByLayer + ) {} } diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LayerAttrStats.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LayerAttrStats.java index d078a120c8..c068c21e79 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LayerAttrStats.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LayerAttrStats.java @@ -2,7 +2,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.onthegomap.planetiler.archive.WriteableTileArchive; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -97,10 +96,6 @@ public VectorLayer(String id, Map fields, int minzoom, int ma this(id, fields, Optional.empty(), OptionalInt.of(minzoom), OptionalInt.of(maxzoom)); } - public static VectorLayer forLayer(String id) { - return new VectorLayer(id, new HashMap<>()); - } - public VectorLayer withDescription(String newDescription) { return new VectorLayer(id, fields, Optional.of(newDescription), minzoom, maxzoom); } @@ -174,7 +169,8 @@ interface ForLayer { private static class StatsForLayer { private final String layer; - private final Map fields = new HashMap<>(); + // use TreeMap to ensure the same output always appears the same in an archive + private final Map fields = new TreeMap<>(); private int minzoom = Integer.MAX_VALUE; private int maxzoom = Integer.MIN_VALUE; diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/util/CompareArchivesTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/util/CompareArchivesTest.java index 53b3f185cb..63b0e39ba0 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/util/CompareArchivesTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/util/CompareArchivesTest.java @@ -73,7 +73,8 @@ void testCompareArchives() throws IOException { var result = CompareArchives.compare( TileArchiveConfig.from(aPath.toString()), TileArchiveConfig.from(bPath.toString()), - config + config, + false ); assertEquals(new CompareArchives.Result( 5, 4, Map.of( From 142d96ab5dc993df627435b06934081c620232f5 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Fri, 12 Jan 2024 06:42:48 -0500 Subject: [PATCH 2/5] comments --- .../com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java index 16a110a41d..a050796fae 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java @@ -60,7 +60,7 @@ private static class Ring { private final Polygon geom; private final double area; private Ring containedBy = null; - // use linked hash set to ensure stable output + // use linked hash set to ensure same input always produces same output private final Set holes = new LinkedHashSet<>(); private Ring(Polygon geom) { @@ -226,7 +226,7 @@ private static void addPolygonRings(List polygons, Geometry geom) { } private static Set groupParentChildShells(List polygons) { - // use linked hash sate to ensure the same input always produces the same output + // use linked hash set to ensure the same input always produces the same output Set shells = new LinkedHashSet<>(); int numPolygons = polygons.size(); if (numPolygons == 0) { From 17d93d63dc2a5690e6cccbee5bf179bd42bf1650 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Fri, 12 Jan 2024 06:54:25 -0500 Subject: [PATCH 3/5] improve test coverage --- .../planetiler/util/CompareArchivesTest.java | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/util/CompareArchivesTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/util/CompareArchivesTest.java index 63b0e39ba0..6a9c1f99e3 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/util/CompareArchivesTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/util/CompareArchivesTest.java @@ -6,6 +6,7 @@ import com.onthegomap.planetiler.archive.TileArchiveConfig; import com.onthegomap.planetiler.archive.TileArchiveMetadata; import com.onthegomap.planetiler.archive.TileEncodingResult; +import com.onthegomap.planetiler.config.Arguments; import com.onthegomap.planetiler.config.PlanetilerConfig; import com.onthegomap.planetiler.geo.TileOrder; import com.onthegomap.planetiler.pmtiles.WriteablePmtiles; @@ -89,4 +90,58 @@ void testCompareArchives() throws IOException { ) ), result); } + + @Test + void testCompareArchivesDifferentCompression() throws IOException { + var aPath = path.resolve("a.pmtiles"); + var bPath = path.resolve("b.pmtiles"); + byte[] a1 = new byte[]{0xa, 0x2}; + byte[] b1 = Gzip.gzip(a1); + byte[] a2 = tile1; + byte[] b2 = Gzip.gzip(tile2); + try ( + var a = WriteablePmtiles.newWriteToFile(aPath); + var b = WriteablePmtiles.newWriteToFile(bPath); + ) { + a.initialize(); + b.initialize(); + try ( + var aWriter = a.newTileWriter(); + var bWriter = b.newTileWriter() + ) { + aWriter + .write(new TileEncodingResult(TileOrder.HILBERT.decode(0), a1, OptionalLong.empty())); + aWriter + .write(new TileEncodingResult(TileOrder.HILBERT.decode(2), a2, OptionalLong.empty())); + aWriter + .write(new TileEncodingResult(TileOrder.HILBERT.decode(4), a1, OptionalLong.empty())); + bWriter.write(new TileEncodingResult(TileOrder.HILBERT.decode(1), b1, OptionalLong.empty())); + bWriter.write(new TileEncodingResult(TileOrder.HILBERT.decode(2), b2, OptionalLong.empty())); + bWriter.write(new TileEncodingResult(TileOrder.HILBERT.decode(3), b1, OptionalLong.empty())); + bWriter + .write(new TileEncodingResult(TileOrder.HILBERT.decode(4), b1, OptionalLong.empty())); + } + a.finish(new TileArchiveMetadata(new Profile.NullProfile(), + PlanetilerConfig.from(Arguments.fromArgs("--tile-compression=none")))); + b.finish(new TileArchiveMetadata(new Profile.NullProfile(), config)); + } + var result = CompareArchives.compare( + TileArchiveConfig.from(aPath.toString()), + TileArchiveConfig.from(bPath.toString()), + config, + false + ); + assertEquals(new CompareArchives.Result( + 5, 4, Map.of( + "archive 2 missing tile", 1L, + "archive 1 missing tile", 2L, + "different decompressed contents", 1L + ), Map.of( + "layer1", Map.of( + "values list unique values", 1L, + "feature ids", 1L + ) + ) + ), result); + } } From 6cca1b4c122b0b50397854091d6290394373b1e5 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Fri, 12 Jan 2024 07:01:50 -0500 Subject: [PATCH 4/5] comment --- .../com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java index a050796fae..88eef7d8af 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmMultipolygon.java @@ -40,7 +40,9 @@ * Multipolygon way members have an "inner" and "outer" role, but they can be incorrectly specified, so instead * determine the nesting order and alternate outer/inner/outer/inner... from the outermost ring inwards. *

- * This class is ported to Java from ... and ... + * This class is ported to Java from + * imposm3 multipolygon.go and + * imposm3 ring.go */ public class OsmMultipolygon { /* From da8e4afe599f33672a467b331dc9cb2c6b4591c3 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Fri, 12 Jan 2024 07:14:45 -0500 Subject: [PATCH 5/5] require same tile ordering --- .../java/com/onthegomap/planetiler/util/CompareArchives.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/CompareArchives.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/CompareArchives.java index 7a99c6b933..3aefe7f0b3 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/CompareArchives.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/CompareArchives.java @@ -141,6 +141,11 @@ private Result getResult(PlanetilerConfig config) { } var order = input1.format().preferredOrder(); + var order2 = input2.format().preferredOrder(); + if (order != order2) { + throw new IllegalArgumentException( + "Archive orders must be the same to compare, got " + order + " and " + order2); + } var stats = config.arguments().getStats(); var total = new AtomicLong(0); var diffs = new AtomicLong(0);