Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .changeset/spatialcanvas-picking-perf-and-rules-of-react.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
"@spatialdata/vis": minor
"@spatialdata/layers": patch
"@spatialdata/react": patch
---

SpatialCanvas hover/picking performance and Rules-of-React cleanup.

Picking/tooltip performance:

- New `hoverTooltipMode` prop (`'off' | 'simple' | 'aggregate'`, default
`'simple'`) on `SpatialCanvas` and `SpatialCanvasViewer`, with a matching
selector in the `SpatialCanvas` UI. `'simple'` resolves the tooltip from the
single top-most pick deck.gl already does for hover/highlight; `'aggregate'`
adds `pickMultipleObjects` GPU passes to include every layer under the cursor
(more expensive); `'off'` makes shape layers non-pickable entirely (no
autoHighlight, no picking-buffer render) — the cheapest mode. Replaces the
earlier boolean `aggregateHoverTooltips`.
- Shape layers are made non-pickable (and `autoHighlight` disabled) while the
camera is being panned/zoomed, so deck.gl does not re-render the shape
geometry into the picking buffer during gestures. New `pickingEnabled` option
on the shapes layer (`@spatialdata/layers`) drives this.
- Hover tooltip resolution is suppressed while a pointer button is held (drag),
and the per-missing-layer supplemental aggregation pick is collapsed into a
single batched pick.

Rules-of-React fixes (eslint-plugin-react-hooks, `pnpm lint:react` now clean and
the `react-lint` CI job is required): removed ref reads/writes during render and
replaced setState-in-effect patterns with derived state in `@spatialdata/react`
`useSpatialData` and the vis `Transforms`, `Table`, `Shapes`, `ImageView`, and
`SpatialCanvas` components.
1 change: 1 addition & 0 deletions .claude/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"name": "vis-demo",
"runtimeExecutable": "pnpm",
"runtimeArgs": ["--filter", "@spatialdata/vis", "dev:demo"],
"autoPort": true,
"port": 5173
},
{
Expand Down
11 changes: 3 additions & 8 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ jobs:
runs-on: ubuntu-latest
permissions:
contents: read
# Surfaces React Hooks / React Compiler (Rules-of-React) findings for the
# React-shipping packages via eslint-plugin-react-hooks. Informational for
# now: there is an existing backlog of findings, so this must not gate merges.
# Enforces React Hooks / React Compiler (Rules-of-React) compliance for the
# React-shipping packages via eslint-plugin-react-hooks. The backlog has been
# cleared, so this is a required gate: any new finding fails the check.
steps:
- uses: actions/checkout@v4
with:
Expand All @@ -39,11 +39,6 @@ jobs:
run: pnpm install --frozen-lockfile

- name: Lint React packages (react-hooks / react-compiler rules)
# continue-on-error (at the step, not the job) keeps this check green
# while the backlog exists, so it does not read as a failing/required
# check; findings still show in this step's log. Remove this line once
# `pnpm lint:react` is clean to turn it into a required gate.
continue-on-error: true
run: pnpm lint:react

test:
Expand Down
14 changes: 10 additions & 4 deletions packages/layers/src/shapesLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,12 @@ export interface CreateShapesDeckLayerOptions {
spatialCoordinateSystem?: string | null;
onShapeHover?: (event: ShapesLayerPickEvent) => void;
onShapeClick?: (event: ShapesLayerPickEvent) => void;
/**
* When false, the layer is rendered non-pickable with autoHighlight disabled.
* Used to suppress deck.gl's per-pointer-move picking-buffer render over large
* shape geometry while the camera is being panned/zoomed. Defaults to true.
*/
pickingEnabled?: boolean;
}

function multiplyAlpha(
Expand Down Expand Up @@ -603,8 +609,8 @@ function createPolygonDeckLayer(
stroked: true,
opacity: options.opacity ?? 1,
modelMatrix: options.modelMatrix,
pickable: true,
autoHighlight: true,
pickable: options.pickingEnabled ?? true,
autoHighlight: options.pickingEnabled ?? true,
highlightColor: [255, 255, 0, 128],
onHover: createPickHandler(
options.id,
Expand Down Expand Up @@ -648,8 +654,8 @@ function createCircleDeckLayer(
},
opacity: options.opacity ?? 1,
modelMatrix: options.modelMatrix,
pickable: true,
autoHighlight: true,
pickable: options.pickingEnabled ?? true,
autoHighlight: options.pickingEnabled ?? true,
highlightColor: [255, 255, 0, 128],
onHover: createPickHandler(
options.id,
Expand Down
52 changes: 35 additions & 17 deletions packages/react/src/hooks/useSpatialData.ts
Original file line number Diff line number Diff line change
@@ -1,36 +1,54 @@
import { useEffect, useState } from 'react';
import type { SpatialData } from '@spatialdata/core';
import { useEffect, useState } from 'react';
import { useSpatialDataContext } from '../provider/SpatialDataProvider';

type ResolvedSpatialData = {
/** The promise this result was produced from, used to detect stale results. */
promise: Promise<SpatialData> | null;
spatialData: SpatialData | null;
error: Error | null;
};

export function useSpatialData() {
const { spatialDataPromise } = useSpatialDataContext();
const [spatialData, setSpatialData] = useState<SpatialData | null>(null);
const [error, setError] = useState<Error | null>(null);
const [loading, setLoading] = useState<boolean>(true);
// Track which promise each settled result came from so loading/reset can be
// derived during render rather than synchronised with a setState-in-effect.
const [resolved, setResolved] = useState<ResolvedSpatialData>({
promise: null,
spatialData: null,
error: null,
});

useEffect(() => {
if (!spatialDataPromise) return;
let cancelled = false;
setLoading(true);
setError(null);
setSpatialData(null);
if (!spatialDataPromise) {
setLoading(false);
return;
}
spatialDataPromise
.then((s) => {
if (!cancelled) setSpatialData(s);
if (!cancelled) setResolved({ promise: spatialDataPromise, spatialData: s, error: null });
})
.catch((e: unknown) => {
if (!cancelled) setError(e instanceof Error ? e : new Error(String(e)));
})
.finally(() => {
if (!cancelled) setLoading(false);
if (!cancelled) {
setResolved({
promise: spatialDataPromise,
spatialData: null,
error: e instanceof Error ? e : new Error(String(e)),
});
}
});
return () => {
cancelled = true;
};
}, [spatialDataPromise]);

return { spatialData, loading, error } as const;
// When the current promise hasn't settled into `resolved` yet, we're loading
// (or idle, if there is no promise). Deriving this avoids resetting state in
// an effect every time `spatialDataPromise` changes.
const settled = resolved.promise === spatialDataPromise;
const loading = Boolean(spatialDataPromise) && !settled;

return {
spatialData: settled ? resolved.spatialData : null,
loading,
error: settled ? resolved.error : null,
} as const;
}
2 changes: 1 addition & 1 deletion packages/vis/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"build": "vite build && tsc --noEmit",
"dev": "node scripts/dev.mjs",
"dev:stop": "node ../../scripts/dev-stop.mjs",
"dev:demo": "vite --config vite.config.demo.ts --host 127.0.0.1 --port 5173 --strictPort",
"dev:demo": "vite --config vite.config.demo.ts",
"watch": "vite build --watch",
"test": "vitest run",
"test:watch": "vitest",
Expand Down
27 changes: 10 additions & 17 deletions packages/vis/scripts/dev.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,18 @@ async function fixtureServerIsOurs(port) {
}

/**
* Check the ports we need before spawning anything, so a leftover dev server
* (often from another checkout) produces a clear message instead of a cryptic
* strict-port crash. Returns whether we still need to start the fixture server.
* Check the ports we need before spawning anything: a busy demo port just gets a
* heads-up (Vite falls back to a free one), while a foreign process on the fixed
* fixture port is fatal. Returns whether we still need to start the fixture server.
*/
async function preflight() {
if (await isPortInUse(DEMO_PORT)) {
const holder = describePortHolder(DEMO_PORT);
console.error(
`\n[dev] Demo port ${DEMO_PORT} is already in use${holder ? `:\n ${holder}` : '.'}`
console.log(
`[dev] Demo port ${DEMO_PORT} is already in use${
holder ? ` (${holder})` : ''
}; Vite will fall back to the next free port.`
);
console.error('[dev] Run `pnpm dev:stop` to clear SpatialData dev processes, then retry.\n');
process.exit(1);
}

if (await isPortInUse(FIXTURE_SERVER_PORT)) {
Expand Down Expand Up @@ -126,13 +126,6 @@ if (startFixtures) {
}
console.log(`Starting ${startedParts.join(', ')}...`);
start('watch', ['vite', 'build', '--watch']);
start('demo', [
'vite',
'--config',
'vite.config.demo.ts',
'--host',
'127.0.0.1',
'--port',
String(DEMO_PORT),
'--strictPort',
]);
// Host/port (and the free-port fallback) are owned by vite.config.demo.ts so
// the port selection stays in one place; honour PORT if the caller pins it.
start('demo', ['vite', '--config', 'vite.config.demo.ts']);
27 changes: 10 additions & 17 deletions packages/vis/src/ImageView/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -148,31 +148,24 @@ export default function ImageView() {
const [selectedImage, setSelectedImage] = useState<string>('');
const [ref, { width, height }] = useMeasure();

useEffect(() => {
if (!spatialData?.images) return;
if (selectedImage === '' || !spatialData.images[selectedImage]) {
setSelectedImage(Object.keys(spatialData.images)[0]);
}
}, [spatialData?.images, selectedImage]);
const imageKeys = useMemo(() => Object.keys(spatialData?.images ?? {}), [spatialData?.images]);
// Default to the first available image, derived during render.
const effectiveImage =
selectedImage && imageKeys.includes(selectedImage) ? selectedImage : (imageKeys[0] ?? '');

const vivStores = useMemo(() => {
return createVivStores();
}, []);
const image = useMemo(() => {
return spatialData?.images?.[selectedImage];
}, [selectedImage, spatialData?.images]);
const [imageUrl, setImageUrl] = useState<string | URL>();
useEffect(() => {
if (image) {
setImageUrl(image.url ?? '');
} else {
setImageUrl('');
}
}, [image]);
return spatialData?.images?.[effectiveImage];
}, [effectiveImage, spatialData?.images]);
// The url is synchronously available on the image, so derive it rather than
// syncing through state in an effect.
const imageUrl = useMemo<string | URL>(() => image?.url ?? '', [image]);
return (
<div ref={ref} style={containerStyle}>
{spatialData?.images && (
<select value={selectedImage || ''} onChange={(e) => setSelectedImage(e.target.value)}>
<select value={effectiveImage} onChange={(e) => setSelectedImage(e.target.value)}>
{Object.keys(spatialData.images).map((key) => (
<option key={key} value={key}>
{key}
Expand Down
57 changes: 26 additions & 31 deletions packages/vis/src/Shapes/index.tsx
Original file line number Diff line number Diff line change
@@ -1,52 +1,47 @@
import { useSpatialData } from '@spatialdata/react';
import JsonView from '@uiw/react-json-view';
import { darkTheme } from '@uiw/react-json-view/dark';
import { useEffect, useMemo, useState } from 'react';
import { useMemo, useState } from 'react';
// import type { Table } from "@spatialdata/core";

export default function ShapesComponent() {
const { spatialData } = useSpatialData();
const [selectedShapes, setSelectedShapes] = useState<string>('');
const shapeKeys = useMemo(() => Object.keys(spatialData?.shapes ?? {}), [spatialData?.shapes]);

// Default to first available shape
useEffect(() => {
if (shapeKeys.length > 0 && (!selectedShapes || !shapeKeys.includes(selectedShapes))) {
setSelectedShapes(shapeKeys[0]);
}
}, [shapeKeys, selectedShapes]);
// Default to the first available shape, derived during render.
const effectiveShapes =
selectedShapes && shapeKeys.includes(selectedShapes) ? selectedShapes : (shapeKeys[0] ?? '');

const shapes = useMemo(() => {
return spatialData?.shapes?.[selectedShapes];
}, [selectedShapes, spatialData?.shapes]);
const [shapesData, setShapesData] = useState<any>(undefined);
useEffect(() => {
if (shapes) {
const result = shapes.getTransformation();
if (result.ok) {
const t = result.value;
setShapesData({
type: t.type,
input: t.input,
output: t.output,
matrix: t.toArray(),
});
} else {
// Show the error info
setShapesData({
error: result.error.message,
availableCoordinateSystems: result.error.availableCoordinateSystems,
});
}
} else {
setShapesData(undefined);
return spatialData?.shapes?.[effectiveShapes];
}, [effectiveShapes, spatialData?.shapes]);

// getTransformation() is synchronous, so the displayed data is pure derived
// state rather than something to sync into an effect.
const shapesData = useMemo(() => {
if (!shapes) return undefined;
const result = shapes.getTransformation();
if (result.ok) {
const t = result.value;
return {
type: t.type,
input: t.input,
output: t.output,
matrix: t.toArray(),
};
}
// Show the error info
return {
error: result.error.message,
availableCoordinateSystems: result.error.availableCoordinateSystems,
};
}, [shapes]);
return (
<div>
<h3>Shapes component:</h3>
{spatialData?.shapes && (
<select value={selectedShapes || ''} onChange={(e) => setSelectedShapes(e.target.value)}>
<select value={effectiveShapes} onChange={(e) => setSelectedShapes(e.target.value)}>
{Object.keys(spatialData.shapes).map((key) => (
<option key={key} value={key}>
{key}
Expand Down
Loading
Loading