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

specify how multi geometries are encoded (or move to explicit enumeration values) #30

Closed
jfirebaugh opened this issue Feb 24, 2015 · 5 comments
Milestone

Comments

@jfirebaugh
Copy link
Contributor

The spec should say how MultiPoint, MultiLineString, and MultiPolygon geometries are to be encoded. As I understand it, currently:

  • A MultiPoint geometry is encoded using GeomType POINT and >1 MoveTo commands in the geometry field.
  • A MultiLineString geometry is encoded using GeomType LINESTRING and >1 MoveTo, LineTo* sequences in the geometry field.
  • A MultiPolygon geometry is encoded using GeomType POLYGON and >1 MoveTo, LineTo*, ClosePath sequences in the geometry field.

These should be documented. But they have the following deficiencies:

  • It's not possible to encode a MultiPoint geometry with a single point in a way that is distinguishable from a Point geometry. Thus VTs cannot round-trip GeoJSON or other formats in which a singleton MultiPoint is valid. The same is true for singleton MultiLineString and MultiPolygon.
  • It's not possible to reconstruct MultiPolygons without complex heuristics for determining inner/outer rings. (Ref Polygon winding order & holes vs outer rings mapnik-vector-tile#59)

For these reasons, I would rather see the spec adopt explicit MultiPoint, MultiLineString, and MultiPolygon types.

jfirebaugh referenced this issue in mapbox/vector-tile-js Feb 24, 2015
@jfirebaugh
Copy link
Contributor Author

I also discovered that mapnik will not generate any vector tile data at all for a MultiPoint geometry of zero points or a MultiLineString geometry of zero lines. It just omits the feature entirely.

jfirebaugh added a commit to mapbox/vector-tile-js that referenced this issue Feb 25, 2015
@hallahan
Copy link

hallahan commented Jun 3, 2015

One thing that just came up in Leaflet.MapboxVectorTile is that we don't have a defined way of how we should handle multiple polygons in a tile with the same ID. This is what happens when you encode a multi-polygon with Mapzen's encoder.

SpatialServer/Leaflet.MapboxVectorTile#24

I can have a set of polygons referenced by an ID, but it doesn't feel right to have a bunch of different polygons come through with the same ID.

@flippmoke flippmoke added this to the v2.0 milestone Nov 18, 2015
@flippmoke
Copy link
Member

In version 2 we specify the existence of MultiLinestring, MultiPoint, and MultiPolygon and note how they are determined. See pull request #39. However, we have not added them specifically to the enum. In future versions of the spec this could possible be included, but if we included it currently it would make v2 tiles incompatible with v1 decoders.

@joto
Copy link

joto commented Nov 19, 2015

How about this to get a migration path:
Include the MultiPoint, MultiLinestring, and MultiPolygon in the enum. Explicitly declare that version 2 compliant readers MUST handle those fields, but that version 2 compliant writers SHOULD NOT write to those fields, but use the Point, Linestring, Polygon enums for backwards compatibility. Document that in version 3 the rules for writers will change. Hopefully the readers have caught up by then.

@flippmoke
Copy link
Member

@joto Using the winding orders we are able to get to the point where we do not require a multipolygon and while I do think your idea is good if we want to have version 2 be backwards compatible with version 3 I have a feeling it will not likely be so! There are likely changes around encoding I think we would like to make that would change the structure and make version 2 not be able to read version 3 (no promises on any of this).

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

No branches or pull requests

4 participants