Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[android] Polygon holes #8557

Merged
merged 3 commits into from
Apr 11, 2017
Merged

[android] Polygon holes #8557

merged 3 commits into from
Apr 11, 2017

Conversation

Guardiola31337
Copy link
Contributor

@Guardiola31337 Guardiola31337 commented Mar 29, 2017

New attempt of fixing #1729

  • Support for 1 hole
    • Support in Polygon
    • Support in PolygonOptions
  • Support for multiple holes

@ivovandongen Could you review/add more comments on C++ part?
At the moment, still remains the following:

  • Tweak C++ code
    • Naming
    • Use of aliases?
    • Separate the implementation in an implementation file
    • Separate the concerns in multiple methods

Anything else?

BTW, does the overall PR make sense? Would you tackle it using a different approach?

@Guardiola31337 Guardiola31337 added Android Mapbox Maps SDK for Android feature labels Mar 29, 2017
@Guardiola31337 Guardiola31337 self-assigned this Mar 29, 2017
Copy link
Contributor

@ivovandongen ivovandongen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should take into consideration that there can be multiple (non-overlapping) holes.

@1ec5
Copy link
Contributor

1ec5 commented Mar 29, 2017

For reference, #7397 attempted to support multiple holes at the Java level, and mbgl does support multiple holes (as seen on iOS and macOS); it’s the JNI part that apparently wasn’t working.

@Guardiola31337 Guardiola31337 changed the title [android] Polygon hole [android] Polygon holes Apr 11, 2017
Copy link
Contributor

@ivovandongen ivovandongen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Guardiola31337 Guardiola31337 merged commit b71d865 into master Apr 11, 2017
@Guardiola31337 Guardiola31337 deleted the pg-7397-android-polygon-hole branch April 11, 2017 17:10
@tobrun tobrun mentioned this pull request May 2, 2017
12 tasks
@tobrun tobrun mentioned this pull request Jun 9, 2017
12 tasks
@tobrun tobrun mentioned this pull request Jun 21, 2017
11 tasks
@tobrun tobrun mentioned this pull request Jun 30, 2017
16 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants