From 4586ddfab8e11d1f73116c6c57deedf5164a16a1 Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Tue, 21 Feb 2023 13:40:31 +0100 Subject: [PATCH 1/2] Add story with annotation that follows pointer ... to test that `useCameraState` updates state when dependencies change --- apps/storybook/src/Annotation.stories.tsx | 52 ++++++++++++++----- .../src/decorators/DefaultCanvas.tsx | 16 ++++++ apps/storybook/src/utils.ts | 2 +- 3 files changed, 55 insertions(+), 15 deletions(-) create mode 100644 apps/storybook/src/decorators/DefaultCanvas.tsx diff --git a/apps/storybook/src/Annotation.stories.tsx b/apps/storybook/src/Annotation.stories.tsx index 8018765b4..010c5fde3 100644 --- a/apps/storybook/src/Annotation.stories.tsx +++ b/apps/storybook/src/Annotation.stories.tsx @@ -1,14 +1,16 @@ -import { Annotation, DefaultInteractions, VisCanvas } from '@h5web/lib'; +import type { CanvasEvent } from '@h5web/lib'; +import { Annotation, useCanvasEvents } from '@h5web/lib'; +import { useRafState } from '@react-hookz/web'; import type { Meta, Story } from '@storybook/react'; +import { useCallback } from 'react'; +import type { Vector3 } from 'three'; +import DefaultCanvas from './decorators/DefaultCanvas'; import FillHeight from './decorators/FillHeight'; +import { formatCoord } from './utils'; export const Default: Story = () => ( - - + <> HTML annotation positioned at (10, 16) @@ -67,15 +69,11 @@ export const Default: Story = () => ( - + ); export const WithZoom: Story = () => ( - - + <> HTML annotation at (10, 16) that scales with zoom. @@ -89,11 +87,37 @@ export const WithZoom: Story = () => ( Another annotation that scales with zoom but this time{' '} centred on (25, 10) - + ); +export const FollowPointer: Story = () => { + const [coords, setCoords] = useRafState(); + + const onPointerMove = useCallback( + (evt: CanvasEvent) => { + setCoords(evt.dataPt); + }, + [setCoords] + ); + + useCanvasEvents({ onPointerMove }); + + if (!coords) { + return <>; // eslint-disable-line react/jsx-no-useless-fragment + } + + const { x, y } = coords; + return ( + {`x=${formatCoord(x)}, y=${formatCoord(y)}`} + ); +}; + export default { title: 'Building Blocks/Annotation', parameters: { layout: 'fullscreen' }, - decorators: [FillHeight], + decorators: [DefaultCanvas, FillHeight], } as Meta; diff --git a/apps/storybook/src/decorators/DefaultCanvas.tsx b/apps/storybook/src/decorators/DefaultCanvas.tsx new file mode 100644 index 000000000..668809858 --- /dev/null +++ b/apps/storybook/src/decorators/DefaultCanvas.tsx @@ -0,0 +1,16 @@ +import { DefaultInteractions, VisCanvas } from '@h5web/lib'; +import type { Story } from '@storybook/react'; + +function DefaultCanvas(MyStory: Story) { + return ( + + + + + ); +} + +export default DefaultCanvas; diff --git a/apps/storybook/src/utils.ts b/apps/storybook/src/utils.ts index 724f37b37..17cabf306 100644 --- a/apps/storybook/src/utils.ts +++ b/apps/storybook/src/utils.ts @@ -1,7 +1,7 @@ import type { Rect } from '@h5web/lib'; import { format } from 'd3-format'; -const formatCoord = format('.2f'); +export const formatCoord = format('.2f'); export function getTitleForSelection(selection: Rect | undefined) { if (!selection) { From 9ff050a2126571fa8e8179a072d085dd9ffba0f3 Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Tue, 21 Feb 2023 13:42:07 +0100 Subject: [PATCH 2/2] Fix regression in `useCameraState` and improve perf --- apps/storybook/src/Utilities.stories.mdx | 20 ++++++++++--- eslint.shared.js | 7 +++++ .../src/toolbar/floating/ResetZoomButton.tsx | 5 +++- packages/lib/src/vis/hooks.ts | 29 ++++++++++++++----- packages/lib/src/vis/shared/Annotation.tsx | 11 ++++--- packages/lib/src/vis/shared/AxisSystem.tsx | 6 ++-- packages/lib/src/vis/shared/DataToHtml.tsx | 7 +++-- .../lib/src/vis/tiles/TiledHeatmapMesh.tsx | 6 ++-- 8 files changed, 65 insertions(+), 26 deletions(-) diff --git a/apps/storybook/src/Utilities.stories.mdx b/apps/storybook/src/Utilities.stories.mdx index 4562c0e8d..b5cf4a78b 100644 --- a/apps/storybook/src/Utilities.stories.mdx +++ b/apps/storybook/src/Utilities.stories.mdx @@ -203,7 +203,7 @@ useCanvasEvents({ onPointerDown: handlePointerDown }); // also supported: `onPoi #### useCameraState -Compute and update a local React state on every React Three Fiber frame. Useful to re-render React components when the user pans/zooms. +Compute and update a state on every React Three Fiber frame. Useful to re-render React components when the user pans/zooms. The hook accepts a callback, which: - receives the `camera` object; @@ -216,11 +216,22 @@ useCameraState( ): T const { dataToHtml } = useVisCanvasContext(); -const htmlPt = useCameraState((camera) => dataToHtml(camera, new Vector3(x, y))); +const htmlPt = useCameraState( + (camera) => dataToHtml(camera, new Vector3(x, y)), + [x, y, dataToHtml] +); +``` + +`useCameraState` accepts a dependency array as second parameter (similarly to `useCallback`) and recomputes the state synchronously whenever any dependency changes. Make sure to configure ESLint's +[`react-hooks/exhaustive-deps` rule](https://github.com/facebook/react/tree/main/packages/eslint-plugin-react-hooks#advanced-configuration) +in your app so it can report missing dependencies: + +```json +"react-hooks/exhaustive-deps": ["error", { "additionalHooks": "(useCameraState)" }] ``` -Internally, `useCameraState` uses React's `useState` and R3F's [`useFrame` hooks](https://docs.pmnd.rs/react-three-fiber/api/hooks#useframe). -By default, React performs a strict equality check to avoid re-rendering the same state twice in a row. If your `factory` function returns a new object reference on every call, +Internally, `useCameraState` uses R3F's [`useFrame` hooks](https://docs.pmnd.rs/react-three-fiber/api/hooks#useframe). +By default, it performs a strict equality check with [`Object.is()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is) to avoid re-rendering the same state twice in a row. If your `factory` function returns a new object reference on every call, you can avoid unnecessary re-renders by providing your own equality function. Note, however, that this optimisation is pointless if `factory` always returns a different state when the camera changes. Example: ```ts @@ -229,6 +240,7 @@ you can avoid unnecessary re-renders by providing your own equality function. No * the previous and next `pt` have the same coordinates. */ const pt = useCameraState( (camera) => camera.scale > 0.5 ? camera.position.clone() : new Vector3()), + [], // no dependencies (prevPt, nextPt) => prevPt.equals(nextPt) ); ``` diff --git a/eslint.shared.js b/eslint.shared.js index b78c714c4..a594f75db 100644 --- a/eslint.shared.js +++ b/eslint.shared.js @@ -48,6 +48,13 @@ module.exports = { rules: { 'react/jsx-no-constructed-context-values': 'off', // too strict 'react/no-unknown-property': 'off', // false positives with R3F + + // `useCameraState` accepts an array of deps like `useEffect` + // https://github.com/facebook/react/tree/main/packages/eslint-plugin-react-hooks#advanced-configuration + 'react-hooks/exhaustive-deps': [ + 'error', + { additionalHooks: '(useCameraState)' }, + ], }, }), createTypeScriptOverride({ diff --git a/packages/lib/src/toolbar/floating/ResetZoomButton.tsx b/packages/lib/src/toolbar/floating/ResetZoomButton.tsx index 80b1b4da4..c49fe7155 100644 --- a/packages/lib/src/toolbar/floating/ResetZoomButton.tsx +++ b/packages/lib/src/toolbar/floating/ResetZoomButton.tsx @@ -8,7 +8,10 @@ function ResetZoomButton() { const camera = useThree((state) => state.camera); const invalidate = useThree((state) => state.invalidate); - const isZoomedIn = useCameraState(({ scale }) => scale.x < 1 || scale.y < 1); + const isZoomedIn = useCameraState( + ({ scale }) => scale.x < 1 || scale.y < 1, + [] + ); function resetZoom() { camera.scale.x = 1; diff --git a/packages/lib/src/vis/hooks.ts b/packages/lib/src/vis/hooks.ts index 7a00f6e78..3abd4cb93 100644 --- a/packages/lib/src/vis/hooks.ts +++ b/packages/lib/src/vis/hooks.ts @@ -5,10 +5,10 @@ import { createMemo, } from '@h5web/shared'; import type { Domain, AnyNumArray } from '@h5web/shared'; -import { useSyncedRef } from '@react-hookz/web'; +import { useRerender, useSyncedRef } from '@react-hookz/web'; import type { Camera } from '@react-three/fiber'; import { useFrame, useThree } from '@react-three/fiber'; -import { useCallback, useMemo, useState } from 'react'; +import { useCallback, useMemo, useRef, useState } from 'react'; import type { RefCallback } from 'react'; import { @@ -53,21 +53,34 @@ export function useDomains( export function useCameraState( factory: (camera: Camera) => T, - equalityFn?: (prev: T, next: T) => boolean + deps: unknown[], + equalityFn = (prev: T, next: T) => Object.is(prev, next) ): T { const camera = useThree((state) => state.camera); + const rerender = useRerender(); - const factoryRef = useSyncedRef(factory); - const [state, setState] = useState(() => factoryRef.current(camera)); + const stateRef = useRef(); // ref instead of state to avoid re-render when deps change + const factoryRef = useSyncedRef(factory); // ensure `useMemo` always sees latest `factory` reference + + useMemo(() => { + // Compute state synchronously when deps change + stateRef.current = factoryRef.current(camera); + }, deps); // eslint-disable-line react-hooks/exhaustive-deps useFrame(() => { + // Recompute state and re-render on every frame const next = factoryRef.current(camera); - if (!equalityFn || !equalityFn(state, next)) { - setState(next); + + // ... unless state hasn't changed + if (equalityFn(stateRef.current as T, next)) { + return; } + + stateRef.current = next; + rerender(); }); - return state; + return stateRef.current as T; // synchronous update in `useMemo` guarantees `T` (which can include `undefined`) } export function useCssColors( diff --git a/packages/lib/src/vis/shared/Annotation.tsx b/packages/lib/src/vis/shared/Annotation.tsx index 8110aa950..5510648ac 100644 --- a/packages/lib/src/vis/shared/Annotation.tsx +++ b/packages/lib/src/vis/shared/Annotation.tsx @@ -32,10 +32,13 @@ function Annotation(props: Props) { } const { dataToHtml } = useVisCanvasContext(); - const { htmlPt, cameraScale } = useCameraState((camera) => ({ - htmlPt: dataToHtml(camera, new Vector3(x, y)), - cameraScale: camera.scale.clone(), - })); + const { htmlPt, cameraScale } = useCameraState( + (camera) => ({ + htmlPt: dataToHtml(camera, new Vector3(x, y)), + cameraScale: camera.scale.clone(), + }), + [x, y, dataToHtml] + ); const transforms = [ center ? 'translate(-50%, -50%)' : '', diff --git a/packages/lib/src/vis/shared/AxisSystem.tsx b/packages/lib/src/vis/shared/AxisSystem.tsx index 51f5c03bb..3d2ef8baa 100644 --- a/packages/lib/src/vis/shared/AxisSystem.tsx +++ b/packages/lib/src/vis/shared/AxisSystem.tsx @@ -16,7 +16,7 @@ function AxisSystem(props: Props) { const { canvasSize, abscissaConfig, ordinateConfig, getVisibleDomains } = useVisCanvasContext(); - const { xVisibleDomain, yVisibleDomain } = useCameraState(getVisibleDomains); + const visibleDomains = useCameraState(getVisibleDomains, [getVisibleDomains]); return ( // Append to `canvasWrapper` instead of `r3fRoot` @@ -32,7 +32,7 @@ function AxisSystem(props: Props) { (props: Props) { const { points, children } = props; const { dataToHtml } = useVisCanvasContext(); - const htmlPoints = useCameraState((camera) => { - return points.map((pt) => dataToHtml(camera, pt)) as MappedTuple; - }); + const htmlPoints = useCameraState( + (camera) => points.map((pt) => dataToHtml(camera, pt)) as MappedTuple, + [points, dataToHtml] + ); return <>{children(...htmlPoints)}; } diff --git a/packages/lib/src/vis/tiles/TiledHeatmapMesh.tsx b/packages/lib/src/vis/tiles/TiledHeatmapMesh.tsx index 685627d09..4d3496cb8 100644 --- a/packages/lib/src/vis/tiles/TiledHeatmapMesh.tsx +++ b/packages/lib/src/vis/tiles/TiledHeatmapMesh.tsx @@ -1,6 +1,6 @@ import { clamp, range } from 'lodash'; import { useRef } from 'react'; -import type { Object3D } from 'three'; +import type { Group } from 'three'; import { getInterpolator } from '../heatmap/utils'; import { useCameraState } from '../hooks'; @@ -37,11 +37,11 @@ function TiledHeatmapMesh(props: Props) { const { canvasSize, visSize } = useVisCanvasContext(); const meshSize = size ?? visSize; - const groupRef = useRef(null); + const groupRef = useRef(null); const ndcToLocalMatrix = useCameraState((camera) => { return getNdcToObject3DMatrix(camera, groupRef); - }); + }, []); const visibleBox = getObject3DVisibleBox(ndcToLocalMatrix); const bounds = scaleBoxToLayer(visibleBox, baseLayerSize, meshSize);