Skip to content

Commit

Permalink
fix: reduce flickering (#660)
Browse files Browse the repository at this point in the history
* chore(release): 2.2.0 [skip ci]

## [2.2.0](v2.1.4...v2.2.0) (2023-06-01)

### Features

* add a dev server ([#656](#656)) ([b025dec](b025dec))

### Code Refactoring

* **distanceBetweenPoints:** optimize ([#646](#646)) ([e0f9a11](e0f9a11))

### Miscellaneous Chores

* **deps-dev:** bump @babel/preset-env from 7.21.5 to 7.22.4 ([#650](#650)) ([982730d](982730d))
* **deps-dev:** bump @babel/runtime-corejs3 from 7.21.5 to 7.22.3 ([#649](#649)) ([1c9f4fb](1c9f4fb))
* **deps-dev:** bump @types/selenium-webdriver from 4.1.14 to 4.1.15 ([#655](#655)) ([526e65c](526e65c))
* **deps-dev:** bump @typescript-eslint/eslint-plugin ([#654](#654)) ([68a94db](68a94db))
* **deps-dev:** bump @typescript-eslint/parser from 5.59.7 to 5.59.8 ([#653](#653)) ([a46d0a7](a46d0a7))
* **deps-dev:** bump geckodriver from 4.0.0 to 4.0.2 ([#652](#652)) ([19518a4](19518a4))
* update actions to use Node.js 16 ([#659](#659)) ([bcbdddf](bcbdddf))

* fix: reduce flickering

* Individual markers

Before this PR all the individual markers were removed from the map during the render cycle
by calling the reset method and to maybe be added back later in renderClusters.

After this CL the individual markers that will still be displayed on the map after the
render cycle are not removed from the map.

* Group markers

This PR still renders the group markers each time but the former group markers are only
removed from the map after the new one are added.

It turns out that removing the group markers in the next raf really help with flickering.

*  chore: resolves merge conflicts on to beta

---------

Co-authored-by: semantic-release-bot <[email protected]>
Co-authored-by: Alex Muramoto <[email protected]>
  • Loading branch information
3 people authored Jun 1, 2023
1 parent 90a62ee commit 8b756a7
Show file tree
Hide file tree
Showing 2 changed files with 152 additions and 7 deletions.
113 changes: 112 additions & 1 deletion src/markerclusterer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,23 @@ describe.each(markerClasses)(
const renderer = { render };

let map: google.maps.Map;
let rafSpy: jest.SpyInstance;

beforeEach(() => {
map = new google.maps.Map(document.createElement("div"));
// Runs the raf callback immediately.
rafSpy = jest
.spyOn(window, "requestAnimationFrame")
.mockImplementation((cb) => {
cb(performance.now());
return 0;
});
});

afterEach(() => {
calculate.mockClear();
render.mockClear();
rafSpy.mockRestore();
});

test("markerClusterer does not render if no map", () => {
Expand Down Expand Up @@ -81,7 +90,7 @@ describe.each(markerClasses)(
markerClusterer.render();

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

Expand Down Expand Up @@ -125,6 +134,86 @@ describe.each(markerClasses)(
expect(deleteSpy).toHaveBeenCalledTimes(1);
});

test("markerClusterer render should not remove markers from the map if they were already rendered", () => {
const marker = new markerClass();
const markers: Marker[] = [marker];

const algorithm = {
calculate: jest.fn().mockReturnValue({
clusters: [new Cluster({ markers })],
changed: true,
}),
};
const markerClusterer = new MarkerClusterer({
markers,
algorithm,
});
markerClusterer.getMap = jest.fn().mockImplementation(() => map);
markerClusterer.getProjection = jest
.fn()
.mockImplementation(() => jest.fn());
markerClusterer["renderClusters"] = jest.fn();
markerClusterer["clusters"] = [new Cluster({ markers })];

MarkerUtils.setMap = jest.fn().mockImplementation(() => null);

markerClusterer["render"]();

expect(MarkerUtils.setMap).toHaveBeenCalledTimes(0);
});

test("markerClusterer render should remove markers from the map if they are no more rendered", () => {
const marker = new markerClass();
const markers: Marker[] = [marker];

const algorithm = {
calculate: jest.fn().mockReturnValue({ clusters: [], changed: true }),
};
const markerClusterer = new MarkerClusterer({
markers,
algorithm,
});
markerClusterer.getMap = jest.fn().mockImplementation(() => map);
markerClusterer.getProjection = jest
.fn()
.mockImplementation(() => jest.fn());
markerClusterer["renderClusters"] = jest.fn();
const cluster = new Cluster({ markers });
cluster.marker = marker;
markerClusterer["clusters"] = [cluster];

MarkerUtils.setMap = jest.fn().mockImplementation(() => null);

markerClusterer["render"]();

expect(MarkerUtils.setMap).toHaveBeenCalledWith(marker, null);
});

test("markerClusterer render should remove all group cluster markers from the map", () => {
const markers: Marker[] = [new markerClass(), new markerClass()];
const algorithm = {
calculate: jest.fn().mockReturnValue({ clusters: [], changed: true }),
};
const markerClusterer = new MarkerClusterer({
markers,
algorithm,
});
markerClusterer.getMap = jest.fn().mockImplementation(() => map);
markerClusterer.getProjection = jest
.fn()
.mockImplementation(() => jest.fn());
markerClusterer["renderClusters"] = jest.fn();
const cluster = new Cluster({ markers });
cluster.marker = new markerClass();
markerClusterer["clusters"] = [cluster];

MarkerUtils.setMap = jest.fn().mockImplementation(() => null);

markerClusterer["render"]();

expect(MarkerUtils.setMap).toHaveBeenCalledWith(cluster.marker, null);
});

test("markerClusterer renderClusters bypasses renderer if just one", () => {
const markers: Marker[] = [new markerClass()];

Expand Down Expand Up @@ -174,6 +263,28 @@ describe.each(markerClasses)(
);
});

test("markerClusterer renderClusters remove all individual markers from the map", () => {
const marker1 = new markerClass();
const marker2 = new markerClass();
const markers: Marker[] = [marker1, marker2];

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

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

const clusters = [new Cluster({ markers })];

markerClusterer["clusters"] = clusters;
markerClusterer["renderClusters"]();

expect(MarkerUtils.setMap).toBeCalledWith(marker1, null);
expect(MarkerUtils.setMap).toBeCalledWith(marker2, null);
});

test("markerClusterer renderClusters does not set click handler", () => {
const markers: Marker[] = [new markerClass()];

Expand Down
46 changes: 40 additions & 6 deletions src/markerclusterer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,44 @@ export class MarkerClusterer extends OverlayViewSafe {
mapCanvasProjection: this.getProjection(),
});

// allow algorithms to return flag on whether the clusters/markers have changed
// Allow algorithms to return flag on whether the clusters/markers have changed.
if (changed || changed == undefined) {
// reset visibility of markers and clusters
this.reset();
// store new clusters
this.clusters = clusters;
// Accumulate the markers of the clusters composed of a single marker.
// Those clusters directly use the marker.
// Clusters with more than one markers use a group marker generated by a renderer.
const singleMarker = new Set<Marker>();
for (const cluster of clusters) {
if (cluster.markers.length == 1) {
singleMarker.add(cluster.markers[0]);
}
}

const groupMarkers: Marker[] = [];
// Iterate the clusters that are currently rendered.
for (const cluster of this.clusters) {
if (cluster.marker == null) {
continue;
}
if (cluster.markers.length == 1) {
if (!singleMarker.has(cluster.marker)) {
// The marker:
// - was previously rendered because it is from a cluster with 1 marker,
// - should no more be rendered as it is not in singleMarker.
MarkerUtils.setMap(cluster.marker, null);
}
} else {
// Delay the removal of old group markers to avoid flickering.
groupMarkers.push(cluster.marker);
}
}

this.clusters = clusters;
this.renderClusters();

// Delayed removal of the markers of the former groups.
requestAnimationFrame(() =>
groupMarkers.forEach((marker) => MarkerUtils.setMap(marker, null))
);
}
google.maps.event.trigger(
this,
Expand Down Expand Up @@ -215,14 +245,18 @@ export class MarkerClusterer extends OverlayViewSafe {
}

protected renderClusters(): void {
// generate stats to pass to renderers
// Generate stats to pass to renderers.
const stats = new ClusterStats(this.markers, this.clusters);
const map = this.getMap() as google.maps.Map;

this.clusters.forEach((cluster) => {
if (cluster.markers.length === 1) {
cluster.marker = cluster.markers[0];
} else {
// Generate the marker to represent the group.
cluster.marker = this.renderer.render(cluster, stats, map);
// Make sure all individual markers are removed from the map.
cluster.markers.forEach((marker) => MarkerUtils.setMap(marker, null));
if (this.onClusterClick) {
cluster.marker.addListener(
"click",
Expand Down

0 comments on commit 8b756a7

Please sign in to comment.