aspectumapp / osm2geojson

Convert OSM and Overpass XML/JSON to GeoJSON
MIT License
100 stars 14 forks source link

Counter-clockwise winding order for MultiPolygons #34

Closed dericke closed 2 years ago

dericke commented 2 years ago

Currently, MultiPolygons are being output with winding order opposite of shapely.geometry.polygon.orient's default, which means Multi/Polygons will have a clockwise winding order, rather than the counter-clockwise winding order called for by the GeoJSON spec:

A linear ring MUST follow the right-hand rule with respect to the area it bounds, i.e., exterior rings are counterclockwise, and holes are clockwise.

https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.6

Although the spec says parsers shouldn't reject Polygons that don't follow this, I know MapRoulette, for one, will reject (Multi)Polygons if they do not follow the right-hand rule. Unless there's a reason I'm missing for outputting shapes counter to the spec, I think it would save work for most users if shapes were output with counter-clockwise winding order.

dericke commented 2 years ago

Of course, I forgot to update the tests.

dericke commented 2 years ago

Following on from

yes, I need more details to understand problem with right-hand rule. Maybe this line can be helpful for you (unfortunately I don't remember the purpose of it) https://github.com/aspectumapp/osm2geojson/blob/master/osm2geojson/main.py#L222

Originally posted by @rapkin in https://github.com/aspectumapp/osm2geojson/issues/33#issuecomment-1108876387

I think it's useful to orient the polygons because most areas in OSM can have their coordinates in either direction, but many GeoJSON consumers (for example, MapRoulette) will expect the coordinates to follow the right-hand rule (many others don't seem to care, for example QGIS). I can't speak to why the polygons would have been oriented to the opposite direction up to this point, however.

rapkin commented 2 years ago

Just verify that you can render those (changed) geojson files with QGIS or https://geojson.io/

dericke commented 2 years ago

While updating the tests, I discovered that QGIS and PostGIS define the right-hand rule to mean the opposite from what the GeoJSON spec means. So the QGIS function called "Force right-hand-rule" will set polygons to have clockwise exterior rings, counter-clockwise interior rings, etc. That is the likely reason the code currently passes the reverse argument to shapely.geometry.polygon.orient.

QGIS and PostGIS have both started migrating to use the terminology of "Counter-clockwise" and "clockwise" in place of "right-hand rule", to avoid confusion. After modifying the test data to match the GeoJSON spec, I confirmed that they still render without issue in QGIS.

Though there are conflicting definitions, I think it's best that we conform to the GeoJSON spec, and so I still think this PR is a good idea.

rapkin commented 2 years ago

Thank you for your work!