Skip to content

Commit

Permalink
fix: add map camera state tracking (#84)
Browse files Browse the repository at this point in the history
Camera state tracking allows the Map component to avoid a feedback-loop when camera values published by the map are fed back into the Map component props.

I measured round-trip times (dispatching an event into the react-application and receiving them back as props) of around 4-5ms. However, there are cases where this roundtrip crosses the frame-boundary, and times more like 20ms can be observed. When this happens, stuttering, janky animations and suboptimal interactions will happen.
  • Loading branch information
usefulthink committed Nov 13, 2023
1 parent b87ba05 commit 1dc1584
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 12 deletions.
13 changes: 13 additions & 0 deletions src/components/__tests__/map.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,16 @@ describe('creating and updating map instance', () => {
expect(options).toMatchObject({mapId: 'othermapid'});
});
});

describe('map events and event-props', () => {
test.todo('events dispatched by the map are received via event-props');
});

describe('camera updates', () => {
test.todo('initial camera state is passed via mapOptions, not moveCamera');
test.todo('updated camera state is passed to moveCamera');
test.todo("re-renders with unchanged camera state don't trigger moveCamera");
test.todo(
"re-renders with props received via events don't trigger moveCamera"
);
});
6 changes: 4 additions & 2 deletions src/components/map/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {useCallbackRef} from '../../libraries/use-callback-ref';
import {MapEventProps, useMapEvents} from './use-map-events';
import {useMapOptions} from './use-map-options';
import {useDeckGLCameraUpdate} from './use-deckgl-camera-update';
import {useInternalCameraState} from './use-internal-camera-state';

export interface GoogleMapsContextValue {
map: google.maps.Map | null;
Expand Down Expand Up @@ -73,8 +74,9 @@ export const Map = (props: PropsWithChildren<MapProps>) => {
}

const [map, mapRef] = useMapInstance(props, context);
useMapOptions(map, props);
useMapEvents(map, props);
const cameraStateRef = useInternalCameraState();
useMapOptions(map, cameraStateRef, props);
useMapEvents(map, cameraStateRef, props);
useDeckGLCameraUpdate(map, viewState);

const isViewportSet = useMemo(() => Boolean(viewport), [viewport]);
Expand Down
47 changes: 47 additions & 0 deletions src/components/map/use-internal-camera-state.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import {MutableRefObject, useRef} from 'react';
import {MapCameraChangedEvent, MapEvent} from './use-map-events';

export type InternalCameraState = {
center: google.maps.LatLngLiteral;
heading: number;
tilt: number;
zoom: number;
};

export type InternalCameraStateRef = MutableRefObject<InternalCameraState>;

/**
* Creates a mutable ref object to track the last known state of the map camera.
* This is updated by `trackDispatchedEvent` and used in `useMapOptions`.
*/
export function useInternalCameraState(): InternalCameraStateRef {
return useRef<InternalCameraState>({
center: {lat: 0, lng: 0},
heading: 0,
tilt: 0,
zoom: 0
});
}

/**
* Records camera data from the last event dispatched to the React application
* in a mutable `IternalCameraStateRef`.
* This data can then be used to prevent feeding these values back to the
* map-instance when a typical "controlled component" setup (state variable is
* fed into and updated by the map).
*/
export function trackDispatchedEvent(
ev: MapEvent,
cameraStateRef: InternalCameraStateRef
) {
const cameraEvent = ev as MapCameraChangedEvent;

// we're only interested in the camera-events here
if (!cameraEvent.detail.center) return;
const {center, zoom, heading, tilt} = cameraEvent.detail;

cameraStateRef.current.center = center;
cameraStateRef.current.heading = heading;
cameraStateRef.current.tilt = tilt;
cameraStateRef.current.zoom = zoom;
}
14 changes: 11 additions & 3 deletions src/components/map/use-map-events.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import {useEffect} from 'react';
import {
InternalCameraStateRef,
trackDispatchedEvent
} from './use-internal-camera-state';

/**
* Handlers for all events that could be emitted by map-instances.
Expand Down Expand Up @@ -40,6 +44,7 @@ export type MapEventProps = Partial<{
*/
export function useMapEvents(
map: google.maps.Map | null,
cameraStateRef: InternalCameraStateRef,
props: MapEventProps
) {
// note: calling a useEffect hook from within a loop is prohibited by the
Expand All @@ -61,12 +66,15 @@ export function useMapEvents(
const listener = map.addListener(
eventType,
(ev?: google.maps.MapMouseEvent | google.maps.IconMouseEvent) => {
handler(createMapEvent(eventType, map, ev));
const mapEvent = createMapEvent(eventType, map, ev);

trackDispatchedEvent(mapEvent, cameraStateRef);
handler(mapEvent);
}
);

return () => listener.remove();
}, [map, eventType, handler]);
}, [map, cameraStateRef, eventType, handler]);
}
}

Expand Down Expand Up @@ -189,7 +197,7 @@ const mouseEventTypes = [
type MapEventPropName = keyof MapEventProps;
const eventPropNames = Object.keys(propNameToEventType) as MapEventPropName[];

type MapEvent<T = unknown> = {
export type MapEvent<T = unknown> = {
type: string;
map: google.maps.Map;
detail: T;
Expand Down
49 changes: 42 additions & 7 deletions src/components/map/use-map-options.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,32 @@
import {useEffect, useLayoutEffect} from 'react';
import {MapProps} from '@vis.gl/react-google-maps';
import {InternalCameraStateRef} from './use-internal-camera-state';
import {isLatLngLiteral} from '../../libraries/is-lat-lng-literal';

/**
* Internal hook to update the map-options and view-parameters when
* Internal hook to update the map-options and camera parameters when
* props are changed.
*
* @param map the map instance
* @param cameraStateRef stores the last values seen during dispatch into the
* react-application in useMapEvents(). We can safely assume that we
* don't need to feed these values back into the map.
* @param mapProps the props to update the map-instance with
* @internal
*/
export function useMapOptions(map: google.maps.Map | null, mapProps: MapProps) {
const {center, zoom, heading, tilt, ...mapOptions} = mapProps;
export function useMapOptions(
map: google.maps.Map | null,
cameraStateRef: InternalCameraStateRef,
mapProps: MapProps
) {
const {center: rawCenter, zoom, heading, tilt, ...mapOptions} = mapProps;
const center = rawCenter
? isLatLngLiteral(rawCenter)
? rawCenter
: rawCenter.toJSON()
: null;
const lat = center && center.lat;
const lng = center && center.lng;

/* eslint-disable react-hooks/exhaustive-deps --
*
Expand All @@ -17,32 +36,48 @@ export function useMapOptions(map: google.maps.Map | null, mapProps: MapProps) {
*/

// update the map options when mapOptions is changed
// Note: due to the destructuring above, mapOptions will be seen as changed
// with every re-render, so we're boldly assuming the maps-api will properly
// deal with unchanged option-values passed into setOptions.
useEffect(() => {
if (!map) return;

map.setOptions(mapOptions);
// Changing the mapId via setOptions will trigger an error-message.
// We will re-create the map-instance in that case anyway, so we
// remove it here to avoid this error-message.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const {mapId, ...opts} = mapOptions;
map.setOptions(opts);
}, [mapOptions]);

useLayoutEffect(() => {
if (!map || !center) return;
if (!map || !Number.isFinite(lat) || !Number.isFinite(lng)) return;
if (
cameraStateRef.current.center.lat === lat &&
cameraStateRef.current.center.lng === lng
)
return;

map.moveCamera({center});
}, [center]);
map.moveCamera({center: {lat: lat as number, lng: lng as number}});
}, [lat, lng]);

useLayoutEffect(() => {
if (!map || !Number.isFinite(zoom)) return;
if (cameraStateRef.current.zoom === zoom) return;

map.moveCamera({zoom: zoom as number});
}, [zoom]);

useLayoutEffect(() => {
if (!map || !Number.isFinite(heading)) return;
if (cameraStateRef.current.heading === heading) return;

map.moveCamera({heading: heading as number});
}, [heading]);

useLayoutEffect(() => {
if (!map || !Number.isFinite(tilt)) return;
if (cameraStateRef.current.tilt === tilt) return;

map.moveCamera({tilt: tilt as number});
}, [tilt]);
Expand Down
8 changes: 8 additions & 0 deletions src/libraries/is-lat-lng-literal.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export function isLatLngLiteral(
obj: unknown
): obj is google.maps.LatLngLiteral {
if (!obj || typeof obj !== 'object') return false;
if (!('lat' in obj && 'lng' in obj)) return false;

return Number.isFinite(obj.lat) && Number.isFinite(obj.lng);
}

0 comments on commit 1dc1584

Please sign in to comment.