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

Double simplification tolerance on roads and boundaries layers. #1718

Merged

Conversation

zerebubuth
Copy link
Member

@zerebubuth zerebubuth commented Dec 5, 2018

This doubles the simplification tolerance from 1 to 2 on roads and boundary layers.

I was expecting noticeable results, but the images look visually extremely similar, and the tile size changes are down in the noise:

Nominal zoom Average tile size - master Average tile size- branch  Reduction
7 34870 34828 0.12%
8 36496 36482 0.04%
9 56668 56648 0.04%
10 112210 112207 0.00%
11 198046 197988 0.03%
12 168503 168518 -0.01%
13 105679 105681 0.00%
14 78279 78279 0.00%
15 76124 76125 0.00%

Connects to #641.

Here's a zoom sequence over New York (don't worry about the missing data - my local DB is currently loaded with only NY & NJ). Left is current master, right is this PR/branch.

Zoom 15

Zoom 14

Zoom 13

Zoom 12

Zoom 11

Zoom 10

Zoom 9

Zoom 8

Zoom 7

…ere the tolerance was being ignored, it made a huge increase in the buildings layer size, so I've reset it to the default.
@zerebubuth
Copy link
Member Author

I thought this was a little bit too small of an effect, so I dug into it a bit more. Turns out that the simplification tolerance was being completely ignored due to a bug. Once that's merged, we should see results like:

Nominal zoom Average tile size - master Average tile size - branch  Reduction
7 34870 34836 0.10%
8 36496 36364 0.36%
9 56668 56643 0.04%
10 112210 111876 0.30%
11 198046 197314 0.37%
12 168503 166557 1.15%
13 105679 105095 0.55%
14 78279 77731 0.70%
15 76124 75819 0.40%

Still not huge, but we're out of the realm of mere noise for most zoom levels now.

@zerebubuth
Copy link
Member Author

Here's what the latest code (d66f438) looks like:

Zoom 15

branch2-z15

Zoom 14

branch2-z14

Zoom 13

branch2-z13

Zoom 12

branch2-z12

Zoom 11

branch2-z11

Zoom 10

branch2-z10

Zoom 9

branch2-z09

Zoom 8

branch2-z08

Zoom 7

branch2-z07

Copy link
Member

@nvkelso nvkelso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed IRL, looks good.

@zerebubuth zerebubuth merged commit 6af028c into master Dec 6, 2018
@zerebubuth zerebubuth deleted the zerebubuth/641-more-roads-boundary-simplification branch December 6, 2018 11:06
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