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

Polygon orientation #36

Closed
exe-dealer opened this issue May 9, 2019 · 6 comments
Closed

Polygon orientation #36

exe-dealer opened this issue May 9, 2019 · 6 comments
Assignees
Labels

Comments

@exe-dealer
Copy link

exe-dealer commented May 9, 2019

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


@ShibaBandit ShibaBandit self-assigned this May 9, 2019
@ShibaBandit
Copy link
Member

I'll check this out

@zxp209
Copy link

zxp209 commented Jun 11, 2019

@ShibaBandit Hi,Did you checkout this issue?

@FelixCali
Copy link

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

marionmaiden pushed a commit to targomo/mapbox-vector-tile-java that referenced this issue Jul 30, 2019
…reation on mvt. This hotfix should be applied until the issue wdtinc#36 get fixed
@ShibaBandit
Copy link
Member

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
Copy link
Member

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 ShibaBandit added the bug label Aug 2, 2019
@ShibaBandit
Copy link
Member

Closing via #41

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

No branches or pull requests

4 participants