Skip to content

Commit

Permalink
Reuse cluster markers (#321)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jeffdgr8 authored and stephenmcd committed Oct 31, 2016
1 parent c20c63c commit 6b2748c
Showing 1 changed file with 20 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 6b2748c

Please sign in to comment.