feat: Add overridable shouldRender() method for use case of rendering when cluster doesn't change

[barbeau]

In the current version of DefaultClusterRenderer, there is a built-in optimization to avoid re-rendering items when there isn't a change in the clusters.

However, as discussed in #774, this prevents use cases where the clusters/markers should be re-rendered even if the cluster contents don't change.

For example, if you want a single item to render as a cluster above a certain zoom level and as a marker below a certain zoom level, this isn't possible - the item will never be updated from the cluster to the marker because that rendering pass is skipped (because the cluster contents didn't change).

This PR adds a new DefaultClusterRenderer.shouldRender() method that apps can override to control this optimization when necessary. This allows the above use case as the app can track the previous and current zoom levels and force the render to occur when the transition over the zoom threshold is made.

A new demo activity ZoomClusteringDemoActivity (shown as "Clustering: Force on Zoom" in the demo app UI) (gms and v3 flavors) is added as part of this PR to clearly illustrate how shouldRender() is used. This demo also illustrates what I believe is an undocumented feature of the library, which is that the ClusterManager will call the onCameraIdle() implementation of any Renderer that implements GoogleMap.OnCameraIdleListener before clustering and rendering takes place. This allows us to capture the zoom level that can then be used in shouldRender().

This feature is backwards-compatible, as the current behavior of apps will not change - the default implementation of shouldRender() is the same as the current version of the library.

Note that the new ZoomClusteringDemoActivity won't build when using the existing published library as a dependency because shouldRender() doesn't exist in that version of the library (I tested using gmsImplementation project(':library') as the dependency), so the CI on this PR will fail. Not sure how you want to handle that?

---

Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open a GitHub issue as a bug/feature request before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)
• Will this cause breaking changes to existing Java or Kotlin integrations? If so, ensure the commit has a BREAKING CHANGE footer so when this change is integrated a major version update is triggered. See: https://www.conventionalcommits.org/en/v1.0.0/

Closes #774 🦕

[arriolac]

Are the sets provided here immutable to ensure no additions/removals are made while this method is invoked?

Also, should this method be defined in ClusterRenderer? Exposing an API like that could be generally useful, although I'm not use if custom cluster renderers apart from DefaultClusterRenderer.

[barbeau]

Are the sets provided here immutable to ensure no additions/removals are made while this method is invoked?

Good catch - no, and this also made be realize that oldClusters could be null. newClusters can't be null in the current implementations of the clustering algorithms in the library, but given that the renderer can be used with 3rd party implementations of Algorithm we need to protect against that too.

I've fixed this for both parameters to either wrap the set in Collections.unmodifiableSet() if it's not null or pass in an empty set if it is - see f61c81b.

[barbeau]

Also, should this method be defined in ClusterRenderer? Exposing an API like that could be generally useful, although I'm not use if custom cluster renderers apart from DefaultClusterRenderer.

Hmmm, good question. I could see it being useful, both for shouldRender() and shouldRenderAsCluster(). We currently only have DefaultClusterRenderer in the library but there could be 3rd party ones.

The main downside I see is that it's a breaking change for any existing 3rd party implementations of ClusterRenderer as it would require them to implement the method before builds would succeed.

Should we add both shouldRender() and shouldRenderAsCluster() to the ClusterRenderer interface in this PR to group the changes?

[arriolac]

Let's just keep note of this in case it comes up in the future for a 3rd party implementation of ClusterRenderer—I'm just not sure if it's worth going through the trouble of exposing this feature at the interface level and breaking usages as you mentioned. There's ways to avoid a breaking change (like creating another interface) but again, not sure if it's worth the trouble.

[arriolac]

[googlemaps-bot]

🎉 This PR is included in version 2.3.0 🎉

The release is available on:

• v2.3.0
• GitHub release

Your semantic-release bot 📦🚀