From 6b2748c7fc147848ac09c60c04d78c34ed57394c Mon Sep 17 00:00:00 2001
From: Jeff Lockhart <jeff@salesrabbit.com>
Date: Mon, 31 Oct 2016 17:56:22 -0600
Subject: [PATCH] Reuse cluster markers (#321)

There was a bug where if a cluster already has a marker on the map when
a cluster operation occurs, the cluster would add a new marker without
reusing the existing marker. The new marker would update the mapping to
mClusterToMarker, adding a new mapping in mMarkerToCluster, while
leaving the old marker mapped in mMarkerToCluster. When the old marker
is then removed, it also removes the new marker's mapping in
mClusterToMarker.

This results in there no longer existing a mapping for the cluster in
mClusterToMarker and getMarker() returns null. If
OnClusterClickListener.onClusterClick() wants to do something like this:

public boolean onClusterClick(Cluster<ClusterItem> cluster) {
Marker marker = mClusterRenderer.getMarker(cluster);
marker.showInfoWindow(); <-- NullPointerException
}

it is unable.

This is easily reproducible by zooming on the map such that the clusters
don't change at all but a new cluster operation occurs. All of the
clusters that did not change at all will have the described issue.

The change uses the same pattern as cluster item uses to reuse an
existing marker for a cluster if it already exists, rather than creating
a new one and removing the old one. This ensures mClusterToMarker and
mMarkerToCluster are always the same size.

Also removed the OnInfoWindowClickListeners in onRemove() the same as
OnMarkerClickListeners. Both are added in onAdd().

Also, as mMap.getProjection() is annotated as @NonNull, the check for
projection == null was unneccesary.
---
 .../view/DefaultClusterRenderer.java          | 38 ++++++++++---------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/library/src/com/google/maps/android/clustering/view/DefaultClusterRenderer.java b/library/src/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
index d92ce7705..9f2172bc1 100644
--- a/library/src/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
+++ b/library/src/com/google/maps/android/clustering/view/DefaultClusterRenderer.java
@@ -175,7 +175,9 @@ public void onInfoWindowClick(Marker marker) {
     @Override
     public void onRemove() {
         mClusterManager.getMarkerCollection().setOnMarkerClickListener(null);
+        mClusterManager.getMarkerCollection().setOnInfoWindowClickListener(null);
         mClusterManager.getClusterMarkerCollection().setOnMarkerClickListener(null);
+        mClusterManager.getClusterMarkerCollection().setOnInfoWindowClickListener(null);
     }
 
     private LayerDrawable makeClusterBackground() {
@@ -240,7 +242,7 @@ public void setMinClusterSize(int minClusterSize) {
         mMinClusterSize = minClusterSize;
     }
 
-	/**
+    /**
      * ViewModifier ensures only one re-rendering of the view occurs at a time, and schedules
      * re-rendering, which is performed by the RenderTask.
      */
@@ -273,10 +275,6 @@ public void handleMessage(Message msg) {
                 return;
             }
             Projection projection = mMap.getProjection();
-            if (projection == null) {
-                // Without a map projection we can't render clusters.
-                return;
-            }
 
             RenderTask renderTask;
             synchronized (this) {
@@ -757,7 +755,7 @@ protected void onClusterRendered(Cluster<T> cluster, Marker marker) {
      */
     protected void onClusterItemRendered(T clusterItem, Marker marker) {
     }
-    
+
     /**
      * Get the marker from a ClusterItem
      * @param clusterItem ClusterItem which you will obtain its marker
@@ -843,17 +841,21 @@ private void perform(MarkerModifier markerModifier) {
                 return;
             }
 
-            MarkerOptions markerOptions = new MarkerOptions().
-                    position(animateFrom == null ? cluster.getPosition() : animateFrom);
-
-            onBeforeClusterRendered(cluster, markerOptions);
-
-            Marker marker = mClusterManager.getClusterMarkerCollection().addMarker(markerOptions);
-            mMarkerToCluster.put(marker, cluster);
-            mClusterToMarker.put(cluster, marker);
-            MarkerWithPosition markerWithPosition = new MarkerWithPosition(marker);
-            if (animateFrom != null) {
-                markerModifier.animate(markerWithPosition, animateFrom, cluster.getPosition());
+            Marker marker = mClusterToMarker.get(cluster);
+            MarkerWithPosition markerWithPosition;
+            if (marker == null) {
+                MarkerOptions markerOptions = new MarkerOptions().
+                        position(animateFrom == null ? cluster.getPosition() : animateFrom);
+                onBeforeClusterRendered(cluster, markerOptions);
+                marker = mClusterManager.getClusterMarkerCollection().addMarker(markerOptions);
+                mMarkerToCluster.put(marker, cluster);
+                mClusterToMarker.put(cluster, marker);
+                markerWithPosition = new MarkerWithPosition(marker);
+                if (animateFrom != null) {
+                    markerModifier.animate(markerWithPosition, animateFrom, cluster.getPosition());
+                }
+            } else {
+                markerWithPosition = new MarkerWithPosition(marker);
             }
             onClusterRendered(cluster, marker);
             newMarkers.add(markerWithPosition);
@@ -910,7 +912,7 @@ private AnimationTask(MarkerWithPosition markerWithPosition, LatLng from, LatLng
         }
 
         public void perform() {
-            ValueAnimator valueAnimator = ValueAnimator.ofFloat(0, 1);
+            ValueAnimator valueAnimator = ValueAnimator.ofFloat(0.0f, 1.0f);
             valueAnimator.setInterpolator(ANIMATION_INTERP);
             valueAnimator.addUpdateListener(this);
             valueAnimator.addListener(this);