d3 / d3-geo-projection

Extended geographic projections for d3-geo.
https://observablehq.com/collection/@d3/d3-geo-projection
Other
1.1k stars 201 forks source link

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

Open mbostock opened 7 years ago

mbostock commented 7 years ago

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.

Fil commented 6 years ago

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 commented 6 years ago

see also https://github.com/d3/d3-geo-projection/pull/146