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
17 changes: 17 additions & 0 deletions docs/docs/layers/overview.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,20 @@ See also the [visualization overview](../vis/overview): deck-only integrators ca
- the public shapes config stays representation-agnostic so a stronger future `deck.gl-geoarrow` backend can slot in without changing saved props

For shapes, the important contract is stable feature identity plus table-join-driven styling/filtering. `@spatialdata/core` loads render-oriented shape data, `@spatialdata/layers` turns that into deck layers, and `@spatialdata/vis` consumes the shared behavior for viewer use.

Points should follow the same package split before we make a GeoArrow migration
load-bearing:

- `@spatialdata/core` discovers SpatialData points stores and exposes coordinate
columns, stable point ids, row-index alignment, and Arrow batches/vectors when
available
- `@spatialdata/layers` owns the deck-facing points renderer, including
filtering/styling feature state and backend choice
- the initial backend can remain `ScatterplotLayer` over typed coordinate
arrays, but the public points config should be representation-agnostic enough
to add `@geoarrow/deck.gl-geoarrow` / `GeoArrowScatterplotLayer` when point
data is GeoArrow-encoded or cheaply adaptable

The migration test is the same as shapes: MDV or Vitessce should be able to
drive hide/fade/color/radius state without knowing whether the renderer used JS
arrays, deck binary attributes, or GeoArrow batches underneath.
98 changes: 67 additions & 31 deletions docs/docs/vis/layer-prop-flow.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ right answer once.
a prop's change should invalidate downstream async work (tile fetching,
re-loading data), it goes into `updateTriggers` on the layer that owns the
side effect. There is no parallel registry.
4. **Viv image layers are an adapter boundary, not an exception.** Images still
follow the same deck rule. Viv constructs the actual image layers, and Viv
0.21's multiscale image tile layer declares `[loader, selections]` as the
`getTileData` trigger set.

If you follow those three rules, deck.gl's existing layer matching + prop
If you follow those rules, deck.gl's existing layer matching + prop
diffing handles the rest. Cosmetic prop tweaks repaint without touching the
tileset cache; structural changes invalidate via `updateTriggers` and refetch.
Nothing more is needed.
Expand Down Expand Up @@ -56,6 +60,40 @@ refetches. The fix is upstream stability, not downstream caching.
- Channel control values that are derived from `LayerConfig.channels` and a
loaded fallback should still be memoized to keep render output cheap, but
identity stability is not required for correctness.
- World bounds are structural too. Computing polygon bounds can be O(n-vertices),
so bounds must be cached by loaded data reference plus transform reference.
Opacity, color, visibility toggles inside the properties pane, and other
cosmetic layer edits must not re-run `boundsFromPolygons` /
`accumulatePolygonBounds`.
- Keep expensive fitting work behind command/effect boundaries. Render should
ask cheap questions such as "is this layer visible and renderable?" rather
than computing bounds just to decide whether a button looks enabled. The
actual bounds lookup belongs in the button handler or the guarded auto-fit
effect.

### For image layers through Viv

SpatialCanvas image rendering currently routes through Viv
`DetailView.getLayers()`, which creates Viv `ImageLayer` /
`MultiscaleImageLayer` instances. That means `@spatialdata/vis` does not
directly own the image tile layer class, but it still owns the props it passes
to Viv.

- Treat Viv's image layer as the tile-loading owner. In Viv 0.21,
`MultiscaleImageLayer` sets `updateTriggers.getTileData` to
`[loader, selections]`.
- Keep `loader` and `selections` identity-stable in `useLayerData`. Cosmetic
image props (`colors`, `contrastLimits`, `channelsVisible`, `opacity`,
`modelMatrix`) can flow through as normal props.
- `VivSpatialViewer` may call `detailView.getLayers()` on each render. The
important requirements are stable layer ids, passing the complete prop bag
into Viv/deck, and avoiding any viewer-local classification of prop names.
- Do not patch Viv-created layers by spreading `layer.props` after creation.
Some Viv/deck props, including extension defaults, are not safe to preserve
with object spread. Pass props into Viv up front, then use `layer.clone()` only
for identity-neutral deck props such as the final layer id.
- If a future Viv version changes the image tile trigger set, update this note
and add or adjust an image behavioral test in the same PR.

### For layer authors (`LabelsLayer`, future custom layers)

Expand All @@ -79,11 +117,11 @@ refetches. The fix is upstream stability, not downstream caching.
That is the only structural change it should make to incoming layers.
- Must not extract, classify, or re-route props by name. The viewer is
transparent to whatever props the producer or extensions chose to pass.
- The only legitimate viewer-local cache is for the layer instances that Viv's
`detailView.getLayers()` itself constructs on each call (because Viv's API
does not return stable references). Cache key is the producer-side
`(loader, selectionsRef)` tuple; visual updates go through the deck-native
`layer.clone(props)` of *all* incoming props, not a hand-picked subset.
- It should not maintain viewer-local layer caches unless there is runtime
evidence that deck's layer matching cannot preserve the relevant Viv layer
state. If such a cache becomes necessary, the cache key must be structural
only (`loader`, `selectionsRef`, and any future Viv-declared tile trigger),
and cosmetic updates must still flow through deck-native props.

## Anti-patterns (do not reintroduce)

Expand Down Expand Up @@ -122,31 +160,29 @@ expect(fetchCount.value).toBe(before);
One such test per layer type (`image`, `labels`, future custom layers) is
enough to keep the contract honest.

## Migration plan

> This section is tactical and should be removed once the follow-up PR
> implementing the redesign has merged.

The cosmetic-prop performance bug exists on `main` at the time of writing.
The redesign that fixes it lives in a follow-up branch. Sequence:

1. **Diagnostic round.** Instrument `useLayerData.getVivLayerProps()` and the
labels branch with one-shot identity-stability logging — for each render,
record which fields' identity changed. Confirm whether the culprit is
`loader`, `selections`, `getTileData`, or all three.
2. **Restore identity stability in `useLayerData`.** Memoize the offending
fields. Do *not* add any new caches outside `useLayerData`.
3. **Simplify `VivSpatialViewer`.** Remove all bespoke per-extra-layer caches.
Keep the minimal image-layer cache only if `detailView.getLayers()` still
requires it; key it on `(loader, selectionsRef)` only.
4. **Simplify `LabelsLayer`.** Plain CompositeLayer that passes props through
via `getSubLayerProps` and declares `updateTriggers.getTileData` on the
inner tile layer. No module caches, no lifecycle overrides for state.
5. **Add the behavioral test** described above for both image and labels.

Acceptance: opacity slider drag and channel-color edit on a labels layer
produce zero new `getTile` calls in the test harness; manual DevTools Network
panel during cosmetic drags stays empty.
## Current audit checklist

Use this checklist when changing images, labels, shapes, or future layer types.

1. **Identify the tile-loading owner.** For labels, that is
`LabelsLayer` / its inner `TileLayer`. For images, that is Viv's
`MultiscaleImageLayer`.
2. **Read the owner's `updateTriggers.getTileData`.** The trigger list is the
structural contract. Mirror it in the producer's identity-stability work; do
not invent a second visual-vs-structural table.
3. **Keep adapter components transparent.** Viewers may normalize ids and
compose layers, but should not sort props into structural and cosmetic
buckets.
4. **Test behavior, not cache mechanics.** A cosmetic opacity/color/channel
change should not produce new tile reads. A real selection/loader change
should.
5. **Profile non-fetch structural work too.** A cosmetic prop change should not
rebuild precomputed shape arrays, re-decode geometry, or re-scan polygon
vertices for world bounds. Tile fetches are only one symptom of a structural
leak.
6. **Check render-time UI state for hidden geometry work.** Buttons and panels
should not call `getWorldBoundsForLayer()` unless they are executing a user
command.

## See also

Expand Down
58 changes: 56 additions & 2 deletions docs/docs/vis/mdv-integration.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,56 @@ For now, the Vitessce note should be treated as an API pressure test and a succe

We should track upstream deck.gl / loaders.gl / deck.gl-community work around Arrow, GeoArrow, and GeoParquet carefully. This affects the boundary between `@spatialdata/core`, `@spatialdata/layers`, `@spatialdata/vis`, deck loaders, and app-specific adapters.

Current upstream read:

- `geoarrow/deck.gl-geoarrow` is the renamed home for the former
`geoarrow/deck.gl-layers` project. The published package to evaluate is
`@geoarrow/deck.gl-geoarrow`; it targets deck.gl 9 and Apache Arrow JS.
- The useful layer for SpatialData points is likely `GeoArrowScatterplotLayer`,
but it expects GeoArrow point/multipoint data, not arbitrary `x` / `y`
columns. SpatialData points currently store coordinate columns in Parquet, so
an adapter still has to build or expose a GeoArrow point column/batch.
- The library is most useful when we can keep Arrow chunks columnar all the way
to deck.gl's binary attribute interface. It is less compelling if we first
materialise every point as JS objects or as the current ndarray-ish wrapper.
- deck.gl-community's Arrow layers are a second signal in the same direction,
but the community docs explicitly warn about maintenance bandwidth. Treat
that as an API pressure test rather than a dependency to bet the public
contract on.

Current local state:

- `@spatialdata/core` currently loads Parquet bytes/tables through `parquet-wasm` in `VTableSource`, inherited by points and shapes sources.
- points currently return an ndarray-ish `{ shape, data }` object with axis columns loaded from Parquet.
- shapes currently expose a render-oriented core payload with stable feature ids, shared row-index alignment, and a mixed backend path in `@spatialdata/layers`.
- labels are still their own image/tile rendering path.
- Vitessce-derived code already has more advanced point handling in places, including tiled point loading, viewport filtering, feature-index filtering, and `DataFilterExtension` use.
- `VTableSource` recognises `points/<key>/points.parquet` and
`points/<key>/points.parquet/part.0.parquet`, but it does not yet model a
multi-file Parquet dataset as multiple chunks/batches. That is the wrong
shape for large point stores.

Points-specific target shape:

- `core` should expose a `PointsRenderData`-style payload, parallel to
`ShapesRenderData`, with stable point ids, row-index alignment, coordinate
axis names, optional `feature_key` / `instance_key` columns, and the original
Arrow table or record batches when available.
- `layers` should own a shared points renderer. The renderer should choose
between:
- a current fallback `ScatterplotLayer` over typed coordinate arrays
- a binary deck.gl attribute path for `x` / `y` / optional `z`
- a `GeoArrowScatterplotLayer` path when data is already GeoArrow point
encoded, or when the adapter can build that point column without copying too
much
- Points need the same feature-state language as shapes: hide, fade, color,
radius, and filtered opacity by stable point id or row index. MDV/Vitessce
filters should update feature-state or filter columns; they should not force a
full Parquet reload.
- Viewport/row-group filtering belongs behind the points data adapter, not in
`SpatialCanvas` UI code. A multi-file Parquet directory can naturally map to
progressive chunks/layers first, then later to row-group or bounding-box
pruning when metadata is available.

Upstream signals to monitor:

Expand Down Expand Up @@ -248,14 +291,22 @@ Recommended direction for now:

- [ ] Keep `@spatialdata/core` free of deck.gl dependencies.
- [ ] Align `@spatialdata/core`'s points and shapes support with Vitessce's SpatialData-derived loaders so we do not fall behind format coverage while the rendering backend evolves.
- [ ] Move toward `core` exposing Arrow-ish columnar primitives for points/shapes/tables, while preserving convenience methods for simple JS arrays.
- [ ] Move toward `core` exposing Arrow-ish columnar primitives for
points/shapes/tables, while preserving convenience methods for simple JS
arrays. For points, that means preserving Arrow batches/vectors alongside the
current coordinate-array convenience path.
- [ ] Make `@spatialdata/layers` responsible for choosing the rendering backend:
- current polygon fallback for compatibility
- current `geoarrow-table` runtime branch for shared columnar payloads
- `deck.gl-geoarrow` as the intended stronger near-term fast path when the external dependency is adopted cleanly
- `@geoarrow/deck.gl-geoarrow` as the intended stronger near-term fast path
when the external dependency is adopted cleanly
- future Arrow/community-layer backends without changing the public shapes config
- [x] Keep feature identity, table association, coordinate transforms, and metadata interpretation in `core`; keep GPU filtering, tiling, layer construction, and picking/render props in `layers`.
- [ ] Avoid baking `parquet-wasm` as the only long-term path. Treat it as the current implementation behind a replaceable interface.
- [ ] Upgrade points before adopting GeoArrow broadly: first add stable point
identity, row-index alignment, feature-state filtering/styling, and multi-part
Parquet discovery; then add the GeoArrow renderer as an adapter behind the
same public points config.

One more API-design note to preserve: the current table-association helpers are still `obs`-oriented and string-column-oriented because that is enough for the first feature-id join path. A future revision should widen the shared contract so style/filter inputs can come coherently from all of the AnnData surfaces we care about: `obs`, `var`, selected `X` columns for chosen `var` rows, `obsm`, potential future `obsp` graph/network data, and `uns`, without forcing integrators to encode everything as ad hoc string column lookups. In principle, many of those richer access patterns should likely be pushed upstream into `anndata.js` rather than duplicated forever in SpatialData.js.
- [ ] Treat Vitessce parity tests as compatibility fixtures: when Vitessce supports a points/shapes SpatialData layout, `core` should either support it too or document why not.
Expand All @@ -268,6 +319,9 @@ One more API-design note to preserve: the current table-association helpers are
Open questions:

- Should `PointsElement.loadPoints()` return Arrow vectors/tables in addition to typed arrays?
- Should `PointsElement` expose a chunked/multipart API (`loadPointBatches`) so
large `points.parquet/part-*.parquet` directories can progressively render
without pretending they are one file?
- Should `ShapesElement.loadPolygonShapes()` preserve feature ids alongside geometry in a first-class row object or columnar structure?
- Where should row-group / viewport filtering live: `core`, `vis`, or app adapter?
- Can deck.gl-community Arrow layers become a dependency of `@spatialdata/vis`, or should they be optional peer/adapter code?
Expand Down
10 changes: 10 additions & 0 deletions docs/docs/vis/spatial-canvas-status.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ sidebar_position: 1
- **Single primary image** path in the Viv viewer composition (first enabled image layer drives `DetailView.getLayers`).
- **View state** conversion between SpatialCanvas and Viv is still **2D-oriented**; full 3D orbit state is not fully round-tripped.
- **`useLayerData`** prefers explicit `LayerConfig.channels` when arrays are non-empty; further **override flags** may still be useful for edge cases.
- **Points are still a minimal scatter path:** `PointsElement.loadPoints()`
loads coordinate columns into an ndarray-ish object and
`@spatialdata/vis` renders them directly with `ScatterplotLayer`. There is
not yet stable point identity, table-backed feature state, viewport/row-group
filtering, progressive multi-file Parquet loading, or a GeoArrow-backed
renderer.

## Upstream Viv Follow-ups

Expand Down Expand Up @@ -49,6 +55,10 @@ sidebar_position: 1
- Flesh out **`SpatialLayer`** sublayers (image, scatter, shapes, …) and keep **`SpatialLayerProps`** migrations honest as kinds grow.
- Harden **`@spatialdata/avivatorish`** for MDV adoption (telemetry hooks, docs).
- **MDV integration** checklist: replace vendored avivatorish, adopt shared layers, scatter/table-backed props, phased contour extraction.
- **Points parity with shapes:** move point rendering into
`@spatialdata/layers`, add stable point ids plus hide/fade/color/radius
feature state, and make multi-file Parquet stores render progressively before
adopting GeoArrow as a fast-path adapter.
- **GeoArrow / Parquet** paths for shapes and points; clarify **`@spatialdata/core`** vs deck-facing buffers.
- **3D view mode** and **time (`t`)** in the public scene contract.

Expand Down
1 change: 1 addition & 0 deletions packages/vis/src/SpatialCanvas/SpatialCanvasViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ function SpatialCanvasViewerInner({
viewState={viewState}
onViewStateChange={onViewStateChange}
layers={renderer.deckLayers}
layerOrder={layerOrder}
vivLayerProps={renderer.vivLayerProps.length > 0 ? renderer.vivLayerProps : undefined}
onHover={handleHover}
onClick={handleClick}
Expand Down
4 changes: 4 additions & 0 deletions packages/vis/src/SpatialCanvas/SpatialViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export interface SpatialViewerProps {
onViewStateChange: (vs: ViewState) => void;
/** deck.gl layers to render (shapes, points, etc.) */
layers: Layer[];
/** Global SpatialCanvas layer order, bottom to top. */
layerOrder?: string[];
/** Optional: Viv layer props for image layers */
vivLayerProps?: ImageLayerConfig[];
/** Optional: Callback on hover */
Expand All @@ -52,6 +54,7 @@ export function SpatialViewer({
viewState,
onViewStateChange,
layers,
layerOrder,
vivLayerProps,
onHover,
onClick,
Expand All @@ -69,6 +72,7 @@ export function SpatialViewer({
onViewStateChange={onViewStateChange}
vivLayerProps={vivLayerProps}
extraLayers={layers}
layerOrder={layerOrder}
onHover={onHover}
onClick={onClick}
deckProps={deckProps}
Expand Down
Loading
Loading