maplibre / maplibre-native

MapLibre Native - Interactive vector tile maps for iOS, Android and other platforms.
https://maplibre.org
BSD 2-Clause "Simplified" License
1.05k stars 303 forks source link

Improve platform inconsistencies #1462

Closed JulianBissekkou closed 9 months ago

JulianBissekkou commented 1 year ago

We are currently maintaining the flutter maplibre package and while implementing the platform views we discovered multiple inconsistencies between Android and iOS (also web, but that's another story)

Here are some APIs that we found that are currently inconsistent:

We are trying to expose a single dart API to the users of the package and platform inconsistencies make this very hard. (Same goes for potential framework wrappers in kotlin native or react-native) I can also imaging that teams that are building standalone android and iOS Apps would benefit from a consistent API since the teams are able to share knowledge.

Of course we will have some differences because we have to make sure maplibre integrates well into the specific platform. We shouldn' have consistent naming across all platforms and ignore platform specific APIs. An example for this might be better SwiftUI or Compose integration or extension that integrate the map easier into platform specific lifecycle events.

In order to improve consistency, we can:

Define a clear API surface...

that we expose to every platform. I mean there is a reason why android and iOS diverged. Why did this happen and what can we do in order to avoid that? Maybe make the C++ api easier to use?

Improve documentation on what features goes where

One reason can be that people don't know all the features and therefore write code in the platform sdks that might not be needed. There is also a certain barrier when it comes to contributing to the C++ code because some people have absolutly no idea and might be terrified to learn or even touch C++. How can we make this easier?

1ec5 commented 1 year ago

I mean there is a reason why android and iOS diverged. Why did this happen and what can we do in order to avoid that?

The Android and iOS SDKs are written for Android and iOS developers – developers familiar with Java, Kotlin, Objective-C, or Swift, each of which have their own naming conventions that this project has little or no control over. If consistency across platforms is overly prioritized, it will result in greater confusion, not less, as developers come from other non-map libraries on their respective platforms and continue to interact with other libraries on those platforms.

In some cases, the Android SDK and GL JS adapted terminology to match Google Maps, while the iOS/macOS SDK adopted terminology to match MapKit, based on the dominant library on each respective platform. But to the extent that something is specific to Mapbox/MapLibre, this project is freer to come up with its own consistent terminology without worrying about causing developers confusion.

If you find the inconsistency between platforms to be problematic, your Flutter users will appreciate your efforts at smoothing them over with an API that’s familiar to Flutter developers, but this doesn’t mean everyone else will necessarily benefit from the same massaging. What would certainly help with consistency is to migrate the iOS/macOS SDK to pure Swift. There will still be many unavoidable differences from Android and Kotlin, but Swift is a more modern language that wouldn’t need some of the more extreme language workarounds that this library employs, such as the NSExpression/NSPredicate language.

JulianBissekkou commented 1 year ago

I think it's fine to have APIs that are more focused towards the target platform in terms of naming (following naming conventions) and of course I may use slightly different classes as you showed here. I think my latest PR #1455 caused some confusion because the naming was similar to android. To clarfiy: We don't want to have the exact same names on all platforms! I am not a iOS dev and therefore I just went with a name that made sense to me, not knowing that there is a totally different naming convention on iOS. We should fix that for sure 👍🏽

The problem we are facing is that some things are just working complelty different, and both SDKs expose the c++ layer in a different way. The CameraUpdate API is a good example. Android build something extra while iOS "only" has the existing mapView.cameraThatFits methods. They also behave slightly different and may come with different limitations. As a team that writes iOS and Android apps they now have to solve the problem twice and they may get different results.

If we have a certained C++ api surface that we expose consistently (with respect to the target platforms guidlines and rules) a lot of the things get easier. Maplibre currently has very limited amount of resources compared to the previous mapbox team. Why maintain so many different methods if we can align most sdks to use a certain amount of C++ APIs and then focus the development resources on them?

I updated the description of the issue to clarify.

1ec5 commented 1 year ago

The CameraUpdate API is a good example. Android build something extra while iOS "only" has the existing mapView.cameraThatFits methods.

This is a good example of how the two frameworks can be reimagined if the C++ layer allows. Both originally started with a monolithic MGLMapView (iOS) and MapboxMap (Android) but have gradually tried to break things up.

The problem, at least on iOS/macOS, is that a lot of functionality is still tied to mbgl. For example, MLNStyle (MGLStyle) got split out of MGLMapView a long time ago, but you can’t use it independently of a map view because of its backpointer to MLNMapView, required for the reference to mbgl::Map.

This might not be a problem on Android, but it wreaks havoc on standard programming patterns in Objective-C and Swift. For example, the mapbox-gl-native repository is chock-full of bug reports caused by developers holding onto a reference to an MGLStyleLayer or MGLSource and trying to share it among multiple map views. Mapbox staunched the bleeding by adding exceptions for some common cases, but it’s still a huge gotcha. Another trap is that it’s very tempting to subclass these style-related classes, but that causes all sorts of problems.

To the extent that functionality is broken out into separate classes, they need to avoid any mbgl dependency and instead function solely as value classes (or structs in Swift). In the case of CameraUpdate, that would be largely redundant to MLNCamera, but there could be a separate class to represent animation options, just as in GL JS.