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

d3.geoStitch may promote polygon holes to exterior rings. #108

Open
mbostock opened this issue Apr 8, 2017 · 2 comments
Open

d3.geoStitch may promote polygon holes to exterior rings. #108

mbostock opened this issue Apr 8, 2017 · 2 comments
Assignees

Comments

@mbostock
Copy link
Member

mbostock commented Apr 8, 2017

Consider the case of two polygons in a MultiPolygon, each with their own distinct hole, where the line BC is on the antimeridian:

A-----------------B-----------------J
|                 |                 |
|                 |                 |
|     E-----F     |     K-----L     |
|     |     |     |     |     |     |
|     |     |     |     |     |     |
|     H-----G     |     N-----M     |
|                 |                 |
|                 |                 |
D-----------------C-----------------I

The first polygon starts with these fragments:

A-----------------B
|                  
|                  
|     E-----F      
|     |     |      
|     |     |      
|     H-----G      
|                  
|                  
D-----------------C

The second polygon starts with these fragments:

B-----------------J
                  |
                  |
      K-----L     |
      |     |     |
      |     |     |
      N-----M     |
                  |
                  |
C-----------------I

When ring DBCA is merged with CIJB, I believe that the first polygon retains ring EFGH. However, the other ring, KLMN, is considered a “standalone ring” not belonging to any exterior ring, and thus two polygons result from stitching:

A-----------------B-----------------J
|                                   |
|                                   |
|     E-----F                       |
|     |     |                       |
|     |     |                       |
|     H-----G                       |
|                                   |
|                                   |
D-----------------C-----------------I

K-----L
|     |
|     |
N-----M

Furthermore, since KLMN has the opposite winding order, it now covers most of the sphere rather than just be a hole inside BADCIJ.

This problem can be avoided by putting every ring in a MultiPolygon into a single Polygon, as demonstrated here:

https://bl.ocks.org/mbostock/83c0be21dba7602ee14982b020b12f51

However, collapsing a Polygon into a MultiPolygon isn’t valid GeoJSON, since it means that the Polygon may have multiple exterior rings. This only works because d3-geo appears to not be dependent on GeoJSON ring semantics, at least for rendering, relying only on winding order.

@mbostock mbostock self-assigned this Apr 8, 2017
@Fil
Copy link
Member

Fil commented May 3, 2018

Your example at https://beta.observablehq.com/@fil/stitching-multipolygons

It seems that d3.geoProject() has the same issue, where MultiPolygons will "explode" and must be transformed back to polygons. (See https://beta.observablehq.com/@fil/d3-vector-tiles-wip#res).

In geoProject the choice of having a ring be a polygon or a hole is made with https://github.com/d3/d3-geo-projection/blob/master/src/project/clockwise.js, which is definitely planar.

[Addendum] https://github.com/d3/d3-geo-projection/blob/master/src/project/index.js#L114 lists issue 1558 which has disappeared, and explains a lot https://webcache.googleusercontent.com/search?q=cache:5UIKWV7HNFYJ:https://github.com/d3/d3/issues/1558

More later…

@Fil
Copy link
Member

Fil commented Aug 19, 2018

see also #146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants