Update for multi-polygon compliance with spec v1.0.2

[bcamper]

The upcoming spec v1.0.2 clarifies how multi-polygons should be encoded: all polygons should be "flattened" into one series of rings, with the winding order of the rings used to determine which are interior/exterior. An exterior ring indicates the start of a new polygon.

I believe this differs from our current handling of multi-polygons, where we "exploded" each polygon into its own feature, and copied the properties of each (as there was no official handling of multi-polygons previously). We should update to be compliant. Tangram already handles the new multi-polygon encoding so no client changes are needed on that side (style rules should be compatible with either method).

mapbox/vector-tile-spec#36

[nvkelso]

@bcamper Will this change work with older v0.3 and earlier Tangrams or does it require the new v0.4 release?

[bcamper]

We should test it but as far as I am aware it should not require any Tangram changes. For decoding MVT tiles we use Mapbox's own JS library. Related, for non-tiled datasources, we do the same poly/ring detection on data generated by Mapbox's geojson-vt (which also outputs multi-polygons in this flattened format), see https://github.com/tangrams/tangram/blob/master/src/sources/geojson.js#L159.

[rmarianski]

This fix didn't end up getting deployed with the latest update. I added tickets to the appropriate chef repos to ensure we get the latest version of mapbox-vector-tile deployed in the future.

tilezen/chef-tilequeue#12
tilezen/chef-tileserver#1

[nvkelso]

Once deployed, how can we verify the fix?

[bcamper]

We should identify a few multipolygon features and compare before/after to make sure they have not changed. I believe The Lake in Central Park is one (we had issues with it previously). Any good candidates?

[nvkelso]

@rmarianski What further testing besides #402 do we need here?

[rmarianski]

@nvkelso can we point an older version of tangram at the dev vector tiles to see if it looks ok?

[bcamper]

I don't believe it's Tangram version dependent...?

On Wed, Nov 11, 2015 at 4:23 PM, Robert Marianski [email protected]
wrote:

@nvkelso https://github.com/nvkelso can we point an older version of
tangram at the dev vector tiles to see if it looks ok?

—
Reply to this email directly or view it on GitHub
#20 (comment)
.

[rmarianski]

Did the decoding library version in tangram change at some point? It sounded like there was some question as to whether this change would impact older versions?

If not, then we should be ok.

[bcamper]

The decoding library is Mapbox's, and should be compatible with old and new encoding methods. If we want to verify, all tangram releases are available by version number: mapzen.com/tangram/[version]/tangram.[min | debug].js

[bcamper]

I was wrong, the Mapbox decoding lib doesn't provide this! This is fixed in Tangram here, when we are ready to merge:

https://github.com/tangrams/tangram/compare/decode-multipolygon

[nvkelso]

@rmarianski Is this part of v0.8 work?

[rmarianski]

I vote for deploying with v0.8. All the work here is done and just needs to be deployed.

We've verified that the new winding order looks to be correct in dev now, as well as the issue with rounding vs truncating not making things worse in tangram.

[nvkelso]

Cool, moving into v0.8

[nvkelso]

Over to @bcamper to confirm this works to spec before taking this to prod.

[bcamper]

Verified in dev!