Skip to content

Commit 39e778d

Browse files
committed
perf(navigation): attach the puck once and sleep the settled camera loop
The follow-camera animation loop called `Marker.addTo(map)` every frame. MapLibre implements `addTo` as `remove()` followed by a full re-append and listener rebind, so the puck was torn down and reattached at display rate for the whole trip. The loop also scheduled its next frame before checking whether there was any work to do, so it ran at full rate with no fix, no route, or a pose that had long since converged. Attachment, pose integration and publication to MapLibre are now three separate jobs. The marker is attached once per map and only re-attached when the map itself changes; setters and `jumpTo` run only for poses that moved far enough to see; and a frame that publishes nothing twice in a row stops asking for frames until an input wakes it. Every input that can change the visible pose -- a fix, route, camera mode, north-up toggle, style reload, gesture end, recenter or visibility change -- wakes it explicitly. Moving cadence is unchanged: no frame cap is introduced, and a 600-frame 60 Hz and 1200-frame 120 Hz replay reproduce the previous puck and camera poses bit-for-bit, including which frame issued each command. `MapCanvas` now returns from its `moveend` listener before reading the camera, so the guarded programmatic path no longer allocates a center it discards. Measured on a Pixel 6 Pro against the previous build, 60 s per arm: moving 2530 puck detach/reattach cycles -> 0 stationary 3497 cycles -> 0, and 14148 animation-frame requests -> 1152
1 parent 6dddfe1 commit 39e778d

8 files changed

Lines changed: 1302 additions & 95 deletions

File tree

apps/web/src/components/map/MapCanvas.test.tsx

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ vi.mock("maplibre-gl", () => {
8080
let onCallCount = 0;
8181
class FakeMap {
8282
jumpTo = vi.fn();
83+
/** Counts camera reads so a test can prove a guarded path never took one. */
84+
cameraReads = 0;
8385

8486
constructor(mapOptions: { center: [number, number]; container: HTMLElement; zoom: number }) {
8587
instances.push(this);
@@ -88,6 +90,13 @@ vi.mock("maplibre-gl", () => {
8890
mapOptions.container.append(document.createElement("canvas"));
8991
}
9092

93+
getCenter = () => {
94+
this.cameraReads += 1;
95+
return { lng: 1.5, lat: 2.5 };
96+
};
97+
getZoom = () => 12;
98+
getBearing = () => 33;
99+
getPitch = () => 44;
91100
isStyleLoaded = () => true;
92101
off = vi.fn();
93102
on = vi.fn(() => {
@@ -128,7 +137,7 @@ vi.mock("maplibre-gl", () => {
128137
};
129138
});
130139

131-
import { useMapStore } from "@openmapx/core";
140+
import { useMapStore, useNavigationStore } from "@openmapx/core";
132141
import * as maplibre from "maplibre-gl";
133142
import * as mapContext from "@/lib/MapContext";
134143
import * as mapStyle from "@/lib/map";
@@ -138,7 +147,9 @@ const maplibreTest = (
138147
maplibre as unknown as {
139148
__test: {
140149
instances: Array<{
150+
cameraReads: number;
141151
jumpTo: ReturnType<typeof vi.fn>;
152+
on: ReturnType<typeof vi.fn>;
142153
remove: ReturnType<typeof vi.fn>;
143154
}>;
144155
options: Array<{ center: [number, number]; zoom: number }>;
@@ -166,8 +177,26 @@ afterEach(() => {
166177
console.error = originalConsoleError;
167178
vi.unstubAllGlobals();
168179
vi.clearAllMocks();
180+
useNavigationStore.getState().stopNavigation();
169181
});
170182

183+
/** Render a map and hand back its registered `moveend` listener. */
184+
async function renderWithMoveEnd() {
185+
maplibreTest.reset();
186+
mapStyleTest.reset();
187+
useMapStore.setState({ bearing: 0, center: [0, 20], pitch: 0, userLocation: null, zoom: 2 });
188+
vi.stubGlobal("navigator", { ...navigator, geolocation: undefined, permissions: undefined });
189+
190+
render(<MapCanvas />);
191+
await waitFor(() => expect(maplibreTest.instances).toHaveLength(1));
192+
const map = maplibreTest.instances[0];
193+
const moveEnd = map.on.mock.calls.find(([event]: unknown[]) => event === "moveend")?.[1] as (
194+
e?: unknown,
195+
) => void;
196+
expect(moveEnd).toBeTypeOf("function");
197+
return { map, moveEnd };
198+
}
199+
171200
describe("MapCanvas", () => {
172201
it("renders the base map without waiting for a granted geolocation callback", async () => {
173202
maplibreTest.reset();
@@ -252,4 +281,52 @@ describe("MapCanvas", () => {
252281
expect(mapContextTest.mapRef.current).toBeNull();
253282
expect(mapContextTest.notifyMapReady).not.toHaveBeenCalled();
254283
});
284+
285+
it("persists the viewport for a user-originated move", async () => {
286+
const { map, moveEnd } = await renderWithMoveEnd();
287+
const readsBefore = map.cameraReads;
288+
289+
act(() => moveEnd({}));
290+
291+
expect(map.cameraReads).toBe(readsBefore + 1);
292+
expect(useMapStore.getState()).toMatchObject({
293+
bearing: 33,
294+
center: [1.5, 2.5],
295+
pitch: 44,
296+
zoom: 12,
297+
});
298+
});
299+
300+
it("persists the viewport for a programmatic move outside navigation", async () => {
301+
const { moveEnd } = await renderWithMoveEnd();
302+
303+
act(() => moveEnd({ programmatic: true }));
304+
305+
expect(useMapStore.getState().center).toEqual([1.5, 2.5]);
306+
});
307+
308+
it("reads no camera state for its own programmatic move while navigating", async () => {
309+
const { map, moveEnd } = await renderWithMoveEnd();
310+
useNavigationStore.setState({ status: "navigating" });
311+
const readsBefore = map.cameraReads;
312+
313+
act(() => moveEnd({ programmatic: true }));
314+
315+
expect(map.cameraReads).toBe(readsBefore);
316+
expect(useMapStore.getState()).toMatchObject({
317+
bearing: 0,
318+
center: [0, 20],
319+
pitch: 0,
320+
zoom: 2,
321+
});
322+
});
323+
324+
it("still persists a user gesture while navigating", async () => {
325+
const { moveEnd } = await renderWithMoveEnd();
326+
useNavigationStore.setState({ status: "navigating" });
327+
328+
act(() => moveEnd({}));
329+
330+
expect(useMapStore.getState().center).toEqual([1.5, 2.5]);
331+
});
255332
});

apps/web/src/components/map/MapCanvas.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,20 @@ export function MapCanvas() {
133133
};
134134

135135
map.on("moveend", (e) => {
136-
const c = map.getCenter();
137-
const center: LngLat = [c.lng, c.lat];
138136
// The navigation follow camera drives the map with a programmatic
139137
// jumpTo every animation frame; skip those so we don't write to the
140-
// store 60×/s while navigating. User gestures and other programmatic
141-
// moves (flyTo, deep links) still persist as before.
138+
// store 60×/s while navigating. Bail before reading the camera at all:
139+
// getCenter() allocates, and this is the hottest listener on the map
140+
// during a trip. User gestures and other programmatic moves (flyTo,
141+
// deep links) still persist as before.
142142
if (
143143
(e as { programmatic?: boolean })?.programmatic &&
144144
useNavigationStore.getState().status !== "idle"
145145
) {
146146
return;
147147
}
148+
const c = map.getCenter();
149+
const center: LngLat = [c.lng, c.lat];
148150
setCenter(center);
149151
setZoom(map.getZoom());
150152
setBearing(map.getBearing());
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
import { describe, expect, it } from "vitest";
2+
import {
3+
CAMERA_BEARING_EPSILON,
4+
CAMERA_LNGLAT_EPSILON,
5+
CAMERA_ZOOM_EPSILON,
6+
cameraPoseChanged,
7+
PUCK_BEARING_EPSILON,
8+
PUCK_LNGLAT_EPSILON,
9+
puckPoseChanged,
10+
SETTLED_FRAMES_BEFORE_SLEEP,
11+
shouldKeepAnimating,
12+
} from "./navCameraScheduler";
13+
14+
const pose = (lng: number, lat: number, bearing: number) => ({ lng, lat, bearing });
15+
const cam = (lng: number, lat: number, bearing: number, zoom: number) => ({
16+
lng,
17+
lat,
18+
bearing,
19+
zoom,
20+
});
21+
22+
describe("puckPoseChanged", () => {
23+
it("treats a never-published pose as changed", () => {
24+
expect(puckPoseChanged(null, pose(0, 0, 0))).toBe(true);
25+
});
26+
27+
it("ignores drift below the puck threshold", () => {
28+
const last = pose(13.4, 52.5, 90);
29+
expect(puckPoseChanged(last, pose(13.4 + PUCK_LNGLAT_EPSILON / 2, 52.5, 90))).toBe(false);
30+
expect(puckPoseChanged(last, pose(13.4, 52.5 + PUCK_LNGLAT_EPSILON / 2, 90))).toBe(false);
31+
expect(puckPoseChanged(last, pose(13.4, 52.5, 90 + PUCK_BEARING_EPSILON / 2))).toBe(false);
32+
});
33+
34+
it("reports movement past the threshold on any axis", () => {
35+
const last = pose(13.4, 52.5, 90);
36+
expect(puckPoseChanged(last, pose(13.4 + 1e-8, 52.5, 90))).toBe(true);
37+
expect(puckPoseChanged(last, pose(13.4, 52.5 - 1e-8, 90))).toBe(true);
38+
expect(puckPoseChanged(last, pose(13.4, 52.5, 90.001))).toBe(true);
39+
});
40+
41+
it("publishes the metre-scale movement one frame of walking produces", () => {
42+
// ~1.4 m/s over a 16 ms frame is ~0.02 m, an order below the camera's own
43+
// threshold — the puck still has to track it frame by frame.
44+
const last = pose(13.4, 52.5, 90);
45+
expect(puckPoseChanged(last, pose(13.4 + 2e-7, 52.5, 90))).toBe(true);
46+
expect(
47+
cameraPoseChanged({ ...last, zoom: 16 }, { ...pose(13.4 + 2e-7, 52.5, 90), zoom: 16 }, true),
48+
).toBe(false);
49+
});
50+
51+
it("measures rotation along the shortest arc across north", () => {
52+
expect(puckPoseChanged(pose(0, 0, 359.99999), pose(0, 0, 0.0))).toBe(false);
53+
expect(puckPoseChanged(pose(0, 0, 359.5), pose(0, 0, 0.5))).toBe(true);
54+
});
55+
});
56+
57+
describe("cameraPoseChanged", () => {
58+
it("ignores sub-pixel centre and bearing drift", () => {
59+
const last = cam(13.4, 52.5, 90, 16);
60+
expect(cameraPoseChanged(last, cam(13.4 + CAMERA_LNGLAT_EPSILON / 2, 52.5, 90, 16), true)).toBe(
61+
false,
62+
);
63+
expect(
64+
cameraPoseChanged(last, cam(13.4, 52.5, 90 + CAMERA_BEARING_EPSILON / 2, 16), true),
65+
).toBe(false);
66+
expect(cameraPoseChanged(last, cam(13.4 + 2e-6, 52.5, 90, 16), true)).toBe(true);
67+
});
68+
69+
it("ignores a sub-threshold zoom drift while commanding zoom", () => {
70+
const last = cam(0, 0, 0, 16);
71+
expect(cameraPoseChanged(last, cam(0, 0, 0, 16 + CAMERA_ZOOM_EPSILON / 2), true)).toBe(false);
72+
expect(cameraPoseChanged(last, cam(0, 0, 0, 16.05), true)).toBe(true);
73+
});
74+
75+
it("never moves the camera for zoom alone once the user owns zoom", () => {
76+
const last = cam(0, 0, 0, 16);
77+
expect(cameraPoseChanged(last, cam(0, 0, 0, 18), false)).toBe(false);
78+
expect(cameraPoseChanged(last, cam(1e-5, 0, 0, 18), false)).toBe(true);
79+
});
80+
});
81+
82+
describe("shouldKeepAnimating", () => {
83+
const frame = (over: Partial<Parameters<typeof shouldKeepAnimating>[0]> = {}) =>
84+
shouldKeepAnimating({
85+
publishedThisFrame: false,
86+
settledFrames: SETTLED_FRAMES_BEFORE_SLEEP,
87+
holdUntilMs: 0,
88+
nowMs: 1000,
89+
...over,
90+
});
91+
92+
it("keeps running while anything was published", () => {
93+
expect(frame({ publishedThisFrame: true })).toBe(true);
94+
});
95+
96+
it("keeps running inside a hold window even with nothing to publish", () => {
97+
expect(frame({ holdUntilMs: 1001 })).toBe(true);
98+
expect(frame({ holdUntilMs: 1000 })).toBe(false);
99+
});
100+
101+
it("needs two settled frames before it sleeps", () => {
102+
expect(frame({ settledFrames: 1 })).toBe(true);
103+
expect(frame({ settledFrames: 2 })).toBe(false);
104+
});
105+
});
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/**
2+
* Visibility predicates for the navigation camera loop, kept pure and free of
3+
* MapLibre so the wake/sleep rule can be exercised without a map, a marker or a
4+
* frame clock.
5+
*
6+
* The loop has two jobs that are easy to conflate: integrating a pose every
7+
* frame, and publishing that pose to MapLibre. Publication is the expensive
8+
* half, so it happens only once a pose has moved far enough to be visible —
9+
* and when nothing has been published for a couple of frames there is, by
10+
* definition, nothing left to animate and the loop can stop asking for frames.
11+
*/
12+
13+
/**
14+
* Puck thresholds. Repositioning a DOM marker is cheap, so these sit far below
15+
* anything renderable (~0.1 mm, ~0.0001°) and exist only to recognise a pose
16+
* that has genuinely stopped converging. Anything a traveller can do — down to
17+
* a walking pace — clears them on every frame, so the puck's moving cadence is
18+
* exactly the frame cadence.
19+
*/
20+
export const PUCK_LNGLAT_EPSILON = 1e-9;
21+
export const PUCK_BEARING_EPSILON = 1e-4;
22+
/**
23+
* Camera thresholds. A `jumpTo` transforms and repaints the whole map, so it is
24+
* worth suppressing for a move nobody can see: ~0.1 m of centre drift, a
25+
* twentieth of a degree of rotation, and four thousandths of a zoom level.
26+
*/
27+
export const CAMERA_LNGLAT_EPSILON = 1e-6;
28+
export const CAMERA_BEARING_EPSILON = 0.05;
29+
export const CAMERA_ZOOM_EPSILON = 0.004;
30+
/**
31+
* Consecutive frames that must publish nothing before the loop sleeps. Two,
32+
* because the first frame after a wake runs with dt = 0 — the filters cannot
33+
* move on it, so a single settled frame is not evidence that the pose has
34+
* converged.
35+
*/
36+
export const SETTLED_FRAMES_BEFORE_SLEEP = 2;
37+
38+
export interface PuckPose {
39+
lng: number;
40+
lat: number;
41+
bearing: number;
42+
}
43+
44+
export interface CameraPose extends PuckPose {
45+
zoom: number;
46+
}
47+
48+
/** Shortest angular distance between two bearings, degrees, always positive. */
49+
function bearingDistance(a: number, b: number): number {
50+
return Math.abs(((a - b + 540) % 360) - 180);
51+
}
52+
53+
function poseChanged(
54+
last: PuckPose | null,
55+
next: PuckPose,
56+
lngLatEpsilon: number,
57+
bearingEpsilon: number,
58+
): boolean {
59+
return (
60+
!last ||
61+
Math.abs(next.lng - last.lng) > lngLatEpsilon ||
62+
Math.abs(next.lat - last.lat) > lngLatEpsilon ||
63+
bearingDistance(next.bearing, last.bearing) > bearingEpsilon
64+
);
65+
}
66+
67+
/**
68+
* Whether a puck pose has moved far enough since the last published one to be
69+
* worth another `setLngLat`/`setRotation`. A missing previous pose counts as
70+
* changed: the puck has never been placed, or a route/map swap invalidated it.
71+
*/
72+
export function puckPoseChanged(last: PuckPose | null, next: PuckPose): boolean {
73+
return poseChanged(last, next, PUCK_LNGLAT_EPSILON, PUCK_BEARING_EPSILON);
74+
}
75+
76+
/**
77+
* Whether a camera pose warrants another `jumpTo`. Zoom only participates while
78+
* the loop still commands zoom — once the user has taken zoom control the loop
79+
* leaves it alone, so a zoom delta must not by itself force a camera transform.
80+
*/
81+
export function cameraPoseChanged(
82+
last: CameraPose | null,
83+
next: CameraPose,
84+
commandsZoom: boolean,
85+
): boolean {
86+
if (poseChanged(last, next, CAMERA_LNGLAT_EPSILON, CAMERA_BEARING_EPSILON)) return true;
87+
return commandsZoom && !!last && Math.abs(next.zoom - last.zoom) > CAMERA_ZOOM_EPSILON;
88+
}
89+
90+
export interface FrameSettlement {
91+
/** Whether this frame moved the puck or the camera. */
92+
publishedThisFrame: boolean;
93+
/** Consecutive frames that published nothing, counting this one. */
94+
settledFrames: number;
95+
/**
96+
* `performance.now()` value until which the loop must keep running whatever
97+
* the pose does — the enter-follow ease and the post-gesture grace window
98+
* both end with a camera hand-back that nothing else would wake.
99+
*/
100+
holdUntilMs: number;
101+
nowMs: number;
102+
}
103+
104+
/** Whether the loop should request another frame after the one just run. */
105+
export function shouldKeepAnimating(frame: FrameSettlement): boolean {
106+
if (frame.nowMs < frame.holdUntilMs) return true;
107+
if (frame.publishedThisFrame) return true;
108+
return frame.settledFrames < SETTLED_FRAMES_BEFORE_SLEEP;
109+
}

0 commit comments

Comments
 (0)