Be more careful to retry when the feature count is exceeded (#257)

* Clip before dealing with multiplier or filters in overzoom

* Be more careful to retry when the feature count is exceeded

* Adjust the estimated total feature count for the multiplier too

* Fix the feature count estimates, I think

* Pass build info into the version string

* Report the actual max zoom of any tiles as the metadata maxzoom

* Revert unneeded renaming to make the diff more readable

* Clean up the adjustments to tile sizes and feature counts

* Update version and changelog

* Dropping a feature into a multiplier cluster still effectively drops it

* Update changelog

* Rethink the changelog description

* Don't try to truncate zooms if we are still tiling at z18

CHANGELOG.md

@@ -1,3 +1,11 @@
+# 2.60.0
+
+* Fix bad interaction between --retain-points-multiplier and stopping early when the tile feature limit is reached
+* Fix another bad interaction between --retain-points-multiplier, dropping-as-needed, and variable depth tile pyramids
+* Add optional BUILD_INFO string to version
+* Reorder overzoom logic to clip before dealing with multiplier and filters
+* When --generate-variable-depth-tile-pyramid is in use, report the actual highest zoom generated as tileset maxzoom
+
 # 2.59.0
 
 * Correct `antimeridian_adjusted_bounds` latitude calculation when vertices extend beyond the edge of the Mercator plane

Makefile

@@ -1,14 +1,15 @@
 PREFIX ?= /usr/local
 MANDIR ?= $(PREFIX)/share/man/man1/
 BUILDTYPE ?= Release
+BUILD_INFO ?=
 SHELL = /bin/sh
 
 
 # inherit from env if set
 CC := $(CC)
 CXX := $(CXX)
-CFLAGS := $(CFLAGS) -fPIE
-CXXFLAGS := $(CXXFLAGS) -std=c++17 -fPIE
+CFLAGS := $(CFLAGS) -fPIE -DBUILD_INFO=$(BUILD_INFO)
+CXXFLAGS := $(CXXFLAGS) -std=c++17 -fPIE -DBUILD_INFO=$(BUILD_INFO)
 LDFLAGS := $(LDFLAGS)
 WARNING_FLAGS := -Wall -Wshadow -Wsign-compare -Wextra -Wunreachable-code -Wuninitialized -Wshadow
 RELEASE_FLAGS := -O3 -DNDEBUG

clip.cpp

@@ -1188,37 +1188,6 @@ std::string overzoom(std::vector<source_tile> const &tiles, int nz, int nx, int
 			static const std::string retain_points_multiplier_sequence = "tippecanoe:retain_points_multiplier_sequence";
 
 			for (auto feature : layer.features) {
-				bool flush_multiplier_cluster = false;
-				if (demultiply) {
-					for (ssize_t i = feature.tags.size() - 2; i >= 0; i -= 2) {
-						if (layer.keys[feature.tags[i]] == retain_points_multiplier_first) {
-							mvt_value v = layer.values[feature.tags[i + 1]];
-							if (v.type == mvt_bool && v.numeric_value.bool_value) {
-								flush_multiplier_cluster = true;
-								feature.tags.erase(feature.tags.begin() + i, feature.tags.begin() + i + 2);
-							}
-						} else if (i < (ssize_t) feature.tags.size() && layer.keys[feature.tags[i]] == retain_points_multiplier_sequence) {
-							mvt_value v = layer.values[feature.tags[i + 1]];
-							feature.seq = mvt_value_to_long_long(v);
-							feature.tags.erase(feature.tags.begin() + i, feature.tags.begin() + i + 2);
-						}
-					}
-				} else {
-					flush_multiplier_cluster = true;
-				}
-
-				if (flush_multiplier_cluster) {
-					if (pending_tile_features.size() > 0) {
-						feature_out(pending_tile_features, *outlayer, keep, attribute_accum, tile_stringpool);
-						pending_tile_features.clear();
-					}
-				}
-
-				std::set<std::string> exclude_attributes;
-				if (filter != NULL && !evaluate(feature, layer, filter, exclude_attributes, nz, unidecode_data)) {
-					continue;
-				}
-
 				drawvec geom;
 				int t = feature.type;
 
@@ -1268,6 +1237,7 @@ std::string overzoom(std::vector<source_tile> const &tiles, int nz, int nx, int
 
 					long long b = outtilesize * buffer / 256;
 					if (xmax < -b || ymax < -b || xmin > outtilesize + b || ymin > outtilesize + b) {
+						// quick exclusion by bounding box
 						continue;
 					}
 
@@ -1281,6 +1251,42 @@ std::string overzoom(std::vector<source_tile> const &tiles, int nz, int nx, int
 					}
 				}
 
+				if (geom.size() == 0) {
+					// clipped away
+					continue;
+				}
+
+				bool flush_multiplier_cluster = false;
+				if (demultiply) {
+					for (ssize_t i = feature.tags.size() - 2; i >= 0; i -= 2) {
+						if (layer.keys[feature.tags[i]] == retain_points_multiplier_first) {
+							mvt_value v = layer.values[feature.tags[i + 1]];
+							if (v.type == mvt_bool && v.numeric_value.bool_value) {
+								flush_multiplier_cluster = true;
+								feature.tags.erase(feature.tags.begin() + i, feature.tags.begin() + i + 2);
+							}
+						} else if (i < (ssize_t) feature.tags.size() && layer.keys[feature.tags[i]] == retain_points_multiplier_sequence) {
+							mvt_value v = layer.values[feature.tags[i + 1]];
+							feature.seq = mvt_value_to_long_long(v);
+							feature.tags.erase(feature.tags.begin() + i, feature.tags.begin() + i + 2);
+						}
+					}
+				} else {
+					flush_multiplier_cluster = true;
+				}
+
+				if (flush_multiplier_cluster) {
+					if (pending_tile_features.size() > 0) {
+						feature_out(pending_tile_features, *outlayer, keep, attribute_accum, tile_stringpool);
+						pending_tile_features.clear();
+					}
+				}
+
+				std::set<std::string> exclude_attributes;
+				if (filter != NULL && !evaluate(feature, layer, filter, exclude_attributes, nz, unidecode_data)) {
+					continue;
+				}
+
 				bool still_need_simplification_after_reduction = false;
 				if (t == VT_POLYGON && tiny_polygon_size > 0) {
 					bool simplified_away_by_reduction = false;

main.cpp

@@ -49,7 +49,6 @@
 #include "tile.hpp"
 #include "pool.hpp"
 #include "projection.hpp"
-#include "version.hpp"
 #include "memfile.hpp"
 #include "main.hpp"
 #include "geojson.hpp"
@@ -3608,7 +3607,7 @@ int main(int argc, char **argv) {
 		}
 
 		case 'v':
-			fprintf(stderr, "tippecanoe %s\n", VERSION);
+			fprintf(stderr, "tippecanoe %s\n", version_str().c_str());
 			exit(EXIT_SUCCESS);
 
 		case 'P':

mbtiles.cpp

@@ -654,6 +654,17 @@ static double sixdig(double val) {
 	return std::round(val * 1e6) / 1e6;
 }
 
+#define str(x) #x
+#define xstr(x) str(x)
+std::string version_str() {
+	std::string s = VERSION;
+	std::string build_info = xstr(BUILD_INFO);
+	if (build_info.size() > 0) {
+		s += " " + build_info;
+	}
+	return s;
+}
+
 metadata make_metadata(const char *fname, int minzoom, int maxzoom, double minlat, double minlon, double maxlat, double maxlon, double minlat2, double minlon2, double maxlat2, double maxlon2, double midlat, double midlon, const char *attribution, std::map<std::string, layermap_entry> const &layermap, bool vector, const char *description, bool do_tilestats, std::map<std::string, std::string> const &attribute_descriptions, std::string const &program, std::string const &commandline, std::vector<strategy> const &strategies, int basezoom, double droprate, int retain_points_multiplier) {
 	metadata m;
 
@@ -684,7 +695,7 @@ metadata make_metadata(const char *fname, int minzoom, int maxzoom, double minla
 		m.attribution = attribution;
 	}
 
-	m.generator = program + " " + VERSION;
+	m.generator = program + " " + version_str();
 	m.generator_options = commandline;
 
 	m.strategies_json = stringify_strategies(strategies);

mbtiles.hpp

@@ -77,5 +77,6 @@ std::map<std::string, layermap_entry> merge_layermaps(std::vector<std::map<std::
 std::map<std::string, layermap_entry> merge_layermaps(std::vector<std::map<std::string, layermap_entry> > const &maps, bool trunc);
 
 void add_to_tilestats(std::map<std::string, tilestat> &tilestats, std::string const &layername, serial_val const &val);
+std::string version_str();
 
 #endif

tests/loop/out/-z0_-O200_--cluster-densest-as-needed.json

@@ -9,7 +9,7 @@
 "maxzoom": "0",
 "minzoom": "0",
 "name": "tests/loop/out/-z0_-O200_--cluster-densest-as-needed.json.check.mbtiles",
-"strategies": "[{\"coalesced_as_needed\":999,\"feature_count_desired\":201}]",
+"strategies": "[{\"coalesced_as_needed\":999,\"feature_count_desired\":1000}]",
 "type": "overlay",
 "version": "2"
 }, "features": [

tests/loop/out/-z0_-O200_--drop-densest-as-needed.json

@@ -9,7 +9,7 @@
 "maxzoom": "0",
 "minzoom": "0",
 "name": "tests/loop/out/-z0_-O200_--drop-densest-as-needed.json.check.mbtiles",
-"strategies": "[{\"dropped_as_needed\":999,\"feature_count_desired\":201}]",
+"strategies": "[{\"dropped_as_needed\":999,\"feature_count_desired\":1000}]",
 "type": "overlay",
 "version": "2"
 }, "features": [

tests/loop/out/-z0_-O200_--drop-fraction-as-needed.json

@@ -9,7 +9,7 @@
 "maxzoom": "0",
 "minzoom": "0",
 "name": "tests/loop/out/-z0_-O200_--drop-fraction-as-needed.json.check.mbtiles",
-"strategies": "[{\"dropped_as_needed\":999,\"feature_count_desired\":201}]",
+"strategies": "[{\"dropped_as_needed\":999,\"feature_count_desired\":1000}]",
 "type": "overlay",
 "version": "2"
 }, "features": [