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

Fix accidental loss (at all zooms) of features with an explicit minzoom #221

Merged
merged 2 commits into from
Mar 22, 2024

Conversation

e-n-f
Copy link
Collaborator

@e-n-f e-n-f commented Mar 20, 2024

As reported in #219, versions of tippecanoe since 2.43.0 have accidentally dropped all features that specified an explicit minzoom with "tippecanoe":{"minzoom":nnn}. This PR adds an else clause to retain these features in the specified zoom range.

The test file is the Natural Earth 110m populated places, with the scalerank used as the minzoom. Each feature contains an object like this to specify its minzoom:

{"type":"Feature","tippecanoe":{"minzoom":8},"properties":{…},…}

The unrelated change to mbtiles.cpp and joined_reordered.json is to round the tileset bounding boxes and centers to six digits to avoid test failures on some platforms from slightly different floating point behavior.

Copy link
Contributor

@springmeyer springmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good 👍🏼

The rounding fix ensures all tests pass for me locally with Apple clang version 15.0.0 (clang-1500.1.0.2.5) on an M2 Mac.

I also confirmed that the new test you added looks good: it correctly catches the problem. If I comment out the fix:

diff --git a/tile.cpp b/tile.cpp
index e8ce1ce..a166939 100644
--- a/tile.cpp
+++ b/tile.cpp
@@ -1179,7 +1179,7 @@ static serial_feature next_feature(decompressor *geoms, std::atomic<long long> *
                                sf.dropped = FEATURE_DROPPED;
                        }
                } else {
-                       sf.dropped = FEATURE_KEPT;
+                       //sf.dropped = FEATURE_KEPT;
                }
 
                // Remove nulls, now that the expression evaluation filter has run

Then I see the new test fail, as expected:

tests/ne_110m_populated_places-minzoom/out/-yNAME_-ySCALERANK_-z5.json.check.out tests/ne_110m_populated_places-minzoom/out/-yNAME_-ySCALERANK_-z5.json differ: char 195, line 4
make: *** [tests/ne_110m_populated_places-minzoom/out/-yNAME_-ySCALERANK_-z5.json.check] Error 1

@e-n-f e-n-f merged commit bd48ba8 into main Mar 22, 2024
4 checks passed
@e-n-f e-n-f deleted the tippecanoe-219-explicit-minzoom branch March 22, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants