From fcecdf9afb381e2b849b3cdd3fe1549ddf3c19c2 Mon Sep 17 00:00:00 2001
From: Erica Fischer <enf@felt.com>
Date: Wed, 14 Feb 2024 16:02:43 -0800
Subject: [PATCH] Don't always keep the first feature in a tile, after all

---
 main.cpp | 42 ++++++++++++++++++++----------------------
 tile.cpp | 20 +++++++++++++-------
 2 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/main.cpp b/main.cpp
index 15212184..bc310785 100644
--- a/main.cpp
+++ b/main.cpp
@@ -298,31 +298,26 @@ struct drop_densest {
 	}
 };
 
-int calc_feature_minzoom(struct index *ix, struct drop_state *ds, int maxzoom, double gamma) {
-	int feature_minzoom = 0;
+int calc_feature_minzoom(struct index *ix, struct drop_state *ds, size_t maxzoom, double gamma) {
+	size_t feature_minzoom = 0;
 
 	if (gamma >= 0 && (ix->t == VT_POINT ||
 			   (additional[A_LINE_DROP] && ix->t == VT_LINE) ||
 			   (additional[A_POLYGON_DROP] && ix->t == VT_POLYGON))) {
-		for (ssize_t i = maxzoom; i >= 0; i--) {
+		feature_minzoom = maxzoom;
+
+		// Advance the counter for all zooms
+
+		for (size_t i = 0; i <= maxzoom; i++) {
 			ds[i].seq++;
 		}
-		for (ssize_t i = maxzoom; i >= 0; i--) {
-			if (ds[i].seq < 0) {
-				feature_minzoom = i + 1;
 
-				// The feature we are pushing out
-				// appears in zooms i + 1 through maxzoom,
-				// so track where that was so we can make sure
-				// not to cluster something else that is *too*
-				// far away into it.
-				for (ssize_t j = i + 1; j <= maxzoom; j++) {
-					ds[j].previndex = ix->ix;
-				}
+		// Find the zoom at which it is time for a feature
 
+		for (size_t i = 0; i <= maxzoom; i++) {
+			if (ds[i].seq >= ds[i].interval) {
+				feature_minzoom = i;
 				break;
-			} else {
-				ds[i].seq -= ds[i].interval;
 			}
 		}
 
@@ -332,19 +327,22 @@ int calc_feature_minzoom(struct index *ix, struct drop_state *ds, int maxzoom, d
 		// we will go ahead and push it out.
 
 		if (preserve_point_density_threshold > 0) {
-			for (ssize_t i = 0; i < feature_minzoom && i < maxzoom; i++) {
+			for (size_t i = 0; i < feature_minzoom && i < maxzoom; i++) {
 				if (ix->ix - ds[i].previndex > ((1LL << (32 - i)) / preserve_point_density_threshold) * ((1LL << (32 - i)) / preserve_point_density_threshold)) {
 					feature_minzoom = i;
-
-					for (ssize_t j = i; j <= maxzoom; j++) {
-						ds[j].previndex = ix->ix;
-					}
-
 					break;
 				}
 			}
 		}
 
+		// Set the counter back in all zooms that this feature will appear in,
+		// and remember its index for the next maximum-distance-exceeded decision
+
+		for (size_t i = feature_minzoom; i <= maxzoom; i++) {
+			ds[i].seq -= ds[i].interval;
+			ds[i].previndex = ix->ix;
+		}
+
 		// XXX manage_gap
 	}
 
diff --git a/tile.cpp b/tile.cpp
index d8528bdb..1930c3b7 100644
--- a/tile.cpp
+++ b/tile.cpp
@@ -1079,7 +1079,7 @@ static serial_feature next_feature(decompressor *geoms, std::atomic<long long> *
 		if (sf.geometry.size() > 0) {
 			(*unclipped_features)++;
 		} else {
-			// XXX should continue, but affects test outputs
+			continue;
 		}
 
 		if (first_time && pass == 0) { /* only write out the next zoom once, even if we retry */
@@ -1163,13 +1163,13 @@ static serial_feature next_feature(decompressor *geoms, std::atomic<long long> *
 
 			std::string &layername = (*layer_unmaps)[sf.segment][sf.layer];
 			auto count = multiplier_state->count.find(layername);
-			if (count == multiplier_state->count.end()) {
-				multiplier_state->count.emplace(layername, 0);
-				count = multiplier_state->count.find(layername);
-				sf.dropped = FEATURE_KEPT;  // the first feature in each tile is always kept
-			}
 
-			if (z >= sf.feature_minzoom || sf.dropped == FEATURE_KEPT) {
+			if (z >= sf.feature_minzoom) {
+				if (count == multiplier_state->count.end()) {
+					multiplier_state->count.emplace(layername, 0);
+					count = multiplier_state->count.find(layername);
+				}
+
 				count->second = 0;
 				sf.dropped = FEATURE_KEPT;  // feature is kept
 
@@ -1178,6 +1178,12 @@ static serial_feature next_feature(decompressor *geoms, std::atomic<long long> *
 					sf.full_values.emplace_back(mvt_bool, "true");
 				}
 			} else if (count->second + 1 < retain_points_multiplier) {
+				if (count == multiplier_state->count.end()) {
+					// just drop features until we reach one that is
+					// supposed to appear in this zoom level
+					continue;
+				}
+
 				count->second++;
 				sf.dropped = count->second;
 			} else {