search

wdtinc / mapbox-vector-tile-java

Java Mapbox Vector Tile Library for Encoding/Decoding
Apache License 2.0
147 stars 73 forks source link

Polygon orientation #36

Closed exe-dealer closed 5 years ago

exe-dealer commented 5 years ago

It seems that rings orientation in polygons is wrong. The specs says that exterior ring must be clockwise in screen coordinates. But orientation is flipped when JTS geometry is encoded to mvt, so preparation step should do the opposite orientation when it operates with JTS geometry. Mapbox encoder implementation does it like I say https://github.com/mapbox/geojson-vt/blob/d3bf094b1329dd84dcc8446ba407c9830469b808/src/tile.js#L113

Because of this issue extruded polygons are displayed inside out in mapboxgl viewer

Screen Recording 2019-05-09 at 2 42 59 PM_1

When I swapped orientation checks for exterior and interior rings in the following file then 3d polygons became normal https://github.com/wdtinc/mapbox-vector-tile-java/blob/e5e3df3fc2260709e289f972a1348c0a1ea30339/src/main/java/com/wdtinc/mapbox_vector_tile/adapt/jts/JtsAdapter.java#L366 https://github.com/wdtinc/mapbox-vector-tile-java/blob/e5e3df3fc2260709e289f972a1348c0a1ea30339/src/main/java/com/wdtinc/mapbox_vector_tile/adapt/jts/JtsAdapter.java#L385

ShibaBandit commented 5 years ago

I'll check this out

zxp209 commented 5 years ago

@ShibaBandit Hi,Did you checkout this issue?

FelixCali commented 5 years ago

We had the same problem when testing with a tegola server as tile source. Should definetely be fixed to conform with the mvt-standard.

ShibaBandit commented 5 years ago

So the method @exe-dealer mentioned takes in geometry in MVT coordinates (y-positive down). It then tries to ensure that the winding order is CW for the outer and CCW for the inner. I guess the issue is that the spec wants that ordering to be determined from the source geometry that does not have a positive Y down axis, so flipping the signs appears to handle this?

ShibaBandit commented 5 years ago

Trying out some changes for this problem, one commit I saw on this issue thread is incomplete because it didn't flip the PolyRingClassifierV2_1#classifyRings check. Therefore unit tests would fail .

ShibaBandit commented 5 years ago

Closing via #41