Exclude more features from places layer at low zooms #1734
Conversation
nvkelso
changed the title
vectordatasource/transform.py
Outdated
| @@ -8129,7 +8129,7 @@ def min_zoom_filter(ctx): | |||
| for feature in features: | |||
| _, props, _ = feature | |||
| min_zoom = props.get('min_zoom') | |||
| if min_zoom is not None and min_zoom <= nominal_zoom + 1: | |||
| if min_zoom is not None and min_zoom <= nominal_zoom + 0.5: | |||
There was a problem hiding this comment.
Rather than change the zoom range we include in the tile (i.e: N to N+1), I think it would be better to change the min zoom we're applying from NE. For example, changing the setting code to assign props['min_zoom'] = min_zoom + 0.5 and similar for max_zoom. The two methods should be mathematically identical.
There was a problem hiding this comment.
Hmm, I don't like changing the min_zoom values to be inconsistent with Natural Earth – mostly because its had for me to wrap my head around as the author of Natural Earth! – but also because it's it's just values in the x.6 to x.9 range that should be ceiling'd up to x+1.0 to be more consistent?
Either way the places layer should be < nominal_zoom +1 not <= to avoid the Poland example?
There was a problem hiding this comment.
Attempting fix in 9914c27.
|
@zerebubuth Ready for another look. FYI: I did implement modified version of |
| @@ -8129,7 +8131,7 @@ def min_zoom_filter(ctx): | |||
| for feature in features: | |||
| _, props, _ = feature | |||
| min_zoom = props.get('min_zoom') | |||
| if min_zoom is not None and min_zoom <= nominal_zoom + 0.5: | |||
| if min_zoom is not None and min_zoom < nominal_zoom + 1: | |||
vectordatasource/transform.py
Outdated
| # don't overstuff features into tiles when they are in the | ||
| # long tail of won't display, but make their min_zoom | ||
| # consistent with when they actually show in tiles | ||
| if ceil(min_zoom) == trunc(min_zoom) + 1: |
There was a problem hiding this comment.
I think this is the same as "if min_zoom is an integer", in which case the if/else isn't necessary and we can just write props['min_zoom'] = ceil(min_zoom) for everything (because ceil(x) = x for integers).
There was a problem hiding this comment.
What I'm trying to do:
- 1.0 stays 1.0
- 1.1 stays 1.1
- 1.2 stays 1.2
- 1.3 stays 1.3
- 1.4 stays 1.4
- 1.5 stays 1.5 (probably fails now)
- 1.6 goes to 2.0
- 1.7 goes to 2.0
- 1.8 goes to 2.0
- 1.9 goes to 2.0
Can you recommend a quick fix for that, please?
There was a problem hiding this comment.
Something like: props['min_zoom'] = ceil(min_zoom) if min_zoom % 1 > 0.5 else min_zoom
Which basically says "if the fractional part is more than half round it up, otherwise keep it the same". If you prefer, it might be more readable as:
if min_zoom % 1 > 0.5:
min_zoom = ceil(min_zoom)
props['min_zoom'] = min_zoom
|
🎉 Huzzah! Merging. |
| @@ -215,7 +215,7 @@ filters: | |||
| # - {kind: ocean} | |||
| # table: shp | |||
| - filter: {meta.source: shp} | |||
| min_zoom: 0 | |||
There was a problem hiding this comment.
Ooops, I think this might have been the polygon, not the ocean label....
There was a problem hiding this comment.
ah: meta.source: shp is for the openstreetmapdata.com source.
There was a problem hiding this comment.
Looks like they are all nodes:
Connects to #1729 to limit places layer features to only >= +0.5 of the nominal zoom instead of >= +1 to reduce amount of features (and reduce file size, translations multiply!), especially at low zooms.
This better matches how Natural Earth is curated starting in version 4.1 for
min_zoom.For example, this will remove the 1.7 features above from the zoom 1 tile, but they'll show up in zoom 2 tile and collide out min_zoom 2.0 features – good).