From 8bce1bdf1e981c22e64b5cdc72e4821dc34da568 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 1 Dec 2017 17:51:44 +0000 Subject: [PATCH 1/4] Clip buildings to tile boundaries. 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. --- integration-test/197-clip-buildings.py | 35 +++++++++++++++----------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/integration-test/197-clip-buildings.py b/integration-test/197-clip-buildings.py index 964a25b54..549c68426 100644 --- a/integration-test/197-clip-buildings.py +++ b/integration-test/197-clip-buildings.py @@ -4,39 +4,46 @@ class ClipBuildings(FixtureTest): def test_high_line(self): + from ModestMaps.Core import Coordinate + from shapely.geometry import box from shapely.geometry import shape + from tilequeue.tile import coord_to_mercator_bounds # this is mid way along the High Line in NYC, which is a huge long # "building". we should be clipping it to the bounds of the tile. # # NOTE: we _don't_ clip the fixture, that has to happen in the # query. + # + # NOTE: https://github.com/tilezen/vector-datasource/issues/1142 + # we want to clip all buildings to the bounding box of the tile, so + # that there are no overlaps. self.load_fixtures([ 'http://www.openstreetmap.org/relation/7141751', ]) - with self.features_in_tile_layer(16, 19295, 24631, 'buildings') \ - as buildings: - # max width and height in mercator meters as the size of the - # above tile - max_w = 611.5 - max_h = 611.5 + coord = Coordinate(zoom=16, column=19295, row=24631) + with self.features_in_tile_layer( + coord.zoom, coord.column, coord.row, 'buildings') as buildings: + # tile bounds as a box + tile_bounds = coord_to_mercator_bounds(coord) + bbox = box(*tile_bounds) # need to check that we at least saw the high line saw_the_high_line = False for building in buildings: - bounds = shape(building['geometry']).bounds - w = bounds[2] - bounds[0] - h = bounds[3] - bounds[1] + building_bounds = shape(building['geometry']).bounds + building_box = box(*building_bounds) if building['properties']['id'] == -7141751: saw_the_high_line = True - self.assertFalse( - w > max_w or h > max_h, - 'feature %r is %rx%r, larger than the allowed ' - '%rx%r.' % (building['properties']['id'], w, h, - max_w, max_h)) + self.assertTrue( + building_box.within(bbox), + 'feature %r extends outside of the bounds of the ' + 'tile (%r not within %r).' % + (building['properties']['id'], building_bounds, + tile_bounds)) self.assertTrue( saw_the_high_line, From 3f96f8ce9154ff1371cd8ffcd4398d93e1a979d8 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 1 Dec 2017 17:57:48 +0000 Subject: [PATCH 2/4] Added a note about building clipping to the documentation. --- docs/layers.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/layers.md b/docs/layers.md index 2815a1543..0a0bea783 100644 --- a/docs/layers.md +++ b/docs/layers.md @@ -194,6 +194,8 @@ Label position points may also have `closed` or `historical` kind_detail values Values for `kind_detail` are sourced from OpenStreetMap's `building` tag for building footprints and from `building:part` tag for building parts. +Note that building geometries, like most geometries in Tilezen tiles, are clipped to the bounds of the tile, even if the building extends beyond the tile. This means that it might be necessary to assemble geometry from several neighbouring tiles to recreate the full building. Some buildings are exceptionally large and span many tiles, so this can be tricky. + #### Building properties (common): * `name` From 7cc4d1b64a068dc1c19ee2bf05d891caf0c16940 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Tue, 5 Dec 2017 17:35:02 +0000 Subject: [PATCH 3/4] Add test for #1226. 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. --- .../1226-landuse-kind-buildings.py | 73 +++++++++++++++++++ integration-test/dsl.py | 7 ++ 2 files changed, 80 insertions(+) create mode 100644 integration-test/1226-landuse-kind-buildings.py diff --git a/integration-test/1226-landuse-kind-buildings.py b/integration-test/1226-landuse-kind-buildings.py new file mode 100644 index 000000000..36a464c5c --- /dev/null +++ b/integration-test/1226-landuse-kind-buildings.py @@ -0,0 +1,73 @@ +from . import FixtureTest + + +class LanduseKindBuildings(FixtureTest): + def test_lax_airport_terminals(self): + self.load_fixtures([ + # Terminal 2, which had landuse_kind set correctly at z14 + 'https://www.openstreetmap.org/way/413226245', + + # Tom Bradley International Terminal was missing landuse_kind + 'https://www.openstreetmap.org/relation/6168071', + + # the aerodrome landuse polygon + 'https://www.openstreetmap.org/way/131190417', + ]) + + # the tile which contains Terminal 2 should set the landuse_kind + # correctly. + self.assert_has_feature( + 14, 2803, 6547, 'buildings', + {'id': 413226245, 'kind': 'building', 'landuse_kind': 'aerodrome'}) + + # the four tiles containing bits of Tom Bradley International should + # do so also. + for x in (2802, 2803): + for y in (6547, 6548): + self.assert_has_feature( + 14, x, y, 'buildings', + {'id': -6168071, 'kind': 'building', + 'landuse_kind': 'aerodrome'}) + + def test_lax_united_airlines_building(self): + self.load_fixtures([ + # United Airlines building spans two tiles at z16, and neither(?) + # of them were getting a landuse_kind assigned. + 'https://www.openstreetmap.org/relation/6168075', + + # the aerodrome landuse polygon + 'https://www.openstreetmap.org/way/131190417', + ]) + + for x in (11209, 11210): + self.assert_has_feature( + 16, x, 26192, 'buildings', + {'id': -6168075, 'kind': 'building', + 'landuse_kind': 'aerodrome'}) + + def test_building_part(self): + import dsl + from tilequeue.tile import coord_to_bounds + from shapely.geometry import box + from ModestMaps.Core import Coordinate + + z, x, y = (16, 0, 0) + bounds = coord_to_bounds(Coordinate(zoom=z, column=x, row=y)) + shape = box(*bounds) + + self.generate_fixtures( + dsl.way(1, shape, + {'landuse': 'park', 'source': 'openstreetmap.org'}), + dsl.way(2, shape, + {'building': 'yes', 'source': 'openstreetmap.org'}), + dsl.way(3, shape, + {'building:part': 'yes', 'source': 'openstreetmap.org'}), + ) + + self.assert_has_feature( + z, x, y, 'buildings', + {'id': 2, 'kind': 'building', 'landuse_kind': 'park'}) + import pdb; pdb.set_trace() + self.assert_has_feature( + z, x, y, 'buildings', + {'id': 3, 'kind': 'building_part', 'landuse_kind': 'park'}) diff --git a/integration-test/dsl.py b/integration-test/dsl.py index 72875f2a8..1499ef2db 100644 --- a/integration-test/dsl.py +++ b/integration-test/dsl.py @@ -13,6 +13,13 @@ def point(id, position, tags): return Feature(id, Point(x, y), tags) +# utility wrapper to generate a Feature from a lat/lon shape. +def way(id, shape, tags): + from shapely.ops import transform + merc_shape = transform(reproject_lnglat_to_mercator, shape) + return Feature(id, merc_shape, tags) + + # the fixture code expects "raw" relations as if they come straight from # osm2pgsql. the structure is a little cumbersome, so this utility function # constructs it from a more readable function call. From ff2a866218758b7d9ca19c81ba3d573698ff64f9 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Tue, 5 Dec 2017 18:01:58 +0000 Subject: [PATCH 4/4] D'oh! Remove debugging code. --- integration-test/1226-landuse-kind-buildings.py | 1 - 1 file changed, 1 deletion(-) diff --git a/integration-test/1226-landuse-kind-buildings.py b/integration-test/1226-landuse-kind-buildings.py index 36a464c5c..d542dbc89 100644 --- a/integration-test/1226-landuse-kind-buildings.py +++ b/integration-test/1226-landuse-kind-buildings.py @@ -67,7 +67,6 @@ def test_building_part(self): self.assert_has_feature( z, x, y, 'buildings', {'id': 2, 'kind': 'building', 'landuse_kind': 'park'}) - import pdb; pdb.set_trace() self.assert_has_feature( z, x, y, 'buildings', {'id': 3, 'kind': 'building_part', 'landuse_kind': 'park'})