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

Support for a tile-relative "rank" attribute #547

Closed

Conversation

keichan34
Copy link
Contributor

I started a discussion a year or so ago with no reaction about this feature, so I decided to try to implement it.

The original motivation behind creating this feature was to cut down on tile size / rendering speed by removing objects in the vector tile that wouldn't be displayed. OpenMapTiles uses postgres to reorder and dynamically set the rank attribute, while Tilemaker currently uses static values set in process.lua. For example, hardcoded min/max zoom and ranks for cities based on population can be fine for country or area level exports, but it's hard to create a general rule when trying to make tiles for a wide area that have different standards for city size / area, etc.

This feature introduces a new Lua function, node:SetRank(x) where x is the value in which the object is sorted. In the layer config, a new property rank_max is introduced that controls how many objects of the given layer are output in each tile. The calculated rank attribute is relative per map tile.

I'm setting this PR as a draft because while I got it working as a quick demo, I want to know if this is a feature that should be in tilemaker in the first place before continuing further (that said, this is something I really want personally so I think I'll be maintaining a fork until there's something comparable...).

This is my first time writing C++ at this scale, so it would be great if someone could look over all my beginner mistakes! Thanks!

@systemed
Copy link
Owner

systemed commented Oct 2, 2023

This looks really good - thank you!

I very much like the idea of per-tile processing for cases like this - I can see it making a massive difference to placename display etc.

The one qualm I have is about increasing the OutputObject size with the extra float. OutputObjects make a big contribution to tilemaker's overall memory consumption so generally I try not to make them any larger.

I wonder if combining z_order and rankValue might work - they're quite a close conceptual fit, and that would mean we only had to store one value. z_order is currently (by default) one byte and that wouldn't be enough for this, but I wonder if we might be able to do something clever with a two-byte format. I'll give that some more thought.

I'm currently working on a significant refactoring in #499 which will eventually become tilemaker 3.0, so I could look at integrating this.

@keichan34
Copy link
Contributor Author

z_order is only used in ways, right? What about reusing z_order as a rank attribute in nodes, and applying a filter function on that? (I wonder if there's a use case for filtering on z_order on ways as well?)

Limiting the rank value to one byte could be problematic though. It could always be scaled down to fit to 256 values, but then the problem would be selecting the appropriate scale maximum. The compile-time option to use a float could be used as a backdoor to allow that kind of processing...

Alternatively, what do you think about storing the rank value as any other attribute would be stored, then overwriting it with the relative rank during the output step? (Thinking of trying this next on the #499 branch)

@systemed
Copy link
Owner

systemed commented Oct 3, 2023

z_order is stored (and settable) with any object, although it's mostly used with ways. The nice thing about using z_order is that we do already sort objects by that (in TileDataSource::getTileData), so all we need to do is change ProcessObjects to keep a count of objects written, and just break from the loop once we've hit the limit.

Some bit juggling should allow us to expand z_order to 16 bits, which I think will be enough if we use slightly lossy encoding. #499 already frees up a bit of the objectID space so we have some extra bits to play with. I think I'll have a go at implementing this as part of #499 - it's a great idea and should mesh nicely with the changes there.

systemed added a commit that referenced this pull request Oct 3, 2023
@systemed
Copy link
Owner

systemed commented Oct 3, 2023

Ok - I've had a stab at this in #499. Quick example of how to use it:

  • add "feature_limit": 20 to the layer definition for building in config-openmaptiles.json
  • add way:ZOrder(way:Area()) around line 480 of process-openmaptiles.lua (i.e. in the bit of code that processes buildings)

Then build some tiles! They'll be generated with only the 20 biggest buildings in each tile.

z_order is stored internally as 16 bits and will accept a value roughly between -50,000,000 and 50,000,000. It stores a lossy value which has a precision of 0.1 for values between -1000 and 1000, and a precision of floor(sqrt(n)) for values outside that range. This means you can use it for the population of a city if you like.

(#499 has a lot of internal changes but generally functions the same, with the exception that the include_ids option isn't currently supported.)

@keichan34
Copy link
Contributor Author

I took a quick look and generated some tiles, looks really good. Thanks!

Regarding the buildings example -- I think there should be another option? to remove the limit at base zoom (or a set zoom?). Maybe we want to keep only the X largest buildings in z13, but all buildings should be in z14. I'm thinking of something like "feature_limit": 20, "feature_limit_maxzoom": 13...

I got it to work by making a building_detail virtual layer that writes_to: building, but I don't think it's very intuitive -- I'm adding buildings both to building building_detail, when it could potentially just be one more option in the layer config. What do you think?

@systemed
Copy link
Owner

systemed commented Oct 4, 2023

That's a good idea - we have _below values for a lot of the config (e.g. combine_below, filter_below) so we could have feature_limit_below too.

@systemed
Copy link
Owner

systemed commented Oct 4, 2023

Now done in 05c3cda. As with the other settings, you set "feature_limit_below": 14 to keep everything at z14 but to filter from z13 and downwards.

@keichan34
Copy link
Contributor Author

@systemed I've been running a build based on the work in #499 and it's working pretty well. I'll close this PR for now. If I have any problems with it I'll comment directly in the PR or create a new issue. Thanks again!

@keichan34 keichan34 closed this Oct 6, 2023
@keichan34 keichan34 deleted the tile-relative-rank-attribute branch October 6, 2023 07:42
@systemed
Copy link
Owner

systemed commented Oct 6, 2023

Thank you for the original PR and idea!

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.

2 participants