Skip to content

Clip buildings to tiles.#1446

Merged
zerebubuth merged 5 commits intomasterfrom
zerebubuth/1142-buildings-clipping
Dec 14, 2017
Merged

Clip buildings to tiles.#1446
zerebubuth merged 5 commits intomasterfrom
zerebubuth/1142-buildings-clipping

Conversation

@zerebubuth
Copy link
Copy Markdown
Member

Looks like this was done already as part of #1352, so this PR just updates the test to have the behaviour that we want; that all building geometry is within the tile bounds.

Fixes #1142.

Matt Amos added 2 commits December 1, 2017 17:51
Looks like this was done already as part of #1352, so this PR just updates the test to have the behaviour that we want; that all building geometry is within the tile bounds.

Fixes #1142.
@nvkelso
Copy link
Copy Markdown
Member

nvkelso commented Dec 1, 2017

This is not obvious, but if you follow all the issue references, we're hoping the building clip solves the landuse_kind intercut problems in #1226. Can you add tests in this PR to prove that's true, please?

Copy link
Copy Markdown
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.

See comment.

@zerebubuth
Copy link
Copy Markdown
Member Author

This won't solve all the landuse_kind intercut problems, but it might make it better. I'll add some examples.

Just to be clear: We'll get different kinds of problems instead when buildings cross tile boundaries and one half gets assigned a different landuse_kind than the other. E.g:

image

@nvkelso
Copy link
Copy Markdown
Member

nvkelso commented Dec 1, 2017

Well, sorry to be the bearer of bad news but now #1226 is assigned to you, too ;)

Thanks for the illustration. I agree it's a related but different issue than this one.

Matt Amos added 3 commits December 5, 2017 16:03
This checks that buildings which cross tile boundaries are more likely to be assigned the correct landuse kind. The issue was that, before the change from query-per-layer to query-per-table, we were clipping the landuse to the tile but the building to a 3x3 tile area. This meant that large buildings had much less overlap with landuse polygons, and some would not be assigned a `landuse_kind`.

Now we clip all inputs (except water) to the tile, which solves this problem, but means that the same building in adjacent tiles could be assigned different `landuse_kind`s.

Additionally, it looks like `kind: building_part`s weren't being assigned a `landuse_kind`, although the test now passes, so something we changed has fixed that too.
@zerebubuth
Copy link
Copy Markdown
Member Author

@nvkelso how do those new tests look?

@zerebubuth
Copy link
Copy Markdown
Member Author

@nvkelso all good to merge?

@nvkelso
Copy link
Copy Markdown
Member

nvkelso commented Dec 14, 2017

Yes.

@zerebubuth zerebubuth merged commit 2bed573 into master Dec 14, 2017
@zerebubuth zerebubuth deleted the zerebubuth/1142-buildings-clipping branch December 14, 2017 10:04
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.

3 participants