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

Clipping monster polygons 2 #607

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

cldellow
Copy link
Contributor

@cldellow cldellow commented Dec 7, 2023

This replaces #606, fixes #605

When writing, we keep a cache of previously clipped geometries. Higher zooms will prefer to clip a cached, clipped geometry over an uncached, unclipped geometry.

With this PR:

  • Hudson Bay: 1m28s
  • Antarctica: 9m5s

Antarctica doesn't max out the CPU due to #596 - I think a next step there could be to have a buffer so workers can just enqueue a tile (up to some limit) and move on with their work.

I also noticed that the Antarctica mbtiles couldn't be converted by the pmtiles convert command. I suspect this is a pre-existing issue, but I've never generated Antarctica before. My guess is it's related to

if (featurePtr->geometry_size()==0) { vtLayer->mutable_features()->RemoveLast(); continue; }
, and we need to not write a tile at all if it has no features, not just remove the empty feature.

Probably it's OK to tackle that as a separate issue.

This replaces systemed#606

Rather than dealing with the special case of fully covered or fully
excluded, we keep a cache of recently clipped geometries.

A higher zoom will look in the cache for a clipped geometry of
the object from a lower zoom and re-use it if possible, falling
back to the whole geometry only if no cache entry is found.

Hudson Bay, master: 22m27s
this PR: 1m28s

This relies on the assumption that clipping a clipped polygon gives
the same result as clipping the original polygon. That seems
reasonable to me, but if you know of edge cases to consider,
let me know.
This is another fix in the vein of systemed@066ca0a

Testing Antarctica with this PR: 9m5s

The generated mbtiles doesn't seem to be convertible to a pmtiles file
-- `pmtiles convert` fails with:

```
main.go:162: Failed to convert /home/cldellow/src/basemap/tiles.mbtiles, Missing row
```

I've never generated Antarctica with the previous code, so I'm not sure
if this is specific to this PR or not.

My intuition is this is not specific to this PR. Wild guess is that
we're creating tiles with no features, due to https://github.com/systemed/tilemaker/blob/d470dc94fea6ae74e948c1a858c6e1228c9f4bf9/src/tile_worker.cpp#L210, and pmtiles doesn't like that.
@cldellow
Copy link
Contributor Author

cldellow commented Dec 7, 2023

This and #590 lets me generate North America in 43 minutes! The resulting mbtiles was able to be converted to a pmtiles file, which gives me some comfort that the Antarctica thing is probably unique to Antarctica.

Runtime looked like it was also impacted by #596, so hopefully we can do better than 43 minutes, too. (updated: with #608, it takes a little less than 40 minutes)

@systemed systemed merged commit 73771a1 into systemed:master Dec 8, 2023
5 checks passed
@systemed
Copy link
Owner

systemed commented Dec 8, 2023

This is a really neat, elegant solution! Thank you.

@systemed systemed mentioned this pull request Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

during writing, much time is spent in seemingly empty tiles
2 participants