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

Add support for clustering #44

Closed
spoelt opened this issue Feb 21, 2022 · 29 comments · Fixed by #258
Closed

Add support for clustering #44

spoelt opened this issue Feb 21, 2022 · 29 comments · Fixed by #258
Assignees
Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. released type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@spoelt
Copy link

spoelt commented Feb 21, 2022

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

@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 arriolac added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Mar 1, 2022
@arriolac
Copy link
Member

arriolac commented Mar 1, 2022

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

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

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

@NitroG42
Copy link

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

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

barbeau commented Apr 26, 2022

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

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

@Psijic
Copy link

Psijic commented May 6, 2022

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

psteiger commented May 6, 2022

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

newmanw commented May 29, 2022

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

@basurahan
Copy link

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

Do we have a timeline for this?

@ColtonIdle
Copy link

👀 #135

@Shusshu
Copy link

Shusshu commented Jun 21, 2022

#140

@barbeau
Copy link
Contributor

barbeau commented Jun 21, 2022

As @Shusshu pointed out above, PR #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
Copy link

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

Psijic commented Jan 3, 2023

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

@ColtonIdle
Copy link

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

Psijic commented Jan 6, 2023

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

StephenVinouze commented Jan 6, 2023

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

Psijic commented Jan 6, 2023

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

@StephenVinouze
Copy link

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

@ColtonIdle
Copy link

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

@basurahan
Copy link

One thing I can say is performance really bad

@DSteve595 DSteve595 assigned DSteve595 and unassigned arriolac Jan 18, 2023
@DSteve595 DSteve595 added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jan 18, 2023
@ColtonIdle
Copy link

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

@basurahan
Copy link

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

@wangela
Copy link
Member

wangela commented Jan 25, 2023

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.

@wangela wangela pinned this issue Jan 25, 2023
@wangela wangela changed the title Add support for clustering RFC: Add support for clustering Jan 25, 2023
@googlemaps-bot
Copy link
Contributor

🎉 This issue has been resolved in version 2.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@wangela wangela changed the title RFC: Add support for clustering Add support for clustering Feb 17, 2023
@wangela wangela unpinned this issue Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. released type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.