Merged
Conversation
…hat 'name' really means 'all name-like properties'. This is useful when names have translations, and we don't want to have to spell out all the variants and translations by hand.
…MultiLineString to separate MultiLineStrings. This was previously a naive O(n^2) algorithm, but is now a naive O(n log n) algorithm.
Member
Author
|
Oh, I probably should have mentioned the times; previously, the |
rmarianski
approved these changes
Feb 26, 2019
| # if there's a large number of geoms, switch to the split index and sort | ||
| # so that the spatially largest objects are towards the end of the list. | ||
| # this should make it more likely that earlier queries are fast. | ||
| if len(mls.geoms) > 15000: |
Member
There was a problem hiding this comment.
Do we want to pull this value from config? Or is this a good number that's not likely to change?
Member
Author
There was a problem hiding this comment.
Good point, thanks. I think it's a pretty good number to start with, but we might end up tweaking it later. I've made it configurable in 5889c36, with the default 15,000.
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
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.
The tile
9/454/201was taking a huge amount of time to render - several hours. This is due to the changes I made in #1703 to prevent intersections between lines being re-added as part of MultiLineString operations. According to the OGC spec, LineStrings within a MultiLineString may only meet at points, but this means we end up with way more points in the output than we need to accurately describe the geometry. Therefore, we split them up into several features, where each MultiLineString in each feature only contains non-overlapping LineStrings.The algorithm I was using to do this wasn't efficient - it simply looped over all the features looking for intersections with previous features. And it worked OK on cities with a lower road density and lots of mergeable cross-shaped junctions, such as New York.
However, in Tokyo, where the tile was taking hours to render, there are 400,000 lines in the tile and the road network is not a grid, meaning there's less opportunity for merging.
Therefore, I replaced the previous algorithm with one that is more efficient for larger numbers of tiles. It works by indexing all the shapes rather than doing direct comparison. For large inputs, the shapes are additionally sorted by bounding box area and the index split between small and large. This means that the small-to-small comparisons are done more quickly (any given two small shapes are much less likely to intersect than a small and a large, or a large and a large).
Finally, we have been stripping
nameoff roads at low zooms, but were not strippingname:*translations orofficial_nameand similar "name-like" properties. This PR adds anall_name_variantsparameter todrop_propertieswhich tells it to treatnameas if it's allname:*and variants. Onedrop_propertieswhich was only droppingnamewas converted todrop_namesinstead. Dropping these might (slightly) increase mergeability and will decrease the number of properties stored in the tile.