diff --git a/.changeset/apply-path-for-in-place-config-edits.md b/.changeset/apply-path-for-in-place-config-edits.md new file mode 100644 index 00000000..609d572f --- /dev/null +++ b/.changeset/apply-path-for-in-place-config-edits.md @@ -0,0 +1,40 @@ +--- +'@spatialdata/core': patch +'@spatialdata/vis': patch +--- + +Make a fill-colour column (or tooltip field) switch actually apply for a host that +edits its layer configs in place. + +Two independent breaks sat between "the user picked a different column" and "the +canvas shows it", and a host only hit them together. #119 fixed the third thing in +that chain — the load-window blank — which is why the remaining two read as "the +colours just never change". + +**The change never reached the resolver.** `useLayerData`'s reconcile effect is the +one place a config change turns into a request, and it was keyed on the identity of +`layers` and the configs inside it. That assumes the caller allocates a fresh config +per edit; MDV's render-stack adapter deliberately does the opposite, keeping one +`LayerConfig` per Stack Entry so a cosmetic edit does not look structural and +re-enter geometry loads. Under that caller the effect never re-ran: the new column +was never requested, `getShapeFillColorEntry` / `getLabelFillColorEntry` went on +correctly serving last-good rows, and last-good was all there would ever be. The +effect now also depends on `describeResolveInputs` — a value key over exactly the +config fields each resolver's `plan()` reads, recomputed per render because a +mutation is invisible to any memo. It holds scalars and short id lists only; a +palette swap or an opacity drag does not move it, so nothing replans on a slider. + +**The settle never reached React.** `SpatialEntryStore` subscribed to its resolvers +in its constructor and tore that bridge down in `dispose()` — which `useLayerData` +calls from an effect cleanup. An effect cleanup is not "the end": StrictMode's dev +double-mount runs cleanup and then re-runs the effect against the same memoised +store, after which the store was permanently deaf to its own resolvers. Every async +settle from then on was dropped, so rows that landed after a switch did not repaint +until an unrelated re-render (a pan) came along. The bridge is now attached on the +first listener and detached on the last, so it is exactly as long-lived as someone +caring about it and survives any number of remounts. `getVersion()` became a derived +sum of the resolvers' versions rather than a counter the bridge maintained, so it +stays true whether or not anything is subscribed. + +No public API change. Verified against MDV driving only `fillColorByColumn` on a +labels layer: switching to a column that has to be fetched now repaints on its own. diff --git a/.changeset/column-colour-not-view-colour.md b/.changeset/column-colour-not-view-colour.md new file mode 100644 index 00000000..e3cc13b2 --- /dev/null +++ b/.changeset/column-colour-not-view-colour.md @@ -0,0 +1,48 @@ +--- +'@spatialdata/layers': minor +'@spatialdata/vis': minor +--- + +Make a column's colours a property of the column, not of the features that loaded. + +Three things decided the encoding from whatever happened to be in view, so two +layers over one annotation could disagree about what a colour means — which reads +as a data difference rather than as a bug: + +- Category indices were assigned in **first-seen feature order**. A shapes layer + walks the loader's geometry order and a labels layer walks the raster's ids, so + the same `cell_type` column rendered in two different schemes on the two kinds. + (`labelColorEncoding.spec.ts` claimed to cover this, but only pinned the indices + on one kind; it now actually builds the column through both.) Categories are now + ordered by value, with numeric-looking values ordered numerically so cluster 10 + follows cluster 9 rather than cluster 1. +- Positional palettes cannot survive a category being **absent from a view** at + all: `tumour` genuinely is the second category present when `stroma` is not. + `categoricalPalette` therefore also accepts `{ byValue: { Tumour: [200, 30, 30] } }`, + with an optional `fallback` for values it does not name (`'oklab'` by default, so + an unnamed category keeps its own hue instead of merging into one bucket). This + is the form to prefer in a saved stack, and the only form an embedding + application can use to make a layer agree with its own charts. +- The continuous ramp measured its extent from the loaded features. `numericDomain` + pins it to the column's own range; values outside clamp rather than extrapolate. + +`numericRamp` also takes more than two stops now, spaced evenly across the domain, +because the ramps people actually use are not two-stop — viridis, a diverging +red/white/blue, or whatever a host has already chosen for the same column in its +own UI. Approximating one by its endpoints loses the midpoint that made it +meaningful. `numericScale: 'symlog'` goes with it: a counts or expression column +whose mass sits near zero with a long tail collapses into the first stop under a +linear position. Symmetric log rather than plain log, because these columns reach +zero and below. + +`featureColorSchemeSignature` now takes the scheme as one object +(`featureColorSchemeSignature(config.fillColorByColumn)`) rather than three +positional arguments, so adding a term to the encoding cannot leave a call site +silently keying on the old set — the failure mode there being a layer that keeps +serving the previous colours after the scheme changed. Named palettes are +serialised in sorted key order, since object key order is insertion order and a +host rebuilding its palette each render need not insert in a stable one. + +**Colours will change** for existing categorical configs that relied on the +implicit first-seen order. Pass `categoricalPalette: { byValue }` to fix a scheme +in place. diff --git a/.changeset/publish-sourcemaps-and-survive-bad-schemes.md b/.changeset/publish-sourcemaps-and-survive-bad-schemes.md new file mode 100644 index 00000000..f498207f --- /dev/null +++ b/.changeset/publish-sourcemaps-and-survive-bad-schemes.md @@ -0,0 +1,21 @@ +--- +'@spatialdata/layers': patch +'@spatialdata/vis': patch +'@spatialdata/avivatorish': patch +'@spatialdata/react': patch +--- + +Publish sourcemaps, and survive a colour scheme that does not match its own type. + +`core` shipped `index.js.map`; `layers`, `vis`, `avivatorish` and `react` did not. +A crash inside one of them reached a consumer as +`Le (…/.vite/deps/@spatialdata_layers.js:396)` — an esbuild-minified name with +nothing to map it back to. An embedding application has only the built artifact to +debug against, so it has to carry a map. + +`resolveCategoricalPalette` and the ramp sampler now always return a colour. A +scheme arrives from a saved Render Stack, so its type is a claim about JSON rather +than a guarantee: a palette object with no `byValue`, a list with a hole in it, or +a ramp with fewer than two stops all used to return `undefined` and fail several +frames later in the arithmetic that reads `rgb[0]`. Wrong colours can be seen and +reported; that `TypeError` cannot. diff --git a/packages/core/src/engine/SpatialEntryStore.ts b/packages/core/src/engine/SpatialEntryStore.ts index c2a183b0..30ffd5a8 100644 --- a/packages/core/src/engine/SpatialEntryStore.ts +++ b/packages/core/src/engine/SpatialEntryStore.ts @@ -29,28 +29,63 @@ export class SpatialEntryStore { private readonly unsubscribes: Array<() => void> = []; /** One AbortController per in-flight task id. Superseding cancels the old one. */ private readonly inFlight = new Map(); - private version = 0; constructor(resolvers: ResolverRegistry) { this.resolvers = resolvers; - // The store's version is the sum of its parts: any resolver mutating is a - // reason for React to re-read. - for (const resolver of Object.values(resolvers)) { + } + + /** + * The store's version is the sum of its parts: any resolver mutating is a reason + * for React to re-read. The bridge that carries that is tied to HAVING LISTENERS, + * not to construction — because the store outlives the effect that consumes it. + * + * Subscribing in the constructor and tearing down in `dispose` looks equivalent + * and is not. `dispose()` runs from a React effect cleanup, and an effect cleanup + * is not "the end": StrictMode's dev double-mount runs cleanup and then re-runs + * the effect against the SAME memoised store. A constructor-time bridge cannot be + * rebuilt, so from that moment the store was permanently deaf to its own resolvers + * — every async settle after mount was dropped, and a fill-colour column whose + * rows landed after the switch never repainted until an unrelated re-render (a + * pan) happened to come along. Attaching on the first listener and detaching on + * the last makes the bridge exactly as long-lived as someone caring about it, and + * survives any number of remounts. + */ + private attachResolvers(): void { + if (this.unsubscribes.length > 0) return; + for (const resolver of Object.values(this.resolvers)) { this.unsubscribes.push(resolver.subscribe(() => this.notify())); } } + private detachResolvers(): void { + for (const unsubscribe of this.unsubscribes) unsubscribe(); + this.unsubscribes.length = 0; + } + subscribe = (listener: () => void): (() => void) => { this.listeners.add(listener); + if (this.listeners.size === 1) this.attachResolvers(); return () => { this.listeners.delete(listener); + if (this.listeners.size === 0) this.detachResolvers(); }; }; - getVersion = (): number => this.version; + /** + * The sum of its parts, DERIVED rather than counted. + * + * A counter incremented from the notification bridge would only be correct while + * something was subscribed — and the bridge is now listener-driven, so that is not + * always. Summing the resolvers' own versions makes this a pure read of the state + * it describes: true with a listener, without one, and across a dispose. + */ + getVersion = (): number => { + let version = 0; + for (const resolver of Object.values(this.resolvers)) version += resolver.getVersion(); + return version; + }; private notify(): void { - this.version += 1; for (const listener of this.listeners) { listener(); } @@ -137,9 +172,13 @@ export class SpatialEntryStore { this.resolvers[kind]?.evict(elementKey); } + /** + * Release everything this store owns. Not a one-way door: a later `subscribe` + * re-attaches the resolver bridge, which is what lets a StrictMode remount (or any + * effect that re-runs against the same store) recover instead of going silent. + */ dispose(): void { - for (const unsubscribe of this.unsubscribes) unsubscribe(); - this.unsubscribes.length = 0; + this.detachResolvers(); for (const controller of this.inFlight.values()) controller.abort(); this.inFlight.clear(); for (const resolver of Object.values(this.resolvers)) resolver.dispose(); diff --git a/packages/core/tests/spatialEntryStoreSubscription.spec.ts b/packages/core/tests/spatialEntryStoreSubscription.spec.ts new file mode 100644 index 00000000..fef43de2 --- /dev/null +++ b/packages/core/tests/spatialEntryStoreSubscription.spec.ts @@ -0,0 +1,134 @@ +import { describe, expect, it, vi } from 'vitest'; +import { + type EntryResources, + Resolution, + type ResolveTask, + SpatialEntryStore, +} from '../src/engine/index.js'; + +/** + * The store's bridge to its resolvers, under the lifecycle React actually gives it. + * + * `SpatialEntryStore` is memoised by `useLayerData` and disposed from an effect + * cleanup — and an effect cleanup is not "the end". StrictMode's dev double-mount + * runs cleanup and then re-runs the effect against the same store instance. When the + * bridge was built in the constructor and torn down in `dispose`, that sequence left + * the store permanently deaf: resolvers went on loading and settling, and nothing + * downstream ever heard about it. The symptom was a fill-colour column that loaded + * and then never painted until an unrelated re-render came along. + * + * These tests drive that sequence directly, with a resolver stub whose only job is to + * emit one settle. + */ + +/** A resolver that does nothing but let a test fire its settle notification. */ +function notifyingResolver() { + const listeners = new Set<() => void>(); + return { + resolver: { + kind: 'labels' as const, + blockingResources: [] as const, + plan: (): readonly ResolveTask[] => [], + load: async () => {}, + snapshot: (): EntryResources => ({ + entryId: 'e', + elementKey: 'k', + resources: {}, + notices: [], + bounds: null, + revision: 0, + }), + evict: () => {}, + dispose: () => { + listeners.clear(); + }, + subscribe: (listener: () => void) => { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; + }, + getVersion: () => 0, + }, + /** Stand in for a load settling — what `finally { this.notify() }` does. */ + settle: () => { + for (const listener of listeners) listener(); + }, + listenerCount: () => listeners.size, + }; +} + +function storeWith(labels: ReturnType['resolver']) { + const inert = { ...labels, subscribe: () => () => {} }; + return new SpatialEntryStore({ + points: inert, + shapes: inert, + images: inert, + labels, + }); +} + +describe('SpatialEntryStore — the resolver notification bridge', () => { + it('forwards a resolver settle to its listeners', () => { + const labels = notifyingResolver(); + const store = storeWith(labels.resolver); + const onChange = vi.fn(); + store.subscribe(onChange); + + labels.settle(); + + expect(onChange).toHaveBeenCalledTimes(1); + }); + + it('still forwards a settle after dispose + resubscribe (the StrictMode remount)', () => { + // THE regression. React runs cleanup then re-runs the effect against the same + // memoised store; nothing about that says the store is finished. + const labels = notifyingResolver(); + const store = storeWith(labels.resolver); + const onChange = vi.fn(); + + const unsubscribe = store.subscribe(onChange); + unsubscribe(); + store.dispose(); + store.subscribe(onChange); + + labels.settle(); + + expect(onChange).toHaveBeenCalledTimes(1); + }); + + it('holds no resolver subscription while nobody is listening', () => { + // The bridge exists for listeners. With none, it must not pin the resolver — + // otherwise a discarded store keeps a live edge into a resolver it no longer owns. + const labels = notifyingResolver(); + const store = storeWith(labels.resolver); + + expect(labels.listenerCount()).toBe(0); + + const unsubscribe = store.subscribe(vi.fn()); + expect(labels.listenerCount()).toBe(1); + + unsubscribe(); + expect(labels.listenerCount()).toBe(0); + }); + + it('attaches once for many listeners, and detaches only when the last one goes', () => { + const labels = notifyingResolver(); + const store = storeWith(labels.resolver); + const first = vi.fn(); + const second = vi.fn(); + + const unsubscribeFirst = store.subscribe(first); + const unsubscribeSecond = store.subscribe(second); + expect(labels.listenerCount()).toBe(1); + + unsubscribeFirst(); + expect(labels.listenerCount()).toBe(1); + + labels.settle(); + expect(second).toHaveBeenCalledTimes(1); + + unsubscribeSecond(); + expect(labels.listenerCount()).toBe(0); + }); +}); diff --git a/packages/layers/src/featureColorEncoding.ts b/packages/layers/src/featureColorEncoding.ts index 3baafb95..e2328040 100644 --- a/packages/layers/src/featureColorEncoding.ts +++ b/packages/layers/src/featureColorEncoding.ts @@ -76,9 +76,38 @@ export function featureColorAt( ]; } +/** + * A category's colour named by the category itself, rather than by its position. + * + * This is the only form that survives the data changing. The other two are + * positional — a colour is whatever the Nth category gets — and "the Nth category" + * is a property of the features that happened to load, not of the column. Two + * layers over the same annotation therefore disagree the moment their feature sets + * differ, which is the normal case: a shapes layer walks the loader's geometry + * order and a labels layer walks its raster ids. + * + * It is also the only form a host can use to say what it means. A viewer embedded + * in an application whose user has already chosen "Tumour is red" cannot express + * that as a list, because it does not know — and must not have to know — which + * index `Tumour` will land on. + * + * Values are the column's cells in canonical string form (see + * {@link normalizeFeatureCellValue}), so a numeric category is `'3'`, not `3`. + */ +export interface FeatureNamedCategoricalPalette { + byValue: Readonly>; + /** + * Colour for a value the map does not name. Defaults to `'oklab'`, which gives + * each unnamed category its own hue — an unnamed category stays visible and + * distinguishable rather than silently merging into a single "other" bucket. + */ + fallback?: 'oklab' | FeatureRgbColor; +} + /** * How to colour categories. JSON-serializable on purpose — this travels in a saved - * layer config, so it is a name or a plain list of colours, never a function. + * layer config, so it is a name, a plain list of colours, or a plain object, never + * a function. * * - `'oklab'` — the points colour-by-feature scheme: OKLCh at fixed lightness and * chroma, hue stepped by the golden angle. **Unbounded** — every @@ -86,10 +115,53 @@ export function featureColorAt( * annotation does not repeat colours. The default. * - a list — your own colours, cycled. An empty list falls back to `'oklab'` * rather than colouring nothing. + * - a map — {@link FeatureNamedCategoricalPalette}. Prefer this whenever you + * know what the categories are; the two positional forms depend on + * which features loaded. */ -export type FeatureCategoricalPaletteSpec = 'oklab' | readonly FeatureRgbColor[]; +export type FeatureCategoricalPaletteSpec = + | 'oklab' + | readonly FeatureRgbColor[] + | FeatureNamedCategoricalPalette; -export type FeatureNumericRampSpec = readonly [FeatureRgbColor, FeatureRgbColor]; +/** + * The colours a continuous column ramps through, low to high. + * + * Two or more stops, spaced evenly across the domain and interpolated in RGB. Two + * is the common case and the default; more exists because the ramps people + * actually want are not two-stop — viridis, a diverging red/white/blue, and any + * palette a host has already chosen for the same column elsewhere in its own UI + * all need more, and approximating them with their endpoints does not just look + * different, it loses the midpoint that made them meaningful. + */ +export type FeatureNumericRampSpec = readonly [ + FeatureRgbColor, + FeatureRgbColor, + ...FeatureRgbColor[], +]; + +/** + * How a value's position along the ramp is measured. + * + * - `'linear'` (default) — position is proportional to the value. + * - `'symlog'` — proportional to `sign(v)·log(1+|v|)`, so a column whose mass sits + * near zero with a long tail (counts, expression) spreads out instead of + * collapsing into the ramp's first stop. Symmetric log rather than plain log + * because it is defined at and below zero, which real columns reach. + */ +export type FeatureNumericScale = 'linear' | 'symlog'; + +/** + * The values the ramp's endpoints stand for, `[low, high]`. + * + * Without one, the extent is measured from the features that loaded — so the same + * column reads as a different scale on a layer covering a subset, and the colours + * of two layers over one annotation are not comparable. Pin it to the column's own + * range (which the store knows and the render does not) whenever you have it. + * + * Values outside the domain clamp to its endpoints rather than extrapolating. + */ +export type FeatureNumericDomain = readonly [number, number]; export const DEFAULT_FEATURE_CATEGORICAL_PALETTE: FeatureCategoricalPaletteSpec = 'oklab'; @@ -99,53 +171,120 @@ export const DEFAULT_FEATURE_NUMERIC_RAMP: FeatureNumericRampSpec = [ ]; /** - * Turn a palette spec into `categoryIndex → colour`. + * `null` is rejected explicitly, not incidentally: `typeof null === 'object'` and + * `Array.isArray(null)` is false, so without this it passes as a named palette and + * the destructure in {@link resolveCategoricalPalette} throws on the spot — + * defeating that function's whole always-returns-a-colour guarantee, and taking + * {@link featureColorSchemeSignature} with it. `{"categoricalPalette": null}` is a + * thing JSON says, so a saved Render Stack can say it. + */ +function isNamedCategoricalPalette( + spec: FeatureCategoricalPaletteSpec +): spec is FeatureNamedCategoricalPalette { + return spec !== null && typeof spec === 'object' && !Array.isArray(spec); +} + +/** + * Turn a palette spec into `(value, categoryIndex) → colour`. * * A function rather than an array because `'oklab'` has no length: its colour is a * pure function of the index, so there is no table to run out of. That is the whole * point of making it the default — the cycling of a fixed list is invisible in the * render (two cell types simply share a colour) and so is exactly the kind of bug * that survives review. + * + * Both arguments are passed because the spec decides which one is authoritative: a + * named palette answers from the value, the positional forms from the index. + * + * **Always returns a colour.** A spec arrives from a saved Render Stack — JSON, so + * the type is a claim, not a guarantee — and returning `undefined` for one it does + * not recognise puts the failure several frames away, in the arithmetic that reads + * `rgb[0]`. That is the shape of crash you cannot diagnose from a stack trace. An + * unrecognised spec falls back to the default scheme instead: wrong colours are + * visible and reportable, a `TypeError` deep in a bundled dependency is not. */ export function resolveCategoricalPalette( spec: FeatureCategoricalPaletteSpec = DEFAULT_FEATURE_CATEGORICAL_PALETTE -): (categoryIndex: number) => FeatureRgbColor { +): (value: string, categoryIndex: number) => FeatureRgbColor { + const byIndex = (_value: string, categoryIndex: number) => featureCodeToRgb(categoryIndex); if (spec === 'oklab') { - return featureCodeToRgb; + return byIndex; + } + if (isNamedCategoricalPalette(spec)) { + const { byValue, fallback = 'oklab' } = spec; + if (!byValue || typeof byValue !== 'object') { + return byIndex; + } + const colorForUnnamed = fallback === 'oklab' ? featureCodeToRgb : (_index: number) => fallback; + return (value, categoryIndex) => byValue[value] ?? colorForUnnamed(categoryIndex); } const colors = spec; - if (colors.length === 0) { - return featureCodeToRgb; + if (!Array.isArray(colors) || colors.length === 0) { + return byIndex; } - return (categoryIndex) => colors[categoryIndex % colors.length]; + return (_value, categoryIndex) => + colors[categoryIndex % colors.length] ?? byIndex('', categoryIndex); +} + +/** Everything about a column's encoding that is not the column itself. */ +export interface FeatureColorScheme { + categoricalPalette?: FeatureCategoricalPaletteSpec; + numericRamp?: FeatureNumericRampSpec; + numericDomain?: FeatureNumericDomain; + numericScale?: FeatureNumericScale; + missingValues?: FeatureMissingValueOptions; } /** * Stable serialisation of a scheme, for projection cache keys. * + * Takes the whole scheme as one object so that adding a term to the encoding + * cannot leave a call site silently keying on the old set — the failure mode being + * a layer that keeps serving the previous colours after the scheme changed. + * * The missing-value policy belongs here too: changing a sentinel or how a missing - * feature renders changes colours without touching the column, so a key that - * omitted it would keep serving the previous table. + * feature renders changes colours without touching the column. */ -export function featureColorSchemeSignature( - categoricalPalette?: FeatureCategoricalPaletteSpec, - numericRamp?: FeatureNumericRampSpec, - missingValues?: FeatureMissingValueOptions -): string { +export function featureColorSchemeSignature({ + categoricalPalette, + numericRamp, + numericDomain, + numericScale, + missingValues, +}: FeatureColorScheme = {}): string { if ( categoricalPalette === undefined && numericRamp === undefined && + numericDomain === undefined && + numericScale === undefined && missingValues === undefined ) { return ''; } return JSON.stringify([ - categoricalPalette ?? null, + serializeCategoricalPalette(categoricalPalette), numericRamp ?? null, + numericDomain ?? null, + numericScale ?? null, missingValues ? [missingValues.treatAsMissing ?? null, missingValues.render ?? null] : null, ]); } +/** + * A named palette is serialised in sorted key order, because object key order is + * insertion order and a caller rebuilding the same map per render need not insert + * in a stable one. Two equal maps have to produce one string, or the layer rebuilds + * its whole colour buffer on renders where nothing changed. + */ +function serializeCategoricalPalette(spec: FeatureCategoricalPaletteSpec | undefined): unknown { + if (spec === undefined) return null; + if (!isNamedCategoricalPalette(spec)) return spec; + return [ + Object.entries(spec.byValue).sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)), + spec.fallback ?? null, + ]; +} + /** * A cell rendered as the canonical string form; `''` means "no usable value". * @@ -193,6 +332,45 @@ function interpolateRgb( ]; } +/** + * Sample a multi-stop ramp at `t ∈ [0, 1]`, stops spaced evenly. + * + * `t` at exactly 1 has to land on the last stop rather than reading past it, which + * is what the `length - 2` clamp is for — the top of the domain is the value most + * likely to be looked at, and reading past the end would silently return the last + * segment interpolated at 1 anyway on some inputs and `undefined` on others. + */ +function sampleRamp(stops: FeatureNumericRampSpec, t: number): FeatureRgbColor { + // The two-stop minimum is a type-level claim about JSON that came out of a saved + // Render Stack, so it is checked. With one stop the arithmetic below indexes -1 + // and the failure surfaces as `low[0]` of undefined, several frames from here. + if (!Array.isArray(stops) || stops.length < 2) { + return stops?.[0] ?? DEFAULT_FEATURE_NUMERIC_RAMP[0]; + } + const clamped = Math.max(0, Math.min(1, t)); + const scaled = clamped * (stops.length - 1); + const lowIndex = Math.min(Math.floor(scaled), stops.length - 2); + return interpolateRgb(stops[lowIndex], stops[lowIndex + 1], scaled - lowIndex); +} + +/** Symmetric log, defined at and below zero. Matches d3's `scaleSymlog` at C = 1. */ +function symlog(value: number): number { + return Math.sign(value) * Math.log1p(Math.abs(value)); +} + +/** Where a value sits in `[min, max]`, as `t ∈ [0, 1]` before clamping. */ +function rampPosition(value: number, min: number, max: number, scale: FeatureNumericScale): number { + if (scale === 'symlog') { + const low = symlog(min); + const high = symlog(max); + // A degenerate domain has no position to report; the midpoint is the one + // answer that does not imply the value is at an extreme of a range it is not + // actually spread over. + return high === low ? 0.5 : (symlog(value) - low) / (high - low); + } + return max === min ? 0.5 : (value - min) / (max - min); +} + function getFiniteExtent(values: Array): [number, number] | undefined { let min = Number.POSITIVE_INFINITY; let max = Number.NEGATIVE_INFINITY; @@ -301,6 +479,10 @@ export interface AssignFeatureColorsOptions { alpha: number; categoricalPalette?: FeatureCategoricalPaletteSpec; numericRamp?: FeatureNumericRampSpec; + /** Pin the ramp's endpoints instead of measuring them. See {@link FeatureNumericDomain}. */ + numericDomain?: FeatureNumericDomain; + /** How position along the ramp is measured. See {@link FeatureNumericScale}. */ + numericScale?: FeatureNumericScale; /** * What the store declares this column to be. Supply it whenever you have it — * `'auto'` trusts it in preference to sniffing the values. See @@ -310,6 +492,31 @@ export interface AssignFeatureColorsOptions { missingValues?: FeatureMissingValueOptions; } +/** + * Category ordering, and so — for the positional palettes — category colour. + * + * Sorted rather than first-seen. First-seen order is a property of the features + * that loaded, not of the column, so it made the same annotation render in + * different colours on a shapes layer and a labels layer over one table, and made a + * saved config's colours drift whenever the data behind it changed. + * + * Numeric-looking values sort numerically, so cluster `10` follows cluster `9` + * rather than cluster `1`; those are the commonest positional categories and + * lexicographic order makes their palette look shuffled. Anything else sorts by + * code unit — deliberately not `localeCompare`, whose answer depends on the + * environment's locale data and would make the colours machine-dependent. + */ +function compareCategoryValues(a: string, b: string): number { + const numericA = featureNumericValue(a); + const numericB = featureNumericValue(b); + if (numericA !== undefined && numericB !== undefined && numericA !== numericB) { + return numericA - numericB; + } + if (numericA !== undefined && numericB === undefined) return -1; + if (numericA === undefined && numericB !== undefined) return 1; + return a < b ? -1 : a > b ? 1 : 0; +} + /** * Colour every feature from its column value. * @@ -319,8 +526,11 @@ export interface AssignFeatureColorsOptions { * of a dense colour buffer — "unannotated" and "annotated with the first palette * entry" have to stay distinguishable. * - * Category indices are assigned in first-seen feature order, so the same column - * yields the same colours for a given element regardless of which kind draws it. + * The encoding is a function of the COLUMN, not of the features that loaded: + * categories are ordered by {@link compareCategoryValues} and the ramp can be + * pinned with `numericDomain`. That is what lets the same annotation render the + * same way on a shapes layer and on a labels layer over the same table, and what + * makes a saved config's colours mean the same thing next time it is opened. */ export function assignFeatureColors({ values, @@ -328,6 +538,8 @@ export function assignFeatureColors({ alpha, categoricalPalette, numericRamp = DEFAULT_FEATURE_NUMERIC_RAMP, + numericDomain, + numericScale = 'linear', columnKind, missingValues, }: AssignFeatureColorsOptions): Array { @@ -361,13 +573,14 @@ export function assignFeatureColors({ const numericValues = values.map((value) => isMissing(value) ? undefined : featureNumericValue(value) ); - const extent = getFiniteExtent(numericValues); + // A pinned domain is used even when nothing in view falls inside it: the point + // of pinning is that the scale does not depend on what loaded. + const extent = numericDomain ?? getFiniteExtent(numericValues); if (!extent) { for (let index = 0; index < values.length; index += 1) applyMissing(index); return colors; } const [min, max] = extent; - const range = max - min; for (let index = 0; index < values.length; index += 1) { const value = numericValues[index]; if (value === undefined) { @@ -376,25 +589,44 @@ export function assignFeatureColors({ applyMissing(index); continue; } - const t = range === 0 ? 0.5 : (value - min) / range; - colors[index] = rgba(interpolateRgb(numericRamp[0], numericRamp[1], t), alpha); + colors[index] = rgba( + sampleRamp(numericRamp, rampPosition(value, min, max, numericScale)), + alpha + ); } return colors; } + // Two passes: the category set has to be complete and ordered before any colour + // is assigned, because a positional palette's answer for the first feature + // depends on categories that may only appear near the end of the column. const categoryIndexByValue = new Map(); + for (const value of new Set(nonEmptyValues)) { + categoryIndexByValue.set(value, 0); + } + const orderedValues = Array.from(categoryIndexByValue.keys()).sort(compareCategoryValues); + for (const [categoryIndex, value] of orderedValues.entries()) { + categoryIndexByValue.set(value, categoryIndex); + } + + // The palette is consulted once per CATEGORY rather than once per feature — a + // categorical column is a handful of distinct values over potentially millions of + // rows. Each feature still gets its own tuple: these are handed out to callers + // that store them per feature, and sharing one array between a category's + // features would make any in-place edit recolour all of them. + const rgbByValue = new Map(); + for (const [value, categoryIndex] of categoryIndexByValue) { + rgbByValue.set(value, colorForCategory(value, categoryIndex)); + } + for (let index = 0; index < values.length; index += 1) { const value = values[index]; if (isMissing(value)) { applyMissing(index); continue; } - let categoryIndex = categoryIndexByValue.get(value); - if (categoryIndex === undefined) { - categoryIndex = categoryIndexByValue.size; - categoryIndexByValue.set(value, categoryIndex); - } - colors[index] = rgba(colorForCategory(categoryIndex), alpha); + const rgb = rgbByValue.get(value); + if (rgb) colors[index] = rgba(rgb, alpha); } return colors; diff --git a/packages/layers/src/index.ts b/packages/layers/src/index.ts index efa0eb90..5ab97c67 100644 --- a/packages/layers/src/index.ts +++ b/packages/layers/src/index.ts @@ -42,10 +42,14 @@ export type { AssignFeatureColorsOptions, FeatureCategoricalPaletteSpec, FeatureColorBuffer, + FeatureColorScheme, FeatureColumnKind, FeatureFillColorMode, FeatureMissingValueOptions, + FeatureNamedCategoricalPalette, + FeatureNumericDomain, FeatureNumericRampSpec, + FeatureNumericScale, FeatureRgbaColor, FeatureRgbColor, } from './featureColorEncoding'; diff --git a/packages/layers/src/labelColorEncoding.ts b/packages/layers/src/labelColorEncoding.ts index 096decb3..dda6da84 100644 --- a/packages/layers/src/labelColorEncoding.ts +++ b/packages/layers/src/labelColorEncoding.ts @@ -39,7 +39,9 @@ import { type FeatureColumnKind, type FeatureFillColorMode, type FeatureMissingValueOptions, + type FeatureNumericDomain, type FeatureNumericRampSpec, + type FeatureNumericScale, type FeatureRgbaColor, type FeatureRgbColor, normalizeFeatureCellValue, @@ -213,6 +215,10 @@ export interface BuildLabelFillColorByFeatureIdOptions { alpha?: number; categoricalPalette?: FeatureCategoricalPaletteSpec; numericRamp?: FeatureNumericRampSpec; + /** Pin the ramp's endpoints instead of measuring them across the loaded features. */ + numericDomain?: FeatureNumericDomain; + /** How position along the ramp is measured; `'linear'` by default. */ + numericScale?: FeatureNumericScale; /** What the store declares the column to be; `'auto'` trusts it over the values. */ columnKind?: FeatureColumnKind; missingValues?: FeatureMissingValueOptions; @@ -232,6 +238,8 @@ export function buildLabelFillColorByFeatureId({ alpha = 255, categoricalPalette, numericRamp, + numericDomain, + numericScale, columnKind, missingValues, }: BuildLabelFillColorByFeatureIdOptions): Record { @@ -250,6 +258,8 @@ export function buildLabelFillColorByFeatureId({ alpha, ...(categoricalPalette ? { categoricalPalette } : {}), ...(numericRamp ? { numericRamp } : {}), + ...(numericDomain ? { numericDomain } : {}), + ...(numericScale ? { numericScale } : {}), ...(columnKind ? { columnKind } : {}), ...(missingValues ? { missingValues } : {}), }); diff --git a/packages/layers/src/shapeColorEncoding.ts b/packages/layers/src/shapeColorEncoding.ts index 7779c387..bb27482f 100644 --- a/packages/layers/src/shapeColorEncoding.ts +++ b/packages/layers/src/shapeColorEncoding.ts @@ -14,7 +14,9 @@ import { type FeatureColumnKind, type FeatureFillColorMode, type FeatureMissingValueOptions, + type FeatureNumericDomain, type FeatureNumericRampSpec, + type FeatureNumericScale, type FeatureRgbaColor, type FeatureRgbColor, normalizeFeatureCellValue, @@ -35,6 +37,10 @@ export interface BuildShapeFillColorByFeatureIdOptions { alpha: number; categoricalPalette?: FeatureCategoricalPaletteSpec; numericRamp?: FeatureNumericRampSpec; + /** Pin the ramp's endpoints instead of measuring them across the loaded features. */ + numericDomain?: FeatureNumericDomain; + /** How position along the ramp is measured; `'linear'` by default. */ + numericScale?: FeatureNumericScale; /** What the store declares the column to be; `'auto'` trusts it over the values. */ columnKind?: FeatureColumnKind; missingValues?: FeatureMissingValueOptions; @@ -54,6 +60,8 @@ export function buildShapeFillColorByFeatureId({ alpha, categoricalPalette, numericRamp, + numericDomain, + numericScale, columnKind, missingValues, }: BuildShapeFillColorByFeatureIdOptions): Record { @@ -70,6 +78,8 @@ export function buildShapeFillColorByFeatureId({ alpha, ...(categoricalPalette ? { categoricalPalette } : {}), ...(numericRamp ? { numericRamp } : {}), + ...(numericDomain ? { numericDomain } : {}), + ...(numericScale ? { numericScale } : {}), ...(columnKind ? { columnKind } : {}), ...(missingValues ? { missingValues } : {}), }); diff --git a/packages/layers/tests/featureColorEncoding.spec.ts b/packages/layers/tests/featureColorEncoding.spec.ts index 56abc4e7..9568aaf1 100644 --- a/packages/layers/tests/featureColorEncoding.spec.ts +++ b/packages/layers/tests/featureColorEncoding.spec.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { assignFeatureColors, featureColorAt, + featureColorSchemeSignature, resolveFeatureFillColorMode, } from '../src/featureColorEncoding'; @@ -85,11 +86,12 @@ describe('missing-value policy', () => { missingValues: { treatAsMissing: ['NA'] }, }); - expect(withSentinel[0]).toEqual(RED); + // Two real categories take the first two palette slots between them: the + // sentinel never entered the category set, so it did not consume one. (Which + // of the two is slot 0 is decided by value order — 'stroma' before 'tumour'.) + expect(withSentinel[0]).toEqual([0, 255, 0, 255]); expect(withSentinel[1]).toBeUndefined(); - // 'stroma' is the SECOND real category, not the third — the sentinel never - // entered the category set, so it did not consume a palette slot. - expect(withSentinel[2]).toEqual([0, 255, 0, 255]); + expect(withSentinel[2]).toEqual(RED); }); it('matches sentinels after trimming, case-insensitively', () => { @@ -162,3 +164,359 @@ describe('featureColorAt bounds', () => { expect(featureColorAt(padded, 1)).toBeUndefined(); }); }); + +/** + * The encoding must be a function of the COLUMN, not of the features that + * happened to load. Everything here is a case where it used not to be, and where + * the symptom was two views of one annotation disagreeing about what a colour + * means — which reads as a data difference, not as a bug. + */ +describe('an encoding that does not depend on which features loaded', () => { + const palette: [number, number, number][] = [ + [255, 0, 0], + [0, 255, 0], + [0, 0, 255], + ]; + + it('gives a category the same colour whatever order the features arrive in', () => { + const forward = assignFeatureColors({ + values: ['tumour', 'stroma'], + mode: 'categorical', + alpha: 255, + categoricalPalette: palette, + }); + const reversed = assignFeatureColors({ + values: ['stroma', 'tumour'], + mode: 'categorical', + alpha: 255, + categoricalPalette: palette, + }); + + expect(forward[0]).toEqual(reversed[1]); + expect(forward[1]).toEqual(reversed[0]); + }); + + it('does not shift a category when another one is absent from the view', () => { + // A layer over a subset that happens to contain no `stroma` must still draw + // `tumour` in the colour the full view draws it in. + const all = assignFeatureColors({ + values: ['alpha', 'stroma', 'tumour'], + mode: 'categorical', + alpha: 255, + categoricalPalette: palette, + }); + const subset = assignFeatureColors({ + values: ['alpha', 'tumour'], + mode: 'categorical', + alpha: 255, + categoricalPalette: palette, + }); + + expect(subset[0]).toEqual(all[0]); + // Positional palettes cannot survive this — `tumour` genuinely is the second + // category present. Naming the colours is the only fix, which is what the + // named-palette tests below cover; here we pin the shift so it stays visible. + expect(subset[1]).not.toEqual(all[2]); + }); + + it('orders numeric-looking categories numerically, not lexicographically', () => { + // Cluster 10 belongs after cluster 9. Under string order it lands between 1 + // and 2, and a 12-cluster annotation renders with a shuffled palette. + const colors = assignFeatureColors({ + values: ['1', '2', '10'], + mode: 'categorical', + alpha: 255, + categoricalPalette: palette, + }); + + expect(colors).toEqual([ + [255, 0, 0, 255], + [0, 255, 0, 255], + [0, 0, 255, 255], + ]); + }); + + it('holds the ramp to a pinned domain instead of the loaded extent', () => { + const pinned = { mode: 'continuous' as const, alpha: 255, numericDomain: [0, 10] as const }; + + // The same value, on two layers covering different parts of the column. + const full = assignFeatureColors({ values: ['0', '3', '10'], ...pinned }); + const subset = assignFeatureColors({ values: ['0', '3'], ...pinned }); + + expect(subset[1]).toEqual(full[1]); + // And without the domain it does not hold: in the subset, `3` is the top of + // the range rather than three tenths of the way up it. + const unpinned = assignFeatureColors({ + values: ['0', '3'], + mode: 'continuous', + alpha: 255, + }); + expect(unpinned[1]).not.toEqual(full[1]); + }); + + it('clamps values outside a pinned domain rather than extrapolating', () => { + const colors = assignFeatureColors({ + values: ['-100', '0', '10', '900'], + mode: 'continuous', + alpha: 255, + numericDomain: [0, 10], + }); + + expect(colors[0]).toEqual(colors[1]); + expect(colors[3]).toEqual(colors[2]); + }); +}); + +describe('a palette that names its categories', () => { + it('colours by value, so two views agree even on different category sets', () => { + const byValue = { tumour: [200, 30, 30] as [number, number, number] }; + const all = assignFeatureColors({ + values: ['alpha', 'stroma', 'tumour'], + mode: 'categorical', + alpha: 255, + categoricalPalette: { byValue }, + }); + const subset = assignFeatureColors({ + values: ['tumour'], + mode: 'categorical', + alpha: 255, + categoricalPalette: { byValue }, + }); + + expect(all[2]).toEqual([200, 30, 30, 255]); + expect(subset[0]).toEqual(all[2]); + }); + + it('gives an unnamed category its own hue rather than merging them', () => { + const colors = assignFeatureColors({ + values: ['tumour', 'stroma', 'other'], + mode: 'categorical', + alpha: 255, + categoricalPalette: { byValue: { tumour: [200, 30, 30] } }, + }); + + expect(colors[1]).not.toEqual(colors[2]); + }); + + it('honours an explicit fallback colour for everything unnamed', () => { + const colors = assignFeatureColors({ + values: ['tumour', 'stroma', 'other'], + mode: 'categorical', + alpha: 255, + categoricalPalette: { byValue: { tumour: [200, 30, 30] }, fallback: [90, 90, 90] }, + }); + + expect(colors[0]).toEqual([200, 30, 30, 255]); + expect(colors[1]).toEqual([90, 90, 90, 255]); + expect(colors[2]).toEqual([90, 90, 90, 255]); + }); + + it('does not name missing values into a category', () => { + // A named palette must not resurrect a sentinel: `isMissing` runs first, so an + // entry for the sentinel string is simply never consulted. + const colors = assignFeatureColors({ + values: ['tumour', 'NA'], + mode: 'categorical', + alpha: 255, + categoricalPalette: { byValue: { tumour: [200, 30, 30], NA: [1, 2, 3] } }, + missingValues: { treatAsMissing: ['NA'] }, + }); + + expect(colors[1]).toBeUndefined(); + }); +}); + +describe('featureColorSchemeSignature', () => { + it('separates schemes that differ only in a pinned domain', () => { + expect(featureColorSchemeSignature({ numericDomain: [0, 10] })).not.toBe( + featureColorSchemeSignature({ numericDomain: [0, 20] }) + ); + }); + + it('reads two equal named palettes as one scheme whatever order they were built in', () => { + // Object key order is insertion order, and a host rebuilding its palette per + // render need not insert in a stable one. A signature that moved would rebuild + // the whole colour buffer on renders where nothing changed. + const a = featureColorSchemeSignature({ + categoricalPalette: { byValue: { tumour: [1, 2, 3], stroma: [4, 5, 6] } }, + }); + const b = featureColorSchemeSignature({ + categoricalPalette: { byValue: { stroma: [4, 5, 6], tumour: [1, 2, 3] } }, + }); + + expect(a).toBe(b); + }); + + it('still separates named palettes that differ in a colour', () => { + expect( + featureColorSchemeSignature({ categoricalPalette: { byValue: { tumour: [1, 2, 3] } } }) + ).not.toBe( + featureColorSchemeSignature({ categoricalPalette: { byValue: { tumour: [9, 9, 9] } } }) + ); + }); +}); + +describe('a ramp with more than two stops', () => { + const diverging: [number, number, number][] = [ + [0, 0, 255], + [255, 255, 255], + [255, 0, 0], + ]; + + it('passes through the middle stop at the middle of the domain', () => { + // The whole reason to allow more than two: a diverging ramp's midpoint is the + // meaning. Interpolating its endpoints alone would put grey-purple here. + const colors = assignFeatureColors({ + values: ['0', '5', '10'], + mode: 'continuous', + alpha: 255, + numericRamp: diverging, + numericDomain: [0, 10], + }); + + expect(colors).toEqual([ + [0, 0, 255, 255], + [255, 255, 255, 255], + [255, 0, 0, 255], + ]); + }); + + it('lands on the last stop at the top of the domain, not past it', () => { + const colors = assignFeatureColors({ + values: ['10'], + mode: 'continuous', + alpha: 255, + numericRamp: diverging, + numericDomain: [0, 10], + }); + + expect(colors[0]).toEqual([255, 0, 0, 255]); + }); + + it('spreads a long tail with a symlog scale', () => { + const values = ['0', '1', '10', '1000']; + const opts = { + mode: 'continuous' as const, + alpha: 255, + numericRamp: diverging, + numericDomain: [0, 1000] as const, + }; + + const linear = assignFeatureColors({ values, ...opts }); + const log = assignFeatureColors({ values, ...opts, numericScale: 'symlog' as const }); + + /** Largest per-channel difference — how far apart two colours actually look. */ + const apart = (a?: number[], b?: number[]) => + Math.max(...[0, 1, 2].map((i) => Math.abs((a?.[i] ?? 0) - (b?.[i] ?? 0)))); + + // Linear: 1 and 10 both sit within 1% of the bottom of the domain, so the + // whole low end of the column collapses into one indistinguishable colour. + expect(apart(linear[1], linear[0])).toBeLessThan(10); + expect(apart(linear[2], linear[0])).toBeLessThan(10); + // Symlog pulls them apart into colours a reader can actually tell apart. + expect(apart(log[1], log[0])).toBeGreaterThan(40); + expect(apart(log[2], log[1])).toBeGreaterThan(40); + // The endpoints still pin to the ends of the ramp. + expect(log[0]).toEqual([0, 0, 255, 255]); + expect(log[3]).toEqual([255, 0, 0, 255]); + }); + + it('handles a domain that crosses zero, where a plain log could not', () => { + const colors = assignFeatureColors({ + values: ['-100', '0', '100'], + mode: 'continuous', + alpha: 255, + numericRamp: diverging, + numericDomain: [-100, 100], + numericScale: 'symlog', + }); + + expect(colors[0]).toEqual([0, 0, 255, 255]); + expect(colors[1]).toEqual([255, 255, 255, 255]); + expect(colors[2]).toEqual([255, 0, 0, 255]); + }); +}); + +/** + * A scheme arrives from a saved Render Stack, so its type is a claim about JSON + * rather than a guarantee. Every case here used to reach the colour arithmetic and + * fail there — `Cannot read properties of undefined (reading '0')`, several frames + * from the malformed field, inside a bundled dependency. Wrong colours can be + * reported by whoever sees them; that TypeError cannot. + */ +describe('a scheme that does not match its own type', () => { + const values = ['tumour', 'stroma']; + + const malformed = (categoricalPalette: unknown) => + assignFeatureColors({ + values, + mode: 'categorical', + alpha: 255, + categoricalPalette: categoricalPalette as never, + }); + + /** + * What the same column looks like with no palette at all. "Falls back" means + * these exact colours: a scheme the caller cannot read should be indistinguishable + * from one they never wrote, not merely something that avoided throwing. + */ + const defaultScheme = assignFeatureColors({ values, mode: 'categorical', alpha: 255 }); + + it('falls back to the default scheme for a palette object with no byValue', () => { + const colors = malformed({ fallback: 'oklab' }); + + expect(colors).toEqual(defaultScheme); + expect(colors[0]).not.toEqual(colors[1]); + }); + + it('falls back to the default scheme for a null palette', () => { + // The sharp edge: `typeof null === 'object'` and `Array.isArray(null)` is + // false, so `null` reads as a named palette to any guard that does not say + // otherwise — and then the destructure throws before any colour is assigned. + // `{"categoricalPalette": null}` is a thing JSON says. + const colors = malformed(null); + + expect(colors).toEqual(defaultScheme); + expect(colors[0]).not.toEqual(colors[1]); + }); + + it('takes a signature for a null palette instead of throwing on one', () => { + // Same root cause, different entry point: the signature helper narrows with + // the same guard, so a null palette used to take the cache key down with it. + expect(() => featureColorSchemeSignature({ categoricalPalette: null as never })).not.toThrow(); + }); + + it('survives a list with a hole in it', () => { + const colors = malformed([[1, 2, 3], undefined]); + + expect(colors[0]).toBeDefined(); + expect(colors[1]).toBeDefined(); + }); + + it('survives a ramp with fewer stops than its type allows', () => { + const one = assignFeatureColors({ + values: ['0', '5', '10'], + mode: 'continuous', + alpha: 255, + numericRamp: [[7, 8, 9]] as never, + }); + + expect(one).toEqual([ + [7, 8, 9, 255], + [7, 8, 9, 255], + [7, 8, 9, 255], + ]); + }); + + it('survives an empty ramp', () => { + const none = assignFeatureColors({ + values: ['0', '10'], + mode: 'continuous', + alpha: 255, + numericRamp: [] as never, + }); + + expect(none[0]).toBeDefined(); + expect(none[1]).toBeDefined(); + }); +}); diff --git a/packages/layers/tests/labelColorEncoding.spec.ts b/packages/layers/tests/labelColorEncoding.spec.ts index 74613673..54a8ce15 100644 --- a/packages/layers/tests/labelColorEncoding.spec.ts +++ b/packages/layers/tests/labelColorEncoding.spec.ts @@ -8,6 +8,7 @@ import { resolveHighlightedLabel, } from '../src/labelColorEncoding'; import { featureCodeToRgb } from '../src/pointsFeatureColor'; +import { buildShapeFillColorByFeatureId } from '../src/shapeColorEncoding'; /** A small fixed palette, for tests whose subject is row alignment rather than * colour choice — the default scheme is procedural, so colours must be pinned. */ @@ -44,29 +45,44 @@ describe('label fill colour encoding', () => { }); expect(colors).toEqual({ - '1': [0, 0, 255, 255], - '2': [0, 255, 0, 255], - '3': [0, 0, 255, 255], + // `stroma` sorts before `tumour`, so it takes palette slot 0. + '1': [0, 255, 0, 255], + '2': [0, 0, 255, 255], + '3': [0, 255, 0, 255], }); }); it('gives a label the same colour the same category gets on a shapes layer', () => { - const shared = { + // The two kinds walk their features in orders neither controls: a labels layer + // walks the raster's ids, a shapes layer walks the loader's geometry. Here they + // walk the SAME two rows in OPPOSITE orders, which is the whole test — under + // first-seen category indices `tumour` would be slot 0 on one kind and slot 1 + // on the other, and one annotation would render in two different colour schemes. + const column = ['tumour', 'stroma']; + + const labelColors = buildLabelFillColorByFeatureId({ rowIds: ['1', '2'], rowIndexByFeatureId: new Map([ ['1', 0], ['2', 1], ]), - column: ['tumour', 'stroma'], - mode: 'categorical' as const, - }; + column, + mode: 'categorical', + }); + const shapeColors = buildShapeFillColorByFeatureId({ + featureIds: ['stroma-shape', 'tumour-shape'], + rowIndexByFeatureIndex: new Int32Array([1, 0]), + column, + mode: 'categorical', + alpha: 255, + }); + expect(labelColors['1']).toEqual(shapeColors['tumour-shape']); + expect(labelColors['2']).toEqual(shapeColors['stroma-shape']); // Default scheme, no palette passed: the OkLab colours points uses for codes // 0 and 1. One scheme across points, shapes and labels. - expect(buildLabelFillColorByFeatureId(shared)).toEqual({ - '1': [...featureCodeToRgb(0), 255], - '2': [...featureCodeToRgb(1), 255], - }); + expect(labelColors['2']).toEqual([...featureCodeToRgb(0), 255]); + expect(labelColors['1']).toEqual([...featureCodeToRgb(1), 255]); }); it('uses the same palette and ramp as shapes for the same column', () => { diff --git a/packages/layers/tests/shapeColorEncoding.spec.ts b/packages/layers/tests/shapeColorEncoding.spec.ts index 35c89617..7b864904 100644 --- a/packages/layers/tests/shapeColorEncoding.spec.ts +++ b/packages/layers/tests/shapeColorEncoding.spec.ts @@ -25,9 +25,11 @@ describe('shape fill colour encoding', () => { }); expect(colors).toEqual({ - 'cell-a': [0, 0, 255, 180], - 'cell-b': [0, 255, 0, 180], - 'cell-c': [0, 0, 255, 180], + // Categories are ordered by value, not by which feature was seen first, so + // `type-x` takes palette slot 0 even though `type-y` is drawn first. + 'cell-a': [0, 255, 0, 180], + 'cell-b': [0, 0, 255, 180], + 'cell-c': [0, 255, 0, 180], 'cell-d': [255, 0, 255, 180], }); }); @@ -122,8 +124,9 @@ describe('shape fill colour encoding', () => { }); expect(colors).toEqual({ - 'circle-a': [0, 0, 255, 180], - 'circle-b': [0, 255, 0, 180], + // Row 1 is `type-y` (slot 1), row 0 is `type-x` (slot 0). + 'circle-a': [0, 255, 0, 180], + 'circle-b': [0, 0, 255, 180], }); }); diff --git a/packages/layers/vite.config.ts b/packages/layers/vite.config.ts index bfcc07ac..6a31a03f 100644 --- a/packages/layers/vite.config.ts +++ b/packages/layers/vite.config.ts @@ -12,6 +12,9 @@ export default defineConfig({ }, build: { outDir: resolve(__dirname, 'dist'), + // See the note in `vite.config.base.ts`: a consumer debugging a crash in here + // has only the built artifact, so it has to carry a map back to source. + sourcemap: true, lib: { entry: resolve(__dirname, 'src/index.ts'), name: 'SpatialDataLayers', diff --git a/packages/vis/src/SpatialCanvas/labelsProjection.ts b/packages/vis/src/SpatialCanvas/labelsProjection.ts index 6cbd7a16..73562c04 100644 --- a/packages/vis/src/SpatialCanvas/labelsProjection.ts +++ b/packages/vis/src/SpatialCanvas/labelsProjection.ts @@ -85,11 +85,7 @@ export function getLabelFillColorSignature(config: LayerConfig | undefined): str const mode: LabelFillColorMode = config.fillColorByColumn.mode; // The scheme is part of the key: swapping a palette changes every colour without // touching the column, so a column-only key would keep serving the old colours. - const scheme = featureColorSchemeSignature( - config.fillColorByColumn.categoricalPalette, - config.fillColorByColumn.numericRamp, - config.fillColorByColumn.missingValues - ); + const scheme = featureColorSchemeSignature(config.fillColorByColumn); return [config.fillColorByColumn.columnName, mode, scheme].join(''); } @@ -137,6 +133,10 @@ export function buildLabelFillColorEntry( ? { categoricalPalette: fillColorByColumn.categoricalPalette } : {}), ...(fillColorByColumn.numericRamp ? { numericRamp: fillColorByColumn.numericRamp } : {}), + ...(fillColorByColumn.numericDomain + ? { numericDomain: fillColorByColumn.numericDomain } + : {}), + ...(fillColorByColumn.numericScale ? { numericScale: fillColorByColumn.numericScale } : {}), }), rowsSource: rows, }; diff --git a/packages/vis/src/SpatialCanvas/resolveInputs.ts b/packages/vis/src/SpatialCanvas/resolveInputs.ts new file mode 100644 index 00000000..01b90b17 --- /dev/null +++ b/packages/vis/src/SpatialCanvas/resolveInputs.ts @@ -0,0 +1,81 @@ +/** + * A value key over the layer-config fields that drive LOADING. + * + * `useLayerData`'s reconcile effect is the one place a config change turns into a + * request to the Resource Resolver. Keying that effect on the identity of `layers` + * (or of the configs inside it) assumes the caller allocates a fresh config object + * per edit — and one important caller does the opposite on purpose. + * + * MDV's render-stack adapter keeps ONE `LayerConfig` object per Stack Entry and + * patches it in place, so that a cosmetic edit (opacity, a channel colour) does not + * look like a structural change and re-enter async geometry loads. Under that + * caller, `layers` and every config in it hold their identity across an edit, so an + * identity-keyed effect never re-runs: the resolver is never asked for the new + * column, `getShapeFillColorEntry` / `getLabelFillColorEntry` correctly keep serving + * the last-good rows (#119), and the layer paints the PREVIOUS column's colours for + * good. The identity discipline the render path needs and the change detection the + * load path needs are different jobs; this key does the second one by value. + * + * Cheap on purpose. It is recomputed every render — that is the whole point, since + * a mutation is invisible to any memo — so it holds scalars and short id lists only. + * Nothing here may serialise a payload that scales with the data: a categorical + * palette, a colour map, a feature catalog. Those affect how a resource is DRAWN, + * not whether it must be loaded, and the projections already handle them by value. + * + * INVARIANT: every config field that a resolver's `plan()` reads must appear here. + * The lines below mirror, one for one, what each `plan()` looks at today — + * `ShapesResolver` and `LabelsResolver` (tooltip fields, fill-colour column), + * `PointsResolver` (memory cap, feature selection, colour-by), and `ImagesResolver` + * (nothing: it plans its loader off element identity alone, which `elementMap` + * already covers). A resolver that starts planning off a new field must add it here + * in the same change, or that field gets exactly the bug described above. + */ + +import type { LayerConfig } from './types'; + +/** Field separator. Neither a layer id, element key nor column name may contain it. */ +const FIELD = '\u0001'; +/** Item separator, for the short id lists (tooltip fields, feature selections). */ +const ITEM = '\u0000'; + +function joinIds(ids: readonly (string | number)[] | undefined): string { + return ids && ids.length > 0 ? ids.join(ITEM) : ''; +} + +/** + * Serialise the load-relevant inputs of the visible layers, in render order. + * + * Invisible layers are omitted rather than encoded, matching the reconcile effect: + * hiding a layer withdraws its resolve context, which is itself a change of key. + */ +export function describeResolveInputs( + layers: Record, + layerOrder: readonly string[] +): string { + const parts: string[] = []; + for (const layerId of layerOrder) { + const config = layers[layerId]; + if (!config?.visible) continue; + parts.push(layerId, config.type, config.elementKey); + switch (config.type) { + case 'shapes': + case 'labels': + parts.push(joinIds(config.tooltipFields), config.fillColorByColumn?.columnName ?? ''); + break; + case 'points': + parts.push( + String(config.pointsMemoryCap ?? ''), + // Both selection forms: names are the durable one and take precedence, but + // codes still drive a config written before names existed. + joinIds(config.featureNames), + joinIds(config.featureCodes), + String(config.colorByFeature ?? '') + ); + break; + case 'image': + // `ImagesResolver.plan` reads no config — only whether the loader is idle. + break; + } + } + return parts.join(FIELD); +} diff --git a/packages/vis/src/SpatialCanvas/shapesProjection.ts b/packages/vis/src/SpatialCanvas/shapesProjection.ts index 19f7d5d7..18540bd5 100644 --- a/packages/vis/src/SpatialCanvas/shapesProjection.ts +++ b/packages/vis/src/SpatialCanvas/shapesProjection.ts @@ -77,11 +77,7 @@ export function getShapeFillColorSignature(config: LayerConfig | undefined): str const mode: ShapeFillColorMode = config.fillColorByColumn.mode; // The scheme is part of the key: swapping a palette changes every colour without // touching the column, so a column-only key would keep serving the old colours. - const scheme = featureColorSchemeSignature( - config.fillColorByColumn.categoricalPalette, - config.fillColorByColumn.numericRamp, - config.fillColorByColumn.missingValues - ); + const scheme = featureColorSchemeSignature(config.fillColorByColumn); return [ config.fillColorByColumn.columnName, mode, diff --git a/packages/vis/src/SpatialCanvas/types.ts b/packages/vis/src/SpatialCanvas/types.ts index 6d49c48f..19e0b229 100644 --- a/packages/vis/src/SpatialCanvas/types.ts +++ b/packages/vis/src/SpatialCanvas/types.ts @@ -7,7 +7,9 @@ import type { SpatialElement } from '@spatialdata/core'; import type { FeatureCategoricalPaletteSpec, FeatureMissingValueOptions, + FeatureNumericDomain, FeatureNumericRampSpec, + FeatureNumericScale, LabelFillColorMode, ShapeFillColorMode, ShapeStrokeWidthUnits, @@ -40,12 +42,33 @@ export interface FillColorByColumn { /** * Categorical scheme. Defaults to `'oklab'` — the same unbounded golden-angle * OKLCh scheme points uses for colour-by-feature, so a column with more - * categories than a fixed list has colours does not repeat them. Pass your own - * RGB list to override it; the list cycles. + * categories than a fixed list has colours does not repeat them. + * + * Pass `{ byValue }` to name the colours — `{ Tumour: [200, 30, 30] }` — which is + * the form to prefer in a saved stack, and the only one an embedding application + * can use to make this layer agree with its own charts. An RGB list is also + * accepted and cycles, but it is positional: which category gets which entry + * depends on the categories present. */ categoricalPalette?: FeatureCategoricalPaletteSpec; - /** Endpoints of the continuous ramp, `[low, high]` as RGB 0–255. */ + /** + * The colours a continuous column ramps through, low to high: two or more RGB + * 0–255 stops, spaced evenly across the domain. Two is the default; pass more to + * carry a real ramp — viridis, a diverging red/white/blue — whose midpoint is + * part of what it means. + */ numericRamp?: FeatureNumericRampSpec; + /** + * The values the ramp's ends stand for. Defaults to the extent of the features + * that loaded — pin it to the column's own range when you know it, so two layers + * over different parts of one annotation stay comparable. + */ + numericDomain?: FeatureNumericDomain; + /** + * How a value's position along the ramp is measured — `'linear'` by default, or + * `'symlog'` for a column whose mass sits near zero with a long tail. + */ + numericScale?: FeatureNumericScale; /** * What counts as missing in this column, and how a feature with no value should * render — keep the layer default, hide it, or take an explicit colour. diff --git a/packages/vis/src/SpatialCanvas/useLayerData.ts b/packages/vis/src/SpatialCanvas/useLayerData.ts index 5ecd319b..ef94b5a8 100644 --- a/packages/vis/src/SpatialCanvas/useLayerData.ts +++ b/packages/vis/src/SpatialCanvas/useLayerData.ts @@ -60,6 +60,7 @@ import { } from './labelsProjection'; import { renderLabelsLayer } from './renderers/labelsRenderer'; import { renderShapesLayer } from './renderers/shapesRenderer'; +import { describeResolveInputs } from './resolveInputs'; import { createNonOwningResolver } from './resolvers/nonOwningResolver'; import { ImagesResolver, LabelsResolver } from './resolvers/RasterResolvers'; import { @@ -416,6 +417,11 @@ export function useLayerData( // eslint-disable-next-line react-hooks/refs -- intentional latest-`layers` mirror consumed during render, see comment above layersRef.current = layers; + // Recomputed every render, NOT memoised on `layers`: a caller that patches its + // configs in place changes nothing this hook could memoise on. It is a handful of + // scalars per visible layer — see `describeResolveInputs` for what may live in it. + const resolveInputsKey = describeResolveInputs(layers, layerOrder); + const [layerLoadStates, setLayerLoadStates] = useState>({}); // Bumped on every resolver settle. The reconcile effect depends on it so that an // async settle (e.g. the preload landing, which flips `supportsFeatureScan`) re-runs @@ -592,12 +598,12 @@ export function useLayerData( // Compiler (`'use no memo'`) — the compiler otherwise memoizes JSX built from these // resolver getters and never repaints on a late async settle. // - // Caveat: `SpatialEntryStore` subscribes to its resolvers in its constructor, which - // runs inside the `useMemo` above. Under React StrictMode's dev-only double-invoke a - // discarded store instance leaks one listener on the (never-disposed) points - // resolver per rebuild; each such listener only calls a dead store's `notify()` - // (empty listener set), so it is inert. Harmless in production; noted so it is not - // mistaken for a real leak. + // The cleanup below is why the store's resolver bridge is tied to having listeners + // rather than to construction. An effect cleanup is not "the end": StrictMode's + // dev double-mount runs it and then re-runs the effect against the SAME memoised + // store, so `dispose()` here has to be recoverable — `subscribe` re-attaches. It + // also means a store the `useMemo` above builds and discards never subscribed to + // anything, so it holds nothing to leak. See `SpatialEntryStore.attachResolvers`. useEffect(() => { const unsubscribe = store.subscribe(notifyLoadedDataChanged); return () => { @@ -616,12 +622,23 @@ export function useLayerData( // element resolution changes without `layers`/`store` changing — e.g. a coordinate // system switch that makes a previously unavailable element resolvable. The map is // memoised on `availableElements`, so this adds no per-render churn. + // + // And on `resolveInputsKey`, which is what makes the effect fire for a caller that + // edits its configs IN PLACE (MDV's render-stack adapter does, deliberately). For + // such a caller neither `layers` nor the config objects inside it ever change + // identity, so identity deps alone leave a switched fill-colour column or tooltip + // field never requested at all. See `describeResolveInputs`. useEffect(() => { // Bare reference: `loadedDataRevision` is a re-trigger, not a value we read. A // resolver settle (the preload landing flips `supportsFeatureScan`) must replan so // the scan/row-codes tasks get emitted; touching it here declares that dependency // honestly to exhaustive-deps. The plan/load dedup makes the extra runs convergent. void loadedDataRevision; + // Bare reference, same reason: the contexts below are read from `layers`, so the + // key is never a value this body uses — it is the only thing that CHANGES when a + // caller mutates those configs in place. Declaring it here is what makes it a + // legitimate dependency rather than an "unnecessary" one. + void resolveInputsKey; const contexts: AnyResolveContext[] = []; for (const layerId of layerOrder) { const config = layers[layerId]; @@ -694,7 +711,15 @@ export function useLayerData( // the list because the name→code resolution above reads its catalog. The catalog // ARRIVING is covered by `loadedDataRevision` (bumped on every resolver settle), // which is what re-resolves a name selection that could not be resolved yet. - }, [layers, layerOrder, store, elementMapValue, loadedDataRevision, pointsEngine]); + }, [ + layers, + layerOrder, + resolveInputsKey, + store, + elementMapValue, + loadedDataRevision, + pointsEngine, + ]); // --- Shapes projection memos (Renderer Adapter side, kept in vis) ------------- @@ -789,6 +814,12 @@ export function useLayerData( ? { categoricalPalette: fillColorByColumn.categoricalPalette } : {}), ...(fillColorByColumn.numericRamp ? { numericRamp: fillColorByColumn.numericRamp } : {}), + ...(fillColorByColumn.numericDomain + ? { numericDomain: fillColorByColumn.numericDomain } + : {}), + ...(fillColorByColumn.numericScale + ? { numericScale: fillColorByColumn.numericScale } + : {}), }), rowsSource: rows, renderSource: renderData, diff --git a/packages/vis/tests/labelsProjection.spec.ts b/packages/vis/tests/labelsProjection.spec.ts index 0cdc9dcb..2d7dd061 100644 --- a/packages/vis/tests/labelsProjection.spec.ts +++ b/packages/vis/tests/labelsProjection.spec.ts @@ -60,8 +60,8 @@ describe('buildLabelFillColorEntry', () => { ); expect(entry?.fillColorByFeatureId).toEqual({ - '1': [0, 0, 255, 255], - '2': [0, 255, 0, 255], + '1': [0, 255, 0, 255], + '2': [0, 0, 255, 255], }); }); @@ -81,8 +81,8 @@ describe('buildLabelFillColorEntry', () => { ); expect(entry?.fillColorByFeatureId).toEqual({ - '1': [10, 20, 30, 255], - '2': [40, 50, 60, 255], + '1': [40, 50, 60, 255], + '2': [10, 20, 30, 255], }); }); diff --git a/packages/vis/tests/resolveInputs.spec.ts b/packages/vis/tests/resolveInputs.spec.ts new file mode 100644 index 00000000..ecde4123 --- /dev/null +++ b/packages/vis/tests/resolveInputs.spec.ts @@ -0,0 +1,129 @@ +import { describe, expect, it } from 'vitest'; +import { describeResolveInputs } from '../src/SpatialCanvas/resolveInputs.js'; +import type { LayerConfig } from '../src/SpatialCanvas/types.js'; + +/** + * The key has two jobs and they pull against each other: it must move for every + * config change that starts a LOAD (or an in-place caller never gets its data), and + * it must NOT move for a cosmetic one (or every opacity drag replans the world). + * Both halves are pinned here. + */ + +const shapes = (extra: Partial> = {}): LayerConfig => ({ + id: 'layer-1', + type: 'shapes', + elementKey: 'cells', + visible: true, + opacity: 1, + ...extra, +}); + +const key = (config: LayerConfig) => describeResolveInputs({ [config.id]: config }, [config.id]); + +describe('describeResolveInputs — what starts a load', () => { + it('moves when the fill-colour column changes', () => { + expect( + key(shapes({ fillColorByColumn: { columnName: 'colA', mode: 'categorical' } })) + ).not.toBe(key(shapes({ fillColorByColumn: { columnName: 'colB', mode: 'categorical' } }))); + }); + + it('moves when the fill-colour column is cleared', () => { + expect( + key(shapes({ fillColorByColumn: { columnName: 'colA', mode: 'categorical' } })) + ).not.toBe(key(shapes())); + }); + + it('moves when the tooltip fields change', () => { + expect(key(shapes({ tooltipFields: ['a'] }))).not.toBe(key(shapes({ tooltipFields: ['b'] }))); + }); + + it('moves when the element behind a layer changes', () => { + expect(key(shapes())).not.toBe(key(shapes({ elementKey: 'nuclei' }))); + }); + + it('moves when a layer is hidden', () => { + expect(key(shapes())).not.toBe(key(shapes({ visible: false }))); + }); + + it('moves for every points field a scan is planned from', () => { + const points = (extra: Partial>): LayerConfig => ({ + id: 'layer-p', + type: 'points', + elementKey: 'transcripts', + visible: true, + opacity: 1, + ...extra, + }); + const base = key(points({})); + + expect(key(points({ pointsMemoryCap: 1_000 }))).not.toBe(base); + expect(key(points({ featureNames: ['EPCAM'] }))).not.toBe(base); + expect(key(points({ featureCodes: [3] }))).not.toBe(base); + expect(key(points({ colorByFeature: false }))).not.toBe(base); + }); + + it('separates two layers that would otherwise concatenate into the same key', () => { + // `a` + `bc` must not read as `ab` + `c`: without a field separator, moving a + // character across the boundary would be invisible to the effect. + const one = describeResolveInputs( + { a: shapes({ id: 'a', tooltipFields: ['x'] }), bc: shapes({ id: 'bc' }) }, + ['a', 'bc'] + ); + const two = describeResolveInputs( + { ab: shapes({ id: 'ab', tooltipFields: ['x'] }), c: shapes({ id: 'c' }) }, + ['ab', 'c'] + ); + + expect(one).not.toBe(two); + }); +}); + +describe('describeResolveInputs — what does not', () => { + // The effect it keys runs `store.reconcile` for every entry. A key that moved on + // a slider drag would put that on the drag's critical path for no load at all. + it('ignores opacity, colours and stroke — nothing there is loaded', () => { + const base = key(shapes()); + + expect(key(shapes({ opacity: 0.3 }))).toBe(base); + expect(key(shapes({ fillColor: [1, 2, 3, 4] }))).toBe(base); + expect(key(shapes({ strokeColor: [1, 2, 3, 4], strokeWidth: 9 }))).toBe(base); + }); + + it('ignores the colour SCHEME of the fill column — same rows, different encoding', () => { + const withColumn = shapes({ fillColorByColumn: { columnName: 'colA', mode: 'categorical' } }); + const withPalette = shapes({ + fillColorByColumn: { + columnName: 'colA', + mode: 'categorical', + categoricalPalette: { A: [1, 2, 3] }, + }, + }); + + expect(key(withPalette)).toBe(key(withColumn)); + }); + + it('ignores per-feature state — a hidden or recoloured feature loads nothing new', () => { + expect( + key( + shapes({ + featureState: { hiddenFeatureIds: ['c1'], fillColorByFeatureId: { c2: [1, 2, 3, 4] } }, + }) + ) + ).toBe(key(shapes())); + }); + + it('ignores image channel state — the images resolver plans off the element alone', () => { + const image = ( + channels?: Extract['channels'] + ): LayerConfig => ({ + id: 'layer-i', + type: 'image', + elementKey: 'morphology', + visible: true, + opacity: 1, + ...(channels ? { channels } : {}), + }); + + expect(key(image({ colors: [[255, 0, 0]], contrastLimits: [[0, 100]] }))).toBe(key(image())); + }); +}); diff --git a/packages/vis/tests/useLayerData.spec.tsx b/packages/vis/tests/useLayerData.spec.tsx index 6d1aaf29..4ea06048 100644 --- a/packages/vis/tests/useLayerData.spec.tsx +++ b/packages/vis/tests/useLayerData.spec.tsx @@ -620,3 +620,107 @@ describe('useLayerData — the hover highlight channel', () => { expect(getRenders()).toBe(baseline); }); }); + +describe('useLayerData — a caller that mutates its layer configs in place', () => { + // MDV's render-stack adapter keeps ONE `LayerConfig` object per stack entry and + // patches it in place, so a cosmetic edit does not re-enter async geometry loads. + // The `layers` record it hands over therefore keeps its identity across an edit — + // which means config identity is NOT a "the load inputs changed" signal, and any + // load the hook plans off that identity silently never happens. + // + // The symptom that got here: switching an already-loaded shapes/labels layer to a + // different `fillColorByColumn` left the colours on the previous column forever. + // The projection had no rows for the new column, so (correctly, per #119) it kept + // serving the last-good entry — but nothing ever asked the resolver to load the + // new column, so "last good" was all there would ever be. + function tableSpatialData() { + const columnValues: Record = { + region: ['cells', 'cells'], + colA: ['a', 'a'], + colB: ['b', 'b'], + }; + const loadObsColumns = vi.fn(async (names: string[]) => + names.map((name) => columnValues[name] ?? []) + ); + const table = { + getTableKeys: () => ({ region: ['cells'], regionKey: 'region' }), + loadObsIndex: vi.fn(async () => ['c1', 'c2']), + loadObsColumns, + }; + const spatialData = { + getAssociatedTable: vi.fn(() => ['table', table]), + } as unknown as SpatialData; + return { spatialData, loadObsColumns }; + } + + /** Every column name the hook has asked the associated table for, in order. */ + const requestedColumns = (loadObsColumns: ReturnType): string[] => + loadObsColumns.mock.calls.flatMap((call) => call[0] as string[]); + + it('loads the new fill-colour column when a shapes config is switched in place', async () => { + const { spatialData, loadObsColumns } = tableSpatialData(); + const elements: ElementsByType = { ...EMPTY_ELEMENTS, shapes: [shapesElement('cells')] }; + // ONE config object, ONE record — both keep their identity for the whole test, + // exactly as they do under the render-stack adapter. + const config: LayerConfig = { + ...shapesConfig('layer-1', 'cells'), + fillColorByColumn: { columnName: 'colA', mode: 'categorical' }, + }; + const layers = { 'layer-1': config }; + const layerOrder = Object.keys(layers); + + const { rerender } = renderHook(() => + useLayerData(layers, layerOrder, elements, null, spatialData) + ); + + await waitFor(() => { + expect(requestedColumns(loadObsColumns)).toContain('colA'); + }); + + // The switch the user makes in the panel: same config object, new column. + config.fillColorByColumn = { columnName: 'colB', mode: 'categorical' }; + rerender(); + + await waitFor(() => { + expect(requestedColumns(loadObsColumns)).toContain('colB'); + }); + }); + + it('loads the new fill-colour column when a labels config is switched in place', async () => { + const { spatialData, loadObsColumns } = tableSpatialData(); + const labels: AvailableElement = { + key: 'segmentation', + type: 'labels', + // The loader load will fail (no real zarr behind it) and that is fine: the + // fill-colour column is a resource of its own and must load regardless. + element: { key: 'segmentation' } as unknown as AvailableElement['element'], + transform: new Matrix4(), + }; + const elements: ElementsByType = { ...EMPTY_ELEMENTS, labels: [labels] }; + const config: LayerConfig = { + id: 'layer-l', + type: 'labels', + elementKey: 'segmentation', + visible: true, + opacity: 1, + fillColorByColumn: { columnName: 'colA', mode: 'categorical' }, + }; + const layers = { 'layer-l': config }; + const layerOrder = Object.keys(layers); + + const { rerender } = renderHook(() => + useLayerData(layers, layerOrder, elements, null, spatialData) + ); + + await waitFor(() => { + expect(requestedColumns(loadObsColumns)).toContain('colA'); + }); + + config.fillColorByColumn = { columnName: 'colB', mode: 'categorical' }; + rerender(); + + await waitFor(() => { + expect(requestedColumns(loadObsColumns)).toContain('colB'); + }); + }); +}); diff --git a/vite.config.base.ts b/vite.config.base.ts index de0f2aee..d6cc6bff 100644 --- a/vite.config.base.ts +++ b/vite.config.base.ts @@ -67,6 +67,10 @@ export function defineViteConfig(options: DefineConfigOptions) { ], build: { outDir: path.resolve(pkgRoot, 'dist'), + // Published so a consumer's stack trace names our source, not `Le` at + // `.vite/deps/@spatialdata_layers.js:396`. An embedding application debugs + // against the built artifact — it is the only form of this code it has. + sourcemap: true, lib: { entry: path.resolve(pkgRoot, 'src/index.ts'), name: libName,