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

fix: fixed marker flickering caused by creating new instances on each render #536

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 83 additions & 4 deletions src/markerclusterer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ describe.each(markerClasses)(
markerClusterer.getProjection = jest
.fn()
.mockImplementation(() => mapCanvasProjection);
markerClusterer["reset"] = jest.fn();
markerClusterer["resetClusters"] = jest.fn();
markerClusterer["renderClusters"] = jest.fn();
markerClusterer.render();

expect(calculate).toBeCalledWith({ map, markers, mapCanvasProjection });
expect(markerClusterer["reset"]).toHaveBeenCalledTimes(1);
expect(markerClusterer["resetClusters"]).toHaveBeenCalledTimes(1);
expect(markerClusterer["renderClusters"]).toHaveBeenCalledTimes(1);
});

Expand All @@ -100,11 +100,11 @@ describe.each(markerClasses)(
markerClusterer.getProjection = jest
.fn()
.mockImplementation(() => mapCanvasProjection);
markerClusterer["reset"] = jest.fn();
markerClusterer["resetClusters"] = jest.fn();
markerClusterer["renderClusters"] = jest.fn();
markerClusterer.render();

expect(markerClusterer["reset"]).toHaveBeenCalledTimes(0);
expect(markerClusterer["resetClusters"]).toHaveBeenCalledTimes(0);
expect(markerClusterer["renderClusters"]).toHaveBeenCalledTimes(0);
});

Expand Down Expand Up @@ -245,6 +245,85 @@ describe.each(markerClasses)(
expect(markerClusterer["markers"]).toHaveLength(1);
});

test("markerClusterer does not recreate marker when unaffected", () => {
const unaffectedMarkerPosition = {
lat: 0,
lng: 0,
};

const unaffectedMarker = new markerClass({
position: unaffectedMarkerPosition,
});

const newMarkerPosition = {
lat: 10,
lng: 10,
};

const newMarker = new markerClass({
position: newMarkerPosition,
});

map.setCenter({ lat: 0, lng: 0 });
map.setZoom(10);

const markerClusterer = new MarkerClusterer({
renderer,
algorithm,
});

MarkerUtils.setMap = jest.fn();
markerClusterer.getMap = jest.fn().mockImplementation(() => map);
markerClusterer.getProjection = jest
.fn()
.mockImplementation(() => map.getProjection());

markerClusterer.addMarker(unaffectedMarker);

jest.spyOn(algorithm, "calculate").mockImplementationOnce(() => {
return {
changed: true,
clusters: [
new Cluster({
markers: [markerClusterer["markers"][0]],
position: unaffectedMarkerPosition,
}),
],
};
});

markerClusterer.addMarker(newMarker);

jest.spyOn(algorithm, "calculate").mockImplementationOnce(() => {
return {
changed: true,
clusters: [
new Cluster({
markers: [markerClusterer["markers"][0]],
position: unaffectedMarkerPosition,
}),
new Cluster({
markers: [markerClusterer["markers"][1]],
position: newMarkerPosition,
}),
],
};
});

jest.spyOn(MarkerUtils, "setMap").mockClear();
jest.spyOn(MarkerUtils, "getPosition").mockClear();

markerClusterer.render();

expect(markerClusterer["markers"]).toHaveLength(2);
expect(markerClusterer["clusters"]).toHaveLength(2);
expect(MarkerUtils.getPosition).toHaveBeenCalledTimes(2);

// cluster.delete calls it once (therefore 2 times clusters = 2)
// and renderClusters should only call it once (for the new cluster)
expect(MarkerUtils.setMap).toHaveBeenCalledTimes(3);
});

test("markerClusterer addMarkers", () => {
const markerClusterer = new MarkerClusterer({
markers: [],
Expand Down
40 changes: 39 additions & 1 deletion src/markerclusterer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,37 @@ export class MarkerClusterer extends OverlayViewSafe {

// allow algorithms to return flag on whether the clusters/markers have changed
if (changed || changed == undefined) {
const records = new Map<string, Cluster>();

this.clusters.forEach(
(cluster) =>
cluster.marker &&
nora-soderlund marked this conversation as resolved.
Show resolved Hide resolved
records.set(
`${cluster.position.lat()}x${cluster.position.lng()}`,
cluster
)
);

clusters.forEach((cluster, index) => {
// if there's clusters that has been "processed" (Cluster.marker is set), then reuse it instead of reprocessing it
const clusterKey = `${cluster.position.lat()}x${cluster.position.lng()}`;

const duplicateCluster = records.get(clusterKey);

if (duplicateCluster) {
// we'll reset the Cluster.markers here and only add the existing item
// down below in renderClusters, it will assign this one item to Cluster.marker
clusters[index].markers.length = 0;
clusters[index].markers.push(duplicateCluster.marker);

duplicateCluster.marker = null;
records.delete(clusterKey);
}
});

// reset visibility of markers and clusters
this.reset();
this.resetClusters();

// store new clusters
this.clusters = clusters;

Expand Down Expand Up @@ -214,6 +243,10 @@ export class MarkerClusterer extends OverlayViewSafe {
this.clusters = [];
}

protected resetClusters(): void {
this.clusters.forEach((cluster) => cluster.delete());
}

protected renderClusters(): void {
// generate stats to pass to renderers
const stats = new ClusterStats(this.markers, this.clusters);
Expand All @@ -222,7 +255,12 @@ export class MarkerClusterer extends OverlayViewSafe {
if (cluster.markers.length === 1) {
cluster.marker = cluster.markers[0];
} else {
// we disable the markers here only when we're re-processing the cluster marker
// this stops the markers from flickering on each map event.
cluster.markers.forEach((marker) => MarkerUtils.setMap(marker, null));

cluster.marker = this.renderer.render(cluster, stats, map);

if (this.onClusterClick) {
cluster.marker.addListener(
"click",
Expand Down