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

Clip buildings to tiles. #1446

Merged
merged 5 commits into from
Dec 14, 2017
Merged

Conversation

zerebubuth
Copy link
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
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
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
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
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
Member Author

@nvkelso how do those new tests look?

@zerebubuth
Copy link
Member Author

@nvkelso all good to merge?

@nvkelso
Copy link
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