Building merging #1689

zerebubuth · 2018-10-17T13:58:11Z

Connects to #1686.

Better building merging, resulting in fewer polygons at zooms 14 & 15 (nominal). In summary, the buildings layer size is drastically reduced:

Tile coordinate (512 size)	Size before (bytes)	Size after (bytes)	Reduction
`13/3033/4647`	442,333	2,950	99.3%
`14/6067/9294`	587,743	67,008	88.6%
`15/12134/18589`	384,337	234,313	39.0%

Zoom 14

Ignore the grey area to the south and west; that's just because I didn't render those tiles. The visual output is very similar to the style before, as we are now dropping buildings which weren't rendered anyway.

Zoom 15

At this zoom, we're now doing much more merging, and in most dense areas we now merge into single "city block" polygons.

Zoom 16

Zoom 16 is almost unfiltered, but we do now drop a few of the very small polygons.

Changes

Building size selections for zoom now more closely match the criteria in the style.
Merging was previously done by slice, which effectively was a random subset of the polygons. This is now done by quad tree decomposition, so nearby polygons are grouped together and are much more likely to merge.
Building heights are now quantized at 10m intervals rather than 5m at zooms 14 & 15, and at 20m intervals for zoom 13.
We drop more properties on buildings (short_name, osm_relation) to enable better merging.
We drop small courtyards and other inners within buildings.

Overall, this leads to much less detail in the buildings layer, but this detail isn't really visible at zooms 14 and 15 anyway. The plus side is that we now include many more "city block" polygons to give better visual texture to cities.

nvkelso · 2018-10-17T14:12:14Z

Nice! Huge file savings :) For zoom 15 can I see a variant where we don’t combine the city buildings in a “block”? I do like the effect, but it tends to deemphasize the real world large buildings. Are the real big buildings still notes so they could be called out? For zoom 16 historically we didn’t do dropping at this max zoom, can you add details on this change? I’m curious how it effects the flower sculpture building near the ferry building in SF and tiny backyard sheds in Daily City south of SF.

zerebubuth · 2018-10-17T14:39:53Z

For zoom 15 can I see a variant where we don’t combine the city buildings in a “block”?

Sure! But I'm not sure what you want to see. Some options:

A version where we do no merging at all?
A version where we merge without buffering?
A version where we quantize the heights at finer granularity (which will prevent some merging)?

I do like the effect, but it tends to deemphasize the real world large buildings. Are the real big buildings still notes so they could be called out?

Personally, I think that's a good trade-off for getting a better visual sense of the building density.

Perhaps we could de-emphasise the "city blocks" in the style - for example, to use a less contrasting outline colour for buildings less than 50m in height or similar. Here's the view at zoom 15.9, and the tall buildings still stand out a lot from the background.

For zoom 16 historically we didn’t do dropping at this max zoom, can you add details on this change?

The changes should only have affected the min_zoom allocation, and in fact should result in more stuff at z16 instead of z17:

   - &z16_area_volume
-      any:
-        way_area: { min: 25 }
-        volume: { min: 50000 }
+      way_area: { min: 10 }

I'll go and check why there was a change in size at z16 - I wonder if perhaps I did the old/new test with different dates.

nvkelso · 2018-10-17T15:03:19Z

Ah, that’s a good tip Re 50 meter height, that’s probably enough - good demo image! I’d probably go farther and remove extrusion on those entirely and stack under roads and then it’s fine client side. Can you add screenshot sequences for SF and DC and Paris, please? They have different urban patterns. I wonder if for zoom 15 we should do larger dilation on the buffer to reduce exterior details, and use large px value for dropper inners?

rmarianski

Put in some questions/comments, but it looks good to me!

rmarianski · 2018-10-17T14:57:24Z

queries.yaml

+        13: vectordatasource.transform.quantize_height_round_nearest_20_meters
+        14: vectordatasource.transform.quantize_height_round_nearest_10_meters
+        15: vectordatasource.transform.quantize_height_round_nearest_10_meters
+      # todo: keep instead of drop?


I think keep/drop just depends on what's easier. IIRC the list of properties that are included on buildings are done so via a whitelist type of mechanism anyway, ie new properties won't suddenly appear to prevent merging, right? But in terms of what side we want to err on, a whitelist feels safer in that I think we'd prefer to merge if we forgot to update this list if we added another property, rather than stop merging because a new property was added. Anyway, just my thinking on this, either way feels fine to me.

rmarianski · 2018-10-17T15:02:08Z

queries.yaml

+      # NOTE: max_merged_features is set to keep the time taken for geometry
+      # merging down, as it seems to go up with the square of the number of
+      # features.
+      max_merged_features: 2000


Wouldn't this mean that the size of the tile would stay larger than we'd like though? And would we want to try another strategy in these cases, like drop some buildings from even appearing that don't rank high enough (not sure this is possible), or increase the buffer size we are using? Maybe it's smarter to leave it and just track this somehow so after runs we can see which tiles triggered the max?

rmarianski · 2018-10-17T15:04:14Z

queries.yaml

+    params:
+      end_zoom: 15
+      source_layers: [buildings]
+      pixel_area: 0.25


Great idea!

How is this different than line 962? We should only do this procedure once.

rmarianski · 2018-10-17T15:04:58Z

queries.yaml

+        path: spreadsheets/scale_rank/buildings.csv
+    params:
+      source_layer: buildings
+      target_value_type: int


Nice catch here.

rmarianski · 2018-10-17T15:09:21Z

queries.yaml

+      source_layer: buildings
+      start_zoom: 14
+      end_zoom: 15
+      where: scale_rank > 3


This makes sense to drop afterwards, but is it crazy to initially drop some of the smaller buildings too to cut down on the candidates we need to consider for merging? Or is this actually counterproductive because it might cut down on the merging that we can actually do? Does the drop smaller inners handle this well enough anyway?

It's more "coherent" to operate on the entire urban fabric... but agree it can result in large aggregate polygons that would otherwise have been dropped as smaller inputs. Generally they'll still have low height and can be filtered client-side, or we could later drop them server-side if we need more file size savings.

rmarianski · 2018-10-17T15:09:35Z

vectordatasource/transform.py

+    # round the smallest values up to the first step.
+    if val < step:
+        return int(step)
+


Great catch!

rmarianski · 2018-10-17T15:17:10Z

vectordatasource/transform.py

+    assert isinstance(merger, RecursiveMerger)
+
+    # don't keep recursing. if we haven't been able to get to a smaller number
+    # of shaped by 5 levels down, then perhaps there are particularly large


# of shapes

Good catch of typo

Yup, well spotted, thanks! Fixed in 937023c.

rmarianski · 2018-10-17T15:27:43Z

vectordatasource/transform.py

+    # this formula seems to give a good balance between larger values, which
+    # merge more but can merge everything into a blob if too large, and small
+    # values which retain detail.
+    tolerance = min(5, 0.4 * tolerance_for_zoom(zoom))


bcamper · 2018-10-17T16:21:56Z

Excited to see more work here!

What's up with the (over-)simplification on that z15.9 screen? Existing or unrelated?

And I'm sorry you remind me again about "nominal" zoom :) Is that the "display" zoom, or the underlying 512px tile zoom?

My main interest here is ensuring that we have the full detail (for some definition) at the max tile zoom (sounds like that is addressed in comments).

But to confirm, this would drastically reduce the buildings in a z14 view like this, right?
https://tangrams.github.io/tangram/#14.080000000000013/40.669452032003285/-73.97878168006201

nvkelso · 2018-10-17T17:56:08Z

What's up with the (over-)simplification on that z15.9 screen? Existing or unrelated?

Oops. That's an new error we should fix in this PR.

But to confirm, this would drastically reduce the buildings in a z14 view like this, right?

Yes, it's just too much data and file size at zoom 14, even after doing the generalization we see in zoom 15 example by combining the buildings in a city block.

zerebubuth · 2018-10-17T19:07:43Z

What's up with the (over-)simplification on that z15.9 screen? Existing or unrelated?

I think that's partly existing, although highlighted by the larger number of visible buildings now:

However, there are some differences: for example, the two "H" shaped buildings above and to the right of the bend in Avenida Professor Alfonso Bovero (above where "Bovero" is in the right image). The new tile has a mangled version and the master version looks better. There are a bunch of places where they look about as bad as eachother, though.

bcamper · 2018-10-17T21:06:29Z

Yep, I've liked having higher building density available (and I think this is still considerably more at z15 than Mapbox?), but totally agree tile size needs to come down for typical base map cases (though may indicate the future value of a couple different tile flavors with more/less feature density, or you know, true "raw" tiles...). I am interested to also see if there is a noticeable improvement in tile build time from reduced feature processing (even in the cases where they are not displayed)! Let me know when an endpoint is available :)

zerebubuth · 2018-10-23T10:00:39Z

I re-ran the last run with a clean working copy. Looks like I must have had some local mods I'd forgotten about, because the results are different. (Although I'm going to check again, to make sure this is reproducible).

I'll also do some runs in SF, Washington DC & Paris.

Tile coordinate (512 size)	Size before (bytes)	Size after (bytes)	Reduction
`13/3033/4647`	442,333	3,012	99.3%
`14/6067/9294`	587,743	71,634	87.8%
`15/12134/18589`	384,337	388,012	-1.0% (i.e: it's larger)

Zoom 14

Zoom 15

Zoom 16

zerebubuth · 2018-10-24T10:04:25Z

San Francisco

San Francisco does pretty well. Like São Paulo, there's a lot of "blocks" which can be merged at zoom 15 nominal.

Overall results:

Tile coordinate (512 size)	Nominal zoom	Size before (bytes)	Size after (bytes)	Reduction
`13/1310/3165`	14	136,234	12,221	91%
`14/2620/6331`	15	368,384	52,801	86%
`15/5241/12663`	16	154,484	154,410	0%

Zoom 14

Zoom 15

Zoom 16

Washington DC

Downtown DC has a lot of big buildings, most of which don't need any kind of merging. Outside those blocks of large buildings, the other blocks also seem to merge well, until we reach the suburbs.

Overall results:

Tile coordinate (512 size)	Nominal zoom	Size before (bytes)	Size after (bytes)	Reduction
`13/2343/3134`	14	65,353	8,357	87%
`14/4687/6268`	15	189,200	24,003	87%
`15/9375/12536`	16	235,878	237,319	-1%

Zoom 14

Zoom 15

Zoom 16

Georgetown Interlude

Just outside DC, in the evocatively named Foxhall Village, there are a lot of detached houses, which don't get merged at all. However, it looks like we're still getting a decent reduction in size (about 80% rather than 90%, but coming from a lower base), perhaps due to more aggressive geometry simplification.

The lack of individual large buildings means the buildings layer for this tile is almost empty at zoom 14 nominal.

Overall results:

Tile coordinate (512 size)	Nominal zoom	Size before (bytes)	Size after (bytes)	Reduction
`13/2341/3133`	14	90,342	1,981	98%
`14/4683/6266`	15	96,979	20,881	78%
`15/9367/12533`	16	82,509	82,482	0%

Zoom 15

Zoom 16

Paris

Paris seems to have a lot of buildings with inner courtyards or other non-rectangular features which result in many inners in the merged blocks at zoom 15 nominal. Perhaps the tolerance for dropping small inners needs to be increased to remove most of these?

Overall results:

Tile coordinate (512 size)	Nominal zoom	Size before (bytes)	Size after (bytes)	Reduction
`13/4149/2817`	14	253,303	17,329	93%
`14/8298/5635`	15	273,873	62,551	77%
`15/16597/11271`	16	207,522	208,308	0%

Zoom 14

Zoom 15

Zoom 16

nvkelso · 2018-10-29T16:16:24Z

Overall this feels like a good improvement and I think we should go forward with it as-is... after confirming we're not regressing zoom 16 as the Ferry Building in SF is missing it's tower and DC buildings look generalized... and file a followup issue to drop small courtyards at zoom 15.

For Brazil zoom 15, it still feels like we're cluttering up the tile with "fake" low rise buildings:

In San Francisco also at zoom 15: I see some merging... but overall too many small features:

At zoom 16 in SF we seem to have lost the tower detail on the Ferry Building right along the water?! Please ensure there's not an off-by-one problem here before merging.

In DC at zoom 15 it is aggregating row houses like I'd expect, so that's a vote to keep it as proposed in this PR:

Zoom 16 in DC also feels over generalized, did that change in this PR?

Zoom 15 in Georgetown does look nice!

Zoom 15 in Paris feels like we could stand dropping small inner courtyards, but otherwise looks good:

nvkelso · 2018-10-29T16:18:41Z

queries.yaml

+    params:
+      end_zoom: 15
+      source_layers: [buildings]
+      pixel_area: 0.25


Could make this 0.5 or 1? But let's track that with a new issue and merge this as-is.

nvkelso · 2018-10-29T16:24:50Z

File followup issue for larger small inner courtyards threshold
Investigate if we're dropping small inner courtyards twice now
Fix typo Rob pointed out

zerebubuth · 2018-10-29T18:56:59Z

Investigate if we're dropping small inner courtyards twice now

We are, deliberately. We have one pass of dropping inners before the merge and another after the merge. The first is just to make life simpler for the merge; some things which were multipolygons are now simple polygons, which is much easier (and computationally cheaper) for the dilate / buffer algorithm to handle. The pass after merging is to because the merging process itself sometimes creates courtyards when merging adjacent buildings (e.g: neighbouring buildings like [] get merged to □).

Instead of dropping small buildings and being left with a lot of individual buildings to merge, instead keep the small buildings and try to merge into city blocks by dropping more properties and quantizing height. Small gaps between buildings and internal courtyards are also dropped, leaving larger individual polygons.

… Refactor feature merging to support different ops at different parts of the hierarchical merge. This isn't used yet - but will be useful when we want to merge between blocks.

…similar and avoid blobbyness.

nvkelso · 2018-10-29T19:04:17Z

And a closer read of the comments say that, sorry for the noise!

zerebubuth · 2018-10-29T19:06:34Z

Follow-up for a re-visit to drop more courtyards in #1696.

zerebubuth · 2018-10-30T11:54:17Z

PS: We also talked about the Ferry Building, and how the tower isn't visible in the new tiles. This is because the building footprint has height=83.1, which is the height of the whole building including the tower, so it covers it up. The building used to have height=16.1, but this has been more recently edited to split it into a building:part with that height.

We've seen a similar issue before with the Fernsehturm in Berlin #1274 and a sort-of similar issue with the Bank of England #1032. And it looks like the plan would be to add the root_id on the building as well, and strip off the height. Although we might want to check first to see that the building:parts cover the footprint of the building.

Not adding a follow-up for this, as I think #1274 has it covered.

zerebubuth requested a review from rmarianski October 17, 2018 13:58

rmarianski approved these changes Oct 17, 2018

View reviewed changes

nvkelso approved these changes Oct 29, 2018

View reviewed changes

nvkelso reviewed Oct 29, 2018

View reviewed changes

zerebubuth added 6 commits October 29, 2018 18:57

Make building selection criteria match those used in the style sheet.…

4e0cbb9

… Refactor feature merging to support different ops at different parts of the hierarchical merge. This isn't used yet - but will be useful when we want to merge between blocks.

Fix tests. Adjust mitre style of buffer for buildings to keep angles …

f880593

…similar and avoid blobbyness.

Rewrite test so that it doesn't derail the --download-only test mode.

bee127f

Revert incorrect test update.

ca759c0

Fix typo: shaped -> shapes.

2ea6a68

zerebubuth force-pushed the zerebubuth/1686-building-merging branch from 937023c to 2ea6a68 Compare October 29, 2018 18:57

zerebubuth merged commit 981fe3c into master Oct 29, 2018

zerebubuth deleted the zerebubuth/1686-building-merging branch October 29, 2018 19:06

nvkelso mentioned this pull request Dec 14, 2018

Not enough building merging & simplification #1686

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Building merging #1689

Building merging #1689

zerebubuth commented Oct 17, 2018 •

edited by nvkelso

Loading

nvkelso commented Oct 17, 2018 via email

zerebubuth commented Oct 17, 2018

nvkelso commented Oct 17, 2018 via email

rmarianski left a comment

rmarianski Oct 17, 2018

rmarianski Oct 17, 2018

rmarianski Oct 17, 2018

nvkelso Oct 29, 2018

rmarianski Oct 17, 2018

rmarianski Oct 17, 2018

nvkelso Oct 29, 2018

rmarianski Oct 17, 2018

rmarianski Oct 17, 2018

nvkelso Oct 29, 2018

zerebubuth Oct 29, 2018

rmarianski Oct 17, 2018

bcamper commented Oct 17, 2018

nvkelso commented Oct 17, 2018

zerebubuth commented Oct 17, 2018

bcamper commented Oct 17, 2018

zerebubuth commented Oct 23, 2018

zerebubuth commented Oct 24, 2018

nvkelso commented Oct 29, 2018

nvkelso Oct 29, 2018

nvkelso commented Oct 29, 2018

zerebubuth commented Oct 29, 2018

nvkelso commented Oct 29, 2018

zerebubuth commented Oct 29, 2018

zerebubuth commented Oct 30, 2018

Building merging #1689

Building merging #1689

Conversation

zerebubuth commented Oct 17, 2018 • edited by nvkelso Loading

Zoom 14

Zoom 15

Zoom 16

Changes

nvkelso commented Oct 17, 2018 via email

zerebubuth commented Oct 17, 2018

nvkelso commented Oct 17, 2018 via email

rmarianski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcamper commented Oct 17, 2018

nvkelso commented Oct 17, 2018

zerebubuth commented Oct 17, 2018

bcamper commented Oct 17, 2018

zerebubuth commented Oct 23, 2018

Zoom 14

Zoom 15

Zoom 16

zerebubuth commented Oct 24, 2018

San Francisco

Zoom 14

Zoom 15

Zoom 16

Washington DC

Zoom 14

Zoom 15

Zoom 16

Georgetown Interlude

Zoom 15

Zoom 16

Paris

Zoom 14

Zoom 15

Zoom 16

nvkelso commented Oct 29, 2018

Choose a reason for hiding this comment

nvkelso commented Oct 29, 2018

zerebubuth commented Oct 29, 2018

nvkelso commented Oct 29, 2018

zerebubuth commented Oct 29, 2018

zerebubuth commented Oct 30, 2018

zerebubuth commented Oct 17, 2018 •

edited by nvkelso

Loading