Make tile extent runtime configurable with default 4096#1565
Open
tasiotas wants to merge 3 commits into
Open
Conversation
Full logs: https://github.com/onthegomap/planetiler/actions/runs/26041130395 |
msbarry
reviewed
May 16, 2026
msbarry
left a comment
msbarry
left a comment
Contributor
There was a problem hiding this comment.
Thanks for adding this! I'm a big nervous about other places that have a 4096 assumption built into them without explicitly referring to 4096. I think the end to end tests in PlanetilerTests will help flush out any of those.
Comment on lines
+94
to
+96
| if (tileExtent <= 0) { | ||
| throw new IllegalArgumentException("tile_extent must be > 0, was " + tileExtent); | ||
| } |
Contributor
There was a problem hiding this comment.
I think that extent needs to be a power of 2?
| ), "layer", Map.of(), 0) | ||
| ) | ||
| )), sortListValues(results.tiles)); | ||
| } |
Contributor
There was a problem hiding this comment.
Can we add a test for end to end point/line/polygon for a few different values of tile extent? At least 8192 and 16384, possibly a lower value like 512 or 1024 as well? I think we should probably test a worst case for each of those with coordinates at (-255, -255) (513, -255) (513, 513) (-255, 513) and a few points in the middle
| scale = Math.min(31 - 14, scale); | ||
| long maxCoordinate = config.tileExtent() * 4L; | ||
| int bits = 64 - Long.numberOfLeadingZeros(maxCoordinate - 1); | ||
| scale = Math.clamp(scale, 0, Math.max(0, 31 - bits)); |
Contributor
There was a problem hiding this comment.
There are a few more subtle places that the 4096 assumption has snuck in that might not explicitly reference 4096 - this is one of them, thanks for fixing! I'm trying to think if there might be any others...
| int y = zigZagEncode((int) Math.round(coord.y * 4096 / 256)); | ||
| int x = zigZagEncode((int) Math.round(coord.x * tileExtent / SIZE)); | ||
| int y = zigZagEncode((int) Math.round(coord.y * tileExtent / SIZE)); | ||
| return (int) Hilbert.hilbertXYToIndex(15, x, y); |
Contributor
There was a problem hiding this comment.
Zig-zag encoding maps coordinates in a range from -2^14 -> 2^14 to 0 -> 2^15. That assumption was based off 4096x4096 tiles with a 4096 buffer, so -4096 -> 8192 that gets mapped to 0 -> 16384 (2^14 plus 1 for safety). I think we could probably bump 15 up to 16, but then if tile extent / size exceeds a certain threshold then we might need to shift x and y right by a certain amount to keep it in the right range.
Comment on lines
+52
to
+56
| /** | ||
| * Rounding precision for 256x256px tiles encoded using {@code tile_extent} values (default 4096). | ||
| */ | ||
| private static final AtomicReference<PrecisionModel> tilePrecision = | ||
| new AtomicReference<>(new PrecisionModel(4096d / 256d)); |
Contributor
There was a problem hiding this comment.
Ideally we move all references to 4096 out of static variables and either pass them as args to functions that need them or extract them from geoutils to a class that you instantiate with a tile extent. That might make this PR very big though, let me know what you think - if it's too much I could do in a followup PR.
Contributor
There was a problem hiding this comment.
This issue also asks for the extent to be configurable per-zoom #1286 so we might even want to make it a global setting 🤔
Author
There was a problem hiding this comment.
how would you implement per zoom tile extent configuration? per zoom args?
--tileExtentZ12, --tileExtentZ13, etc... ?
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi
I added runtime arg for tile extent. Defaults to 4096, I tested it with 8192 and 16384. My use case required higher precision because I'm rendering maps with tileserver-gl at very high dpi for print.
Passed all tests. I used Codex 5.3