googlemaps / android-maps-compose

Jetpack Compose composables for the Maps SDK for Android
https://developers.google.com/maps/documentation/android-sdk/maps-compose
Apache License 2.0
1.17k stars 143 forks source link

Add support for clustering #44

Closed spoelt closed 1 year ago

spoelt commented 2 years ago

Do you plan on adding clustering to the Compose version of Google Maps? I tried making the switch from AndroidView({ MapView() }) to the new Composable function but struggle to implement the ClusterManager given that I cannot directly reference the map as an input parameter when instantiating the ClusterManager. (... and thank you for giving us this awesome Composable in the first place 🙏)

jpoehnelt commented 2 years ago

@spoelt Thank you for opening this issue. 🙏 Please check out these other resources that might be applicable:

This is an automated message, feel free to ignore.

arriolac commented 2 years ago

Yes, I would like to add this! Adding clustering support isn't quite that simple though as Maps Compose entirely manages the underlying GoogleMap. I've summarized my thoughts on this in this comment

StephenVinouze commented 2 years ago

Can't this be achieved already with the Maps Utils Library? Since that's where you get the ClusterManager. I haven't tried yet but I naively thought Map Compose would interact with this lib the same way as with Views 🤔

StephenVinouze commented 2 years ago

Hmm I see, the ClusterManager needs a map as an argument. Never mind my previous comment then 😅

NitroG42 commented 2 years ago

I think I'll provide some more info on why the cluster manager is needed: Right now, trying to display a high number of marker on the Maps will make the render freezes for a few seconds with +500 (+1000) markers, using a simple:

poi.forEach {
    Marker(...)
}

I think the only solution for now is either display less (based on the viewport for example, restrict to a certain amount or distance), or make an "almost" clustering implementation outside of the composition (based on the viewport, and make "cluster" markers on the fly by grouping your markers based on a distance or lat/long. It might be long to make that of course)

sajithlaldev commented 2 years ago

It was a nice feature in Native android. Now I am switched to Jetpack and screwed with my project. :( Hope this will be added soon

barbeau commented 2 years ago

@sajithlaldev maps-compose doesn't yet support clustering, but note that if the rest of your project is written using Compose you can still use the normal Android Maps SDK within your project by using the Compose Interoperability APIs: https://developer.android.com/jetpack/compose/interop/interop-apis#views-in-compose

So if clustering is critical to you I'd recommend sticking with the normal Maps SDK for now and using compose interop solution, which should work even if the rest of your app is written in Compose.

sajithlaldev commented 2 years ago

Compose Interoperability APIs this would be nice. thanks for the info @barbeau

Psijic commented 2 years ago

It looks like the original clustering / layers where made with a "difficult" code/architecture and now should be implemented from scratch to Compose maps. Maybe it'll add some better code structure and performance but currently is it possible to make clusters using custom markers images? Like you add a marker with a circle image and it's really a cluster that will change it's image with zooming.

psteiger commented 2 years ago

@Psijic as an user of the current clustering library, I agree it begs for a complete rewrite in Compose mindset.

Those are my superficial thoughts on this matter:

1) Given a set of (x,y) points, a set of clusters must be generated, where a cluster is itself a (x,y) point but holds a set of 1 or more (x,y) points, representing the original points it is clustering. The x,y coordinates of the cluster is some kind of median between all x and all y of the original points.

2) Every time zoom changes, clusters must be recalculated. If the new zoom makes the points in the cluster not close enough for clustering, they should individually animate from the cluster position to the original position. Conversely, if the new zoom makes the points too close together, they should animate towards the cluster.

3) clicking on a clustering zooms in just enough so that the cluster destructures into its individual items.

Before we have a proper library for that, I'm thinking maybe this computation can be done in ViewModel, assuming the ViewModel stores the complete set of Items and the camera zoom. Thinking in Flow or MutableState, looks like it would be possible to combine the camera zoom and the List<Item>, resulting in a List<ClusterItem> where ClusterItem owns a (x,y) position, an image, and N items the cluster is representing.

The animation part sounds tricky. The clustering algorithm looks like non-trivial but the algorithms can easily be reused from the original clustering library.

newmanw commented 2 years ago

Adding first class clustering would be amazing! Still surprises me that Apple has clustering support built into the SDK and Google does not.

basurahan commented 2 years ago

clustering should be part of the next milestone I don't know why google is not prioritizing this its such a critical part of performance

jeffnyauke commented 2 years ago

Do we have a timeline for this?

ColtonIdle commented 2 years ago

👀 https://github.com/googlemaps/android-maps-compose/issues/135

Shusshu commented 2 years ago

https://github.com/googlemaps/android-maps-compose/pull/140

barbeau commented 2 years ago

As @Shusshu pointed out above, PR https://github.com/googlemaps/android-maps-compose/pull/140 has been merged which adds an experimental MapEffect composable which exposes the GoogleMap object that enables a workaround for clustering support.

See the new MapClusteringActivity in the demo app for a demo: https://github.com/googlemaps/android-maps-compose/blob/main/app/src/main/java/com/google/maps/android/compose/MapClusteringActivity.kt#L28

Note also some of the gotchas documented in the README: https://github.com/googlemaps/android-maps-compose#obtaining-access-to-the-raw-googlemap-experimental

basurahan commented 2 years ago

I am using the sample clustering using MapEffect but the info window of the markers are not kept open when navigating back to the map. How can i solve that?

Psijic commented 1 year ago

Is there any custom-made clustering? I suppose, all the layout features unsupported now?

ColtonIdle commented 1 year ago

@Psijic I don't work on maps compose, but I don't understand your question? i have clustering working with compose. what do you mean by custom-made clustering?

Psijic commented 1 year ago

@ColtonIdle I mean, half a year ago clustering wasn't available in Compose Google Maps. How did you enable it here? GoogleMap(Modifier.fillMaxSize(), content = content) Or you added old xml-based maps for clustering in your Compose layer? It's not the proper way for Compose, just an ad-hoc.

StephenVinouze commented 1 year ago

They made a workaround to let you access the map instance you need to use the ClusterManager from the util library. It works fine, with some limitations – it's marked as experimental. Everything is mentioned above along with the documentation: https://github.com/googlemaps/android-maps-compose#obtaining-access-to-the-raw-googlemap-experimental. With that approach, you can now use clusterization while keeping a full compose. I believe they keep the issue open because they want to use their own, more optimized implementation. But they'll likely not arrive anytime soon given the complexity this entails; so you can still fallback to this.

Psijic commented 1 year ago

It's something good. Does it have the close to Compose performance or it updates 60 FPS?

StephenVinouze commented 1 year ago

I cannot say about benchmarks but on my side, it runs pretty smoothly so far :)

ColtonIdle commented 1 year ago

Same here. I've used the new method with success. No more android view!

basurahan commented 1 year ago

One thing I can say is performance really bad

ColtonIdle commented 1 year ago

@basurahan no perf issues here (in either debug or release builds). Do you have a repository you can put up to reproduce?

basurahan commented 1 year ago

Really ? @ColtonIdle the initial loading of the map is really bad I doubt you don't have performance issues

wangela commented 1 year ago

We'd love for folks interested in this issue to take a look at the draft PR #258 and comment on whether that would work better for you. Feel free to make suggestions as well. We'll leave it open for at least a week to provide some time for comment.

Note that PR #258 also proposes to deliver this in a maps-compose-utils library (separate dependency) so that android-maps-compose stays true to the contents of Maps SDK for Android. Open to feedback on that decision as well.

googlemaps-bot commented 1 year ago

:tada: This issue has been resolved in version 2.11.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: