Skip to content

Commit 577fbe4

Browse files
airsliceCopilot
andauthored
fix: unnecessary tile re-renders when viewerProperty changes (#135)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 0b1a6c2 commit 577fbe4

2 files changed

Lines changed: 117 additions & 7 deletions

File tree

src/engines/Cesium/core/Imagery.test.ts

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,44 @@
11
import { renderHook } from "@testing-library/react";
22
import { UrlTemplateImageryProvider } from "cesium";
3-
import { expect, test, vi } from "vitest";
3+
import { expect, test, vi, beforeEach } from "vitest";
44

55
import type { CustomProviderConfig } from "../../../Map/types/customProvider";
66

7-
import { type Tile, useImageryProviders } from "./Imagery";
7+
import ImageryLayers, { type Tile, useImageryProviders } from "./Imagery";
8+
9+
// Mock Cesium scene and imageryLayerCollection for ImageryLayers component tests
10+
const mockAdd = vi.fn();
11+
const mockRemove = vi.fn();
12+
const mockContains = vi.fn(() => true);
13+
const mockIndexOf = vi.fn(() => 0);
14+
const mockRequestRender = vi.fn();
15+
16+
const mockImageryLayerCollection = {
17+
add: mockAdd,
18+
remove: mockRemove,
19+
contains: mockContains,
20+
indexOf: mockIndexOf,
21+
};
22+
23+
const mockScene = {
24+
requestRender: mockRequestRender,
25+
isDestroyed: () => false,
26+
};
27+
28+
vi.mock("resium", () => ({
29+
useCesium: () => ({
30+
imageryLayerCollection: mockImageryLayerCollection,
31+
scene: mockScene,
32+
}),
33+
}));
34+
35+
beforeEach(() => {
36+
mockAdd.mockClear();
37+
mockRemove.mockClear();
38+
mockContains.mockClear();
39+
mockIndexOf.mockClear();
40+
mockRequestRender.mockClear();
41+
});
842

943
test("useImageryProviders", () => {
1044
const provider = vi.fn(({ url }: { url?: string } = {}): any => ({ hoge: url }));
@@ -153,3 +187,58 @@ test("useImageryProviders", () => {
153187
typedRerender({ tiles: [] });
154188
expect(result.current.providers).toEqual({});
155189
});
190+
191+
test("ImageryLayers should not re-render when tiles array reference changes but content is the same", async () => {
192+
const tiles: Tile[] = [{ id: "1", type: "open_street_map", opacity: 0.8 }];
193+
194+
const { rerender } = renderHook(
195+
({ tiles }: { tiles: Tile[] }) => {
196+
return ImageryLayers({
197+
tiles,
198+
cesiumIonAccessToken: undefined,
199+
customProvider: undefined,
200+
onTilesChange: undefined,
201+
});
202+
},
203+
{
204+
initialProps: { tiles },
205+
},
206+
);
207+
208+
// Wait for initial render to complete
209+
await new Promise(resolve => setTimeout(resolve, 0));
210+
211+
// Clear mock calls from initial render
212+
mockAdd.mockClear();
213+
mockRemove.mockClear();
214+
215+
// Re-render with a NEW tiles array reference but SAME content
216+
const newTilesArraySameContent: Tile[] = [{ id: "1", type: "open_street_map", opacity: 0.8 }];
217+
218+
rerender({ tiles: newTilesArraySameContent });
219+
220+
// Wait for any effects to run
221+
await new Promise(resolve => setTimeout(resolve, 0));
222+
223+
// Effect should NOT have run again - no layers should be removed or added
224+
expect(mockRemove).not.toHaveBeenCalled();
225+
expect(mockAdd).not.toHaveBeenCalled();
226+
227+
// Clear mocks
228+
mockAdd.mockClear();
229+
mockRemove.mockClear();
230+
231+
// Re-render with DIFFERENT content (changed opacity)
232+
const differentTiles: Tile[] = [
233+
{ id: "1", type: "open_street_map", opacity: 0.5 }, // opacity changed
234+
];
235+
236+
rerender({ tiles: differentTiles });
237+
238+
// Wait for effects to run
239+
await new Promise(resolve => setTimeout(resolve, 0));
240+
241+
// Effect SHOULD run - old layers removed and new ones added
242+
expect(mockRemove).toHaveBeenCalled();
243+
expect(mockAdd).toHaveBeenCalled();
244+
});

src/engines/Cesium/core/Imagery.tsx

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,23 @@ export default function ImageryLayers({
5353
}: Props) {
5454
const { imageryLayerCollection, scene } = useCesium();
5555

56+
// Create a stable tiles reference that only changes when the content actually changes
57+
// Normalize `undefined` to a stable empty array so `useImageryProviders` doesn't allocate a new default `[]` each render.
58+
const emptyTiles = useMemo<Tile[]>(() => [], []);
59+
const tilesValue = tiles ?? emptyTiles;
60+
61+
const prevTilesRef = useRef<Tile[]>(tilesValue);
62+
const stableTiles = useMemo(() => {
63+
if (!isEqual(prevTilesRef.current, tilesValue)) {
64+
prevTilesRef.current = tilesValue;
65+
}
66+
return prevTilesRef.current;
67+
}, [tilesValue]);
68+
69+
// Pass stableTiles to useImageryProviders to prevent providers from being recreated
70+
// when tiles reference changes but content is the same
5671
const { providers } = useImageryProviders({
57-
tiles,
72+
tiles: stableTiles,
5873
cesiumIonAccessToken,
5974
customProvider,
6075
presets: tilePresets,
@@ -66,7 +81,9 @@ export default function ImageryLayers({
6681
let cancelled = false;
6782
const addedLayers: CesiumImageryLayer[] = [];
6883
// Track layers by their intended index to maintain order with async loading
69-
const layersByIndex: (CesiumImageryLayer | null)[] = new Array(tiles?.length || 0).fill(null);
84+
const layersByIndex: (CesiumImageryLayer | null)[] = new Array(stableTiles?.length || 0).fill(
85+
null,
86+
);
7087

7188
const reorderLayers = () => {
7289
if (cancelled || scene.isDestroyed()) return;
@@ -91,7 +108,7 @@ export default function ImageryLayers({
91108
scene.requestRender();
92109
};
93110

94-
tiles?.forEach(({ id, zoomLevel, opacity, heatmap }, i) => {
111+
stableTiles?.forEach(({ id, zoomLevel, opacity, heatmap }, i) => {
95112
const providerOrPromise = providers[id]?.[3];
96113
if (!providerOrPromise) return;
97114

@@ -117,7 +134,9 @@ export default function ImageryLayers({
117134
};
118135

119136
if (providerOrPromise instanceof Promise) {
120-
providerOrPromise.then(doAdd).catch(err => console.error("Failed to load imagery provider:", err));
137+
providerOrPromise
138+
.then(doAdd)
139+
.catch(err => console.error("Failed to load imagery provider:", err));
121140
} else {
122141
doAdd(providerOrPromise);
123142
}
@@ -134,7 +153,9 @@ export default function ImageryLayers({
134153
}
135154
}
136155
};
137-
}, [providers, tiles, imageryLayerCollection, scene, onTilesChange]);
156+
// Note: Using `stableTiles` to prevent re-renders when tiles reference changes but content is identical.
157+
// This also stabilizes `providers` since it depends on tiles in useImageryProviders.
158+
}, [providers, stableTiles, imageryLayerCollection, scene, onTilesChange]);
138159

139160
return null;
140161
}

0 commit comments

Comments
 (0)