diff --git a/.changeset/labels-hover-highlight.md b/.changeset/labels-hover-highlight.md new file mode 100644 index 00000000..7ed2130f --- /dev/null +++ b/.changeset/labels-hover-highlight.md @@ -0,0 +1,10 @@ +--- +'@spatialdata/layers': minor +'@spatialdata/vis': minor +--- + +Highlight the labels feature under the cursor, the way shapes already do. + +Nothing to configure on either canvas surface: the highlight follows the same hover pick +that feeds the tooltip, so it respects `hoverTooltipMode`. Hosts driving `LabelsLayer` +directly get `highlightedLabelId` and an optional `highlightColor`. diff --git a/docs/docs/vis/headless-viewer.mdx b/docs/docs/vis/headless-viewer.mdx index 47d53c33..5f88d355 100644 --- a/docs/docs/vis/headless-viewer.mdx +++ b/docs/docs/vis/headless-viewer.mdx @@ -284,6 +284,20 @@ that the fragment shader samples. The property that matters is the same on both: a feature-state change re-uploads only the small table, never the tiles. Picking consults that table too, so a hidden label cannot be picked. +Hovering a label highlights it, the way `autoHighlight` highlights a shape. Nothing to +configure: both canvas surfaces — `SpatialCanvasViewer` and the full-UI `SpatialCanvas` — +drive it from the same pick that feeds the tooltip, through one shared resolver +(`resolveHoveredLabel`). It therefore follows `hoverTooltipMode`: `'off'` disables picking +and with it the highlight. It is deliberately **not** part of the Render Stack: hover +changes on every pointer move and would be meaningless in a saved view, so it never +appears in an entry's `props`. A label the filter hides is never highlighted, for the same +reason it can never be picked. + +Hosts driving `LabelsLayer` directly get `highlightedLabelId` (the integer id, or `-1` for +none) and `highlightColor` — deck's own prop name, with deck's own meaning, so the tint is +set the same way on labels as on shapes. Alpha is the blend weight toward the tint, not an +opacity. + ### Choosing the colour scheme `fillColorByColumn` carries the scheme, not just the column name. Every field is diff --git a/docs/docs/vis/layer-prop-flow.mdx b/docs/docs/vis/layer-prop-flow.mdx index 77029022..22852c11 100644 --- a/docs/docs/vis/layer-prop-flow.mdx +++ b/docs/docs/vis/layer-prop-flow.mdx @@ -328,6 +328,34 @@ only the small table, **never the tiles**. element alone let two layers colouring one element by different columns evict each other on every plan. +### Hover highlight: a uniform, not a table entry + +The label under the cursor is drawn highlighted, matching what `autoHighlight` gives +shapes. Deck's own machinery cannot do it here: a labels tile's picking colour covers the +whole quad, so there is no per-label deck object for `picking_filterHighlightColor` to act +on, and enabling `autoHighlight` would light up the entire tile. The highlight is resolved +**per fragment** instead — the shader compares the sampled instance id against a +`highlightedLabelId` uniform, rather than a LUT entry, so a pointer move re-uploads +nothing. + +- **Both canvas surfaces resolve the pick through one function.** `SpatialCanvasViewer` + and the full-UI `SpatialCanvas` each own a `handleHover`; the highlight shipped working + in the first and dead in the second precisely because of that. `resolveHoveredLabel` in + `featureTooltipHover.ts` is the single implementation both must call — when adding hover + behaviour, add it there, not in one `handleHover`. +- **Hover is runtime render state, never Render Stack config.** `useLayerData` keeps it on + a ref plus a version counter (`setHoveredLabel`) so a saved view can never carry it, and + so pointer motion *within* one label schedules no re-render. Points already carry their + highlight the same way. +- **One slot, not a per-layer map.** Only one thing is under the cursor at a time, so + "moved to a different labels layer" and "moved off" are the same transition. +- **`resolveHighlightedLabel` refuses background and hidden labels** before the value + reaches the shader, so an id that went stale between the pick and the frame cannot light + up something the filter hides. +- **The tint reuses deck's `highlightColor`**, with the same meaning (alpha is the blend + weight), so one prop name covers shapes and labels. The labels default is declared in + `defaultProps`; see the anti-patterns below for why it cannot be a use-site fallback. + ### Anti-patterns (labels) | Anti-pattern | Why it's wrong | @@ -335,6 +363,8 @@ only the small table, **never the tiles**. | Building the LUT per tile sublayer | Duplicates a multi-megabyte table across every tile | | Returning a fresh `FeatureColorBuffer` wrapper each render | Re-uploads the texture every frame; keep the `colors` identity stable | | Keying a colour resource by element alone | Two layers, two columns, one element → eviction ping-pong that never settles | +| Carrying the hovered label in the colour LUT | Re-uploads a multi-megabyte texture on every pointer move; use the uniform | +| Defaulting a prop deck also defines with `props.x ?? MY_DEFAULT` | deck fills its own default in, so the prop is never absent and your fallback never runs. This drew every labels hover in deck's navy `highlightColor`. Redefine it in the layer's `defaultProps`, which does override deck's | ## Shape/table annotation controls diff --git a/docs/docs/vis/mdv-release-checklist.mdx b/docs/docs/vis/mdv-release-checklist.mdx index a8be3a4e..283f6508 100644 --- a/docs/docs/vis/mdv-release-checklist.mdx +++ b/docs/docs/vis/mdv-release-checklist.mdx @@ -110,6 +110,10 @@ Labels take the same API, so MDV drives them the same way — no separate code p - [x] Colour resolves through a label-id-indexed LUT the shader samples, so a selection change re-uploads a small table rather than the tiles; picking consults the same table, so a hidden label cannot be picked. +- [x] Hovering a label highlights it, as `autoHighlight` does for shapes. MDV + configures nothing: it follows `hoverTooltipMode` and is driven from the same + pick as the tooltip. It is runtime render state, so it never appears in a + saved Render Stack entry — do not try to persist or restore it. The one thing MDV must get right is the id: a labels feature id is the label's **integer instance id as a string** — the raster's own pixel value, and the same diff --git a/packages/layers/src/LabelsBitmaskTileLayer.ts b/packages/layers/src/LabelsBitmaskTileLayer.ts index 28fdf55b..daa48cb5 100644 --- a/packages/layers/src/LabelsBitmaskTileLayer.ts +++ b/packages/layers/src/LabelsBitmaskTileLayer.ts @@ -3,9 +3,11 @@ import { picking, project32 } from '@deck.gl/core'; import { XRLayer } from '@hms-dbmi/viv'; import { Matrix4 } from '@math.gl/core'; import { + DEFAULT_LABEL_HIGHLIGHT_COLOR, isLabelVisibleInLut, LABEL_COLOR_LUT_WIDTH, type LabelColorLut, + resolveHighlightedLabel, } from './labelColorEncoding'; import { fs, labelsBitmaskUniforms, vs } from './labelsBitmaskLayerShaders'; @@ -91,6 +93,14 @@ export class LabelsBitmaskTileLayer extends UntypedXRLayer { // already megabytes for a large segmentation. featureColorLut: { type: 'object', value: null, compare: false }, featureColorTexture: { type: 'object', value: null, compare: false }, + // Hover state. A scalar prop rather than anything in the LUT, so a pointer move + // changes a uniform and nothing is re-uploaded. `-1` means "nothing hovered"; + // label 0 is background and is discarded before the highlight runs anyway. + highlightedLabelId: { type: 'number', value: -1, compare: true }, + // Deck's own `Layer` prop, redefaulted from its navy `[0, 0, 128, 128]`. Declaring + // it here is what makes the labels default win: deck fills its base default in, so + // the prop is never absent and a `?? DEFAULT` at the use site would never run. + highlightColor: { type: 'array', value: DEFAULT_LABEL_HIGHLIGHT_COLOR, compare: true }, }; // biome-ignore lint/complexity/noUselessConstructor: widens the base UntypedXRLayer constructor so `new LabelsBitmaskTileLayer(props)` typechecks. @@ -259,6 +269,8 @@ export class LabelsBitmaskTileLayer extends UntypedXRLayer { channelStrokeWidths, featureColorLut, featureColorTexture, + highlightedLabelId, + highlightColor, maxZoom, opacity = 1, zoom, @@ -274,8 +286,18 @@ export class LabelsBitmaskTileLayer extends UntypedXRLayer { const lut = featureColorLut as LabelColorLut | undefined; const useFeatureColors = lut && featureColorTexture ? 1 : 0; + const highlighted = resolveHighlightedLabel(highlightedLabelId as number | null, lut); + const highlightRgba = (highlightColor as readonly number[] | undefined) ?? []; + const highlightNormalized = getNormalizedColor( + highlightRgba.length >= 3 ? highlightRgba : DEFAULT_LABEL_HIGHLIGHT_COLOR + ); + const highlightWeight = + (highlightRgba.length >= 4 ? highlightRgba[3] : DEFAULT_LABEL_HIGHLIGHT_COLOR[3]) / 255; + const labelsBitmask = { color0: [...color, 1] as const, + highlightColor: [...highlightNormalized, highlightWeight] as const, + highlightedLabelId: highlighted, channelFilled0: (channelsFilled?.[0] ?? true) ? 1 : 0, channelOpacity0: channelOpacities?.[0] ?? 0.18, channelOutlineOpacity0: channelOutlineOpacities?.[0] ?? 0.95, diff --git a/packages/layers/src/LabelsLayer.ts b/packages/layers/src/LabelsLayer.ts index 7da8fdd4..134e88b4 100644 --- a/packages/layers/src/LabelsLayer.ts +++ b/packages/layers/src/LabelsLayer.ts @@ -12,10 +12,13 @@ import { import { LabelsBitmaskTileLayer } from './LabelsBitmaskTileLayer'; import { buildLabelColorLut, + DEFAULT_LABEL_HIGHLIGHT_COLOR, LABEL_COLOR_LUT_WIDTH, type LabelColorLut, type LabelFeatureStateInput, + type LabelRgbaColor, type LabelRgbColor, + NO_HIGHLIGHTED_LABEL, } from './labelColorEncoding'; /** One instance-ID raster per labels element (see `LabelsBitmaskTileLayer`). */ @@ -68,6 +71,27 @@ export interface LabelsLayerProps { * that {@link featureState} would otherwise do inside the layer. */ featureColorLut?: LabelColorLut; + /** + * The label id under the cursor, or `-1` / omitted for none. + * + * Runtime render state, not config: it changes on every pointer move and must + * never be serialized into a saved view. It reaches the shader as a uniform, so + * hovering re-uploads nothing — the LUT and the tiles are both untouched. + */ + highlightedLabelId?: number | null; + /** + * Hover tint, RGBA 0–255; defaults to {@link DEFAULT_LABEL_HIGHLIGHT_COLOR}. + * + * Deliberately deck's own `Layer` prop name, and deliberately the same meaning: + * deck blends its `highlightColor` over a highlighted fragment weighted by that + * colour's alpha, which is exactly what the labels shader does. One name across + * shapes (via `autoHighlight`) and labels. The labels default is declared in + * `defaultProps`, which is what overrides deck's navy base default. + * + * Unlike deck's, this one must be an array — the accessor/function form has no + * meaning here, since there is no per-label deck object to call it with. + */ + highlightColor?: LabelRgbaColor; onClick?: (info: unknown) => void; onHover?: (info: unknown) => void; _subLayerProps?: CompositeLayerProps['_subLayerProps']; @@ -172,6 +196,8 @@ class SingleScaleLabelsLayer extends CompositeLayer { selections: selectionsProp, featureColorLut, featureColorTexture, + highlightedLabelId, + highlightColor, } = this.props; const selections = [firstSelection(selectionsProp)]; const channelColors = stylePlane(channelColorsProp, [255, 255, 255]); @@ -210,6 +236,8 @@ class SingleScaleLabelsLayer extends CompositeLayer { selections, featureColorLut, featureColorTexture, + highlightedLabelId: highlightedLabelId ?? NO_HIGHLIGHTED_LABEL, + highlightColor, bounds, id: `image-sub-layer-${bounds}-${id}`, interpolation: 'nearest', @@ -340,6 +368,11 @@ export class LabelsLayer extends CompositeLayer { channelOutlineOpacities: [0.95], channelsFilled: [true], channelStrokeWidths: [1.5], + // Overrides deck's own `Layer` default for this prop (navy `[0, 0, 128, 128]`). + // The default MUST be declared here rather than applied with `?? DEFAULT` at the + // use site: deck fills its base default in, so the prop is never absent and a + // use-site fallback can never run. + highlightColor: DEFAULT_LABEL_HIGHLIGHT_COLOR, } satisfies Partial; /** @@ -424,6 +457,8 @@ export class LabelsLayer extends CompositeLayer { channelOutlineOpacities = [0.95], channelsFilled = [true], channelStrokeWidths = [1.5], + highlightedLabelId, + highlightColor, onClick, onHover, } = this.props; @@ -447,6 +482,8 @@ export class LabelsLayer extends CompositeLayer { opacity, featureColorLut, featureColorTexture: this.state?.featureColorTexture ?? null, + highlightedLabelId: highlightedLabelId ?? NO_HIGHLIGHTED_LABEL, + highlightColor, channelsVisible: stylePlane(channelsVisible, true), channelColors: stylePlane(channelColors, [255, 255, 255]), channelOpacities: stylePlane(channelOpacities, 0.18), diff --git a/packages/layers/src/index.ts b/packages/layers/src/index.ts index ce556377..efa0eb90 100644 --- a/packages/layers/src/index.ts +++ b/packages/layers/src/index.ts @@ -76,13 +76,16 @@ export { buildLabelFeatureStateRuntime, buildLabelFillColorByFeatureId, DEFAULT_LABEL_FILTERED_OPACITY_MULTIPLIER, + DEFAULT_LABEL_HIGHLIGHT_COLOR, EMPTY_LABEL_FEATURE_STATE_RUNTIME, isLabelFeatureStateRuntime, isLabelVisibleInLut, LABEL_COLOR_LUT_MAX_LABELS, LABEL_COLOR_LUT_WIDTH, + NO_HIGHLIGHTED_LABEL, normalizeLabelFeatureState, parseLabelId, + resolveHighlightedLabel, } from './labelColorEncoding'; export type { PointsLayerProps } from './PointsLayer'; export { PointsLayer } from './PointsLayer'; diff --git a/packages/layers/src/labelColorEncoding.ts b/packages/layers/src/labelColorEncoding.ts index 80f45dd6..096decb3 100644 --- a/packages/layers/src/labelColorEncoding.ts +++ b/packages/layers/src/labelColorEncoding.ts @@ -87,6 +87,52 @@ export type LabelFeatureStateInput = LabelFeatureState | LabelFeatureStateRuntim export const DEFAULT_LABEL_FILTERED_OPACITY_MULTIPLIER = 0.35; +/** + * Hover tint, matching the `highlightColor` shapes pass to deck's `autoHighlight` + * so the same pointer gesture reads the same way on either kind. + * + * Alpha is the MIX WEIGHT toward this colour, not an opacity — the shader also + * lifts the hovered label's fill, which a plain alpha blend over a 0.18-opacity + * fill would not make visible. + */ +export const DEFAULT_LABEL_HIGHLIGHT_COLOR: LabelRgbaColor = [255, 255, 0, 128]; + +/** `highlightedLabelId` when nothing is hovered. Label ids are non-negative. */ +export const NO_HIGHLIGHTED_LABEL = -1; + +/** + * The label the shader should actually draw as hovered, given the table in force. + * + * Separate from the layer because the interesting part is the refusals, and they + * are all cheap to get wrong: + * + * - Label `0` is background. It is discarded before the highlight runs, but a + * caller that maps "nothing picked" to `0` rather than `-1` would otherwise be + * asking to highlight the background. + * - A label the filter HIDES must not light up. Picking already refuses to return + * one, so this only catches an id that went stale between the pick and the + * frame — but the shader has no view of the hidden set beyond the LUT alpha it + * is about to discard on, so the check belongs on this side. + * - A non-finite or fractional id would reach the shader as a `float` and compare + * against sampled ids by proximity; rounding here keeps that comparison exact. + */ +export function resolveHighlightedLabel( + highlightedLabelId: number | null | undefined, + lut?: LabelColorLut +): number { + if (highlightedLabelId == null || !Number.isFinite(highlightedLabelId)) { + return NO_HIGHLIGHTED_LABEL; + } + const labelId = Math.round(highlightedLabelId); + if (labelId <= 0) { + return NO_HIGHLIGHTED_LABEL; + } + if (lut && !isLabelVisibleInLut(lut, labelId)) { + return NO_HIGHLIGHTED_LABEL; + } + return labelId; +} + /** Singleton for the common case of no feature-state at all. */ export const EMPTY_LABEL_FEATURE_STATE_RUNTIME = Object.freeze({ fillColorByFeatureId: new Map(), diff --git a/packages/layers/src/labelsBitmaskLayerShaders.ts b/packages/layers/src/labelsBitmaskLayerShaders.ts index d18c7375..ee6c048d 100644 --- a/packages/layers/src/labelsBitmaskLayerShaders.ts +++ b/packages/layers/src/labelsBitmaskLayerShaders.ts @@ -6,6 +6,7 @@ const labelsUniformBlock = `\ uniform labelsBitmaskUniforms { vec4 color0; + vec4 highlightColor; float channelOpacity0; float channelOutlineOpacity0; float channelStrokeWidth0; @@ -16,6 +17,7 @@ uniform labelsBitmaskUniforms { float useFeatureColors; float featureTexWidth; float featureCount; + float highlightedLabelId; } labelsBitmask; `; @@ -24,6 +26,13 @@ export const labelsBitmaskUniforms = { fs: labelsUniformBlock, uniformTypes: { color0: 'vec4', + /** + * Hover highlight tint, RGB 0–1 with alpha as the MIX WEIGHT (not an opacity). + * Kept next to `color0` because both are `vec4`: std140 aligns a `vec4` to 16 + * bytes, so grouping them ahead of the scalars keeps the block free of padding + * holes. + */ + highlightColor: 'vec4', channelOpacity0: 'f32', channelOutlineOpacity0: 'f32', channelStrokeWidth0: 'f32', @@ -37,6 +46,14 @@ export const labelsBitmaskUniforms = { featureTexWidth: 'f32', /** Number of addressable label ids; ids at or beyond this are unannotated. */ featureCount: 'f32', + /** + * The label id under the cursor, or `-1` for none. + * + * A uniform rather than a bit in the LUT: hover changes on every pointer move, + * and re-uploading a table that is megabytes for a large segmentation to carry + * one changed entry is the thing this whole design avoids. + */ + highlightedLabelId: 'f32', }, } as const; @@ -206,6 +223,26 @@ void main() { labelsBitmask.channelOutlineOpacity0 * featureStyle.a, labelsBitmask.channelFilled0 ); + + // Hover highlight — the labels analogue of deck's \`autoHighlight\` on shapes. + // + // Resolved per FRAGMENT from the sampled instance id, because a tile's deck + // picking colour covers the whole quad: there is no per-label deck object for + // \`picking_filterHighlightColor\` to act on, so enabling deck's own autoHighlight + // here would light up the entire tile. Placed after the hidden-label discard, so + // a filtered-out label cannot highlight even if a stale id points at it. + if (labelMatch(dat0.y, labelsBitmask.highlightedLabelId) > 0.5) { + vec4 highlight = labelsBitmask.highlightColor; + fragColor.rgb = mix(fragColor.rgb, highlight.rgb, highlight.a); + // Tinting alone is nearly invisible at the default fill opacity (0.18), so the + // hovered label's fill is lifted to at least the highlight's own weight. The + // coverage factor keeps the boundary anti-aliased, and \`channelFilled0\` gates + // it so outline-only mode highlights the outline instead of growing a fill the + // display mode says should not be there. + float fillBoost = highlight.a * dat0.z * step(0.5, labelsBitmask.channelFilled0); + fragColor.a = max(fragColor.a, fillBoost); + } + fragColor.a = fragColor.a * labelsBitmask.labelOpacity; fragColor = picking_filterHighlightColor(fragColor); diff --git a/packages/layers/tests/labelColorEncoding.spec.ts b/packages/layers/tests/labelColorEncoding.spec.ts index 082cf11a..74613673 100644 --- a/packages/layers/tests/labelColorEncoding.spec.ts +++ b/packages/layers/tests/labelColorEncoding.spec.ts @@ -3,7 +3,9 @@ import { buildLabelColorLut, buildLabelFillColorByFeatureId, isLabelVisibleInLut, + NO_HIGHLIGHTED_LABEL, parseLabelId, + resolveHighlightedLabel, } from '../src/labelColorEncoding'; import { featureCodeToRgb } from '../src/pointsFeatureColor'; @@ -194,3 +196,38 @@ describe('parseLabelId', () => { expect(parseLabelId('1e3')).toBeUndefined(); }); }); + +describe('resolveHighlightedLabel', () => { + it('passes through a real label id', () => { + expect(resolveHighlightedLabel(7)).toBe(7); + }); + + it('treats absent, non-finite and background ids as no highlight', () => { + expect(resolveHighlightedLabel(undefined)).toBe(NO_HIGHLIGHTED_LABEL); + expect(resolveHighlightedLabel(null)).toBe(NO_HIGHLIGHTED_LABEL); + expect(resolveHighlightedLabel(Number.NaN)).toBe(NO_HIGHLIGHTED_LABEL); + expect(resolveHighlightedLabel(Number.POSITIVE_INFINITY)).toBe(NO_HIGHLIGHTED_LABEL); + // Label 0 is background: never drawn, so never hovered. A caller that spells + // "nothing picked" as 0 rather than -1 must not light up the background. + expect(resolveHighlightedLabel(0)).toBe(NO_HIGHLIGHTED_LABEL); + expect(resolveHighlightedLabel(-1)).toBe(NO_HIGHLIGHTED_LABEL); + }); + + it('rounds, so the float the shader compares against is exact', () => { + expect(resolveHighlightedLabel(3.4)).toBe(3); + expect(resolveHighlightedLabel(3.6)).toBe(4); + }); + + it('refuses a label the table hides', () => { + const lut = buildLabelColorLut({ + featureState: { hiddenFeatureIds: ['2'] }, + defaultColor: [255, 255, 255], + }); + // Picking already refuses a hidden label; this catches an id that went stale + // between the pick and the frame, which the shader could not catch itself. + expect(resolveHighlightedLabel(2, lut)).toBe(NO_HIGHLIGHTED_LABEL); + expect(resolveHighlightedLabel(1, lut)).toBe(1); + // Past the end of the table is unannotated, not hidden. + expect(resolveHighlightedLabel(900, lut)).toBe(900); + }); +}); diff --git a/packages/layers/tests/labelsLayer.spec.ts b/packages/layers/tests/labelsLayer.spec.ts index c9a5d553..3fbf4fc4 100644 --- a/packages/layers/tests/labelsLayer.spec.ts +++ b/packages/layers/tests/labelsLayer.spec.ts @@ -1,7 +1,10 @@ import { describe, expect, it, vi } from 'vitest'; import type { LabelsLayerProps } from '../src/LabelsLayer'; import { LabelsLayer } from '../src/LabelsLayer'; -import { buildLabelColorLut } from '../src/labelColorEncoding'; +import { buildLabelColorLut, DEFAULT_LABEL_HIGHLIGHT_COLOR } from '../src/labelColorEncoding'; + +/** deck.gl core `Layer.defaultProps.highlightColor`, which ours must override. */ +const DECK_CORE_HIGHLIGHT_COLOR = [0, 0, 128, 128]; type TileLayerLike = { props: { @@ -261,6 +264,77 @@ describe('LabelsLayer prop flow', () => { expect(bitmaskLayer.props.featureColorLut).toBe(featureColorLut); }); + it('forwards the hovered label id to bitmask tiles', () => { + const { loader } = makeLabelsLoader(); + const tileLayer = renderLabelsLayer({ loader, highlightedLabelId: 2 }); + + const bitmaskLayer = tileLayer.props.renderSubLayers({ + ...tileLayer.props, + id: 'tile-0-0-0', + data: { data: [new Float32Array([0, 1, 2, 0])], width: 2, height: 2 }, + tile: { + bbox: { left: 0, top: 0, right: 2, bottom: 2 }, + index: { x: 0, y: 0, z: 0 }, + zoom: 0, + }, + }) as { props: Record }; + + expect(bitmaskLayer.props.highlightedLabelId).toBe(2); + }); + + it('redefaults deck’s own highlightColor to the labels tint', () => { + const { loader } = makeLabelsLoader(); + const tileLayer = renderLabelsLayer({ loader, highlightedLabelId: 2 }); + + const bitmaskLayer = tileLayer.props.renderSubLayers({ + ...tileLayer.props, + id: 'tile-0-0-0', + data: { data: [new Float32Array([0, 1, 2, 0])], width: 2, height: 2 }, + tile: { + bbox: { left: 0, top: 0, right: 2, bottom: 2 }, + index: { x: 0, y: 0, z: 0 }, + zoom: 0, + }, + }) as { props: Record }; + + // We reuse deck's `highlightColor` name, so the labels default has to be declared + // in `defaultProps` to beat deck's navy base default. Applying it with `?? DEFAULT` + // at the use site does NOT work — deck fills its own default in, so the prop is + // never absent and every hover drew navy. Assert the value, not the plumbing: that + // is the part that silently regressed. + expect(bitmaskLayer.props.highlightColor).toEqual(DEFAULT_LABEL_HIGHLIGHT_COLOR); + expect(bitmaskLayer.props.highlightColor).not.toEqual(DECK_CORE_HIGHLIGHT_COLOR); + }); + + it('does not reload tiles when only the hovered label changes', async () => { + const { loader, onGetTile } = makeLabelsLoader(); + const initial = renderLabelsLayer({ loader, highlightedLabelId: -1 }); + + await loadOneTile(initial); + expect(onGetTile).toHaveBeenCalledTimes(1); + + // The whole point of carrying hover as a uniform: the pointer moves constantly, + // and a segmentation's tiles are the most expensive thing on screen to refetch. + const hovered = renderLabelsLayer({ loader, highlightedLabelId: 7 }); + if (triggerChanged(initial, hovered)) { + await loadOneTile(hovered); + } + + expect(getTileTrigger(hovered)).toEqual(getTileTrigger(initial)); + expect(onGetTile).toHaveBeenCalledTimes(1); + }); + + it('does not rebuild the colour table when only the hovered label changes', () => { + const { loader } = makeLabelsLoader(); + const featureState = { hiddenFeatureIds: ['3'] }; + const initial = renderLabelsLayer({ loader, featureState, highlightedLabelId: -1 }); + const hovered = renderLabelsLayer({ loader, featureState, highlightedLabelId: 2 }); + + // Identity, not equality: a fresh table with identical bytes would still be a + // multi-megabyte texture upload on every pointer move. + expect(hovered.props.featureColorLut).toBe(initial.props.featureColorLut); + }); + it('builds a lookup table from serializable featureState when none is supplied', () => { const { loader } = makeLabelsLoader(); const featureState = { hiddenFeatureIds: ['3'] }; diff --git a/packages/vis/src/SpatialCanvas/SpatialCanvasViewer.tsx b/packages/vis/src/SpatialCanvas/SpatialCanvasViewer.tsx index 2152ebc9..285f0653 100644 --- a/packages/vis/src/SpatialCanvas/SpatialCanvasViewer.tsx +++ b/packages/vis/src/SpatialCanvas/SpatialCanvasViewer.tsx @@ -5,7 +5,11 @@ import type { DeckGLProps, DeckGLRef, Layer, PickingInfo } from 'deck.gl'; import { type CSSProperties, type ReactNode, useCallback, useEffect, useMemo, useRef } from 'react'; import { ensureCodecWorkers } from '../codecWorkers'; import type { FeatureColorResolver } from './featureColorResolver'; -import { type HoverPointerEvent, isHoverDuringDrag } from './featureTooltipHover'; +import { + type HoverPointerEvent, + isHoverDuringDrag, + resolveHoveredLabel, +} from './featureTooltipHover'; import { ImageLayerContextProvider } from './ImageLayerContext'; import { type RenderStackHostLayerResolver, @@ -533,8 +537,17 @@ function SpatialCanvasViewerInner({ // changing the view, not inspecting features, so suppress tooltip work. if (isHoverDuringDrag(event)) { clearTooltip(); + // Drop the highlight too: the pointer is steering the camera, so a label + // left lit under it would read as a selection the gesture did not make. + renderer.setHoveredLabel(null); return; } + // Cleared unless this hover lands on a label — moving onto empty space, onto a + // different layer kind, or off the canvas all resolve to null. Shared with the + // full-UI `SpatialCanvas`, which has its own `handleHover`. + renderer.setHoveredLabel( + resolveHoveredLabel(info, (layerId) => layerInputs.layers[layerId]?.type === 'labels') + ); if (info.picked && typeof info.x === 'number' && typeof info.y === 'number') { const rawLayerId = typeof info.layer?.id === 'string' ? info.layer.id : ''; const normalizedLayerId = rawLayerId.replace(/-#.*#$/, ''); @@ -567,6 +580,7 @@ function SpatialCanvasViewerInner({ [ clearTooltip, coordinateSystem, + layerInputs.layers, onFeatureHover, onHover, onShapeHover, diff --git a/packages/vis/src/SpatialCanvas/featureTooltipHover.ts b/packages/vis/src/SpatialCanvas/featureTooltipHover.ts index e71d209b..15d86042 100644 --- a/packages/vis/src/SpatialCanvas/featureTooltipHover.ts +++ b/packages/vis/src/SpatialCanvas/featureTooltipHover.ts @@ -1,4 +1,5 @@ import { mergeSpatialFeatureTooltips, type SpatialFeatureTooltipData } from '@spatialdata/core'; +import { parseLabelId } from '@spatialdata/layers'; import type { DeckGLRef, PickingInfo } from 'deck.gl'; const DEFAULT_PICK_RADIUS = 4; @@ -16,6 +17,52 @@ export function isHoverDuringDrag(event?: HoverPointerEvent | null): boolean { return (event?.srcEvent?.buttons ?? 0) !== 0; } +/** The labels layer and label id drawn as hovered, or `null` for nothing. */ +export interface HoveredLabel { + layerId: string; + labelId: number; +} + +/** + * The label under the cursor, from a deck hover pick. + * + * Shared because there are TWO hover implementations — `SpatialCanvasViewer` + * (headless/embedded) and `SpatialCanvas` (full UI) — each with its own + * `handleHover`. The highlight shipped working on the first and dead on the + * second; one function both must call is what stops that recurring. + * + * Deliberately cheap: it reads the id straight off the pick object rather than + * going through `getFeaturePickEvent`, which also builds a tooltip. This runs on + * every pointer move, and the full UI already pays for tooltip resolution + * separately. + * + * `isLabelsLayer` is the guard that the pick belongs to a labels layer at all — + * only `LabelsBitmaskTileLayer` puts a `labelId` on a picked object, but a config + * check costs nothing and keeps a stray object shape from lighting something up. + */ +export function resolveHoveredLabel( + info: { picked?: boolean; layer?: { id?: unknown } | null; object?: unknown }, + isLabelsLayer: (layerId: string) => boolean +): HoveredLabel | null { + if (!info.picked || !info.object || typeof info.object !== 'object') { + return null; + } + const rawLayerId = typeof info.layer?.id === 'string' ? info.layer.id : ''; + if (!rawLayerId) { + return null; + } + const layerId = normalizeDeckLayerId(rawLayerId); + if (!isLabelsLayer(layerId)) { + return null; + } + const rawLabelId = Reflect.get(info.object, 'labelId'); + if (rawLabelId === undefined || rawLabelId === null) { + return null; + } + const labelId = parseLabelId(String(rawLabelId)); + return labelId === undefined ? null : { layerId, labelId }; +} + export interface PickMultipleObjectsCapable { props?: { layers?: unknown; diff --git a/packages/vis/src/SpatialCanvas/index.tsx b/packages/vis/src/SpatialCanvas/index.tsx index 9a6eb01e..49608b3c 100644 --- a/packages/vis/src/SpatialCanvas/index.tsx +++ b/packages/vis/src/SpatialCanvas/index.tsx @@ -21,7 +21,11 @@ import { useState, } from 'react'; import { SpatialCanvasProvider, useSpatialCanvasActions, useSpatialCanvasStore } from './context'; -import { type HoverPointerEvent, isHoverDuringDrag } from './featureTooltipHover'; +import { + type HoverPointerEvent, + isHoverDuringDrag, + resolveHoveredLabel, +} from './featureTooltipHover'; import { ImageChannelPanelFromStore } from './ImageChannelPanel'; import { LabelsChannelPanel } from './LabelsChannelPanel'; import { LayerOrderList } from './LayerOrderList'; @@ -437,6 +441,7 @@ function SpatialCanvasInner({ getWorldBoundsForLayer, getWorldBoundsForVisibleLayers, hasEnabledLayers, + setHoveredLabel, hasLayersDrawn, hasRenderableLayerData, isBlocking, @@ -574,11 +579,18 @@ function SpatialCanvasInner({ // changing the view, not inspecting features, so suppress tooltip work. if (isHoverDuringDrag(event)) { clearTooltip(); + // Drop the highlight too: the pointer is steering the camera, so a label + // left lit under it would read as a selection the gesture did not make. + setHoveredLabel(null); return; } + // Same resolver the headless `SpatialCanvasViewer` uses — this surface has its + // own `handleHover`, and the two drifting is exactly how the highlight came to + // work there and do nothing here. + setHoveredLabel(resolveHoveredLabel(info, (layerId) => layers[layerId]?.type === 'labels')); resolveTooltip(info); }, - [resolveTooltip, clearTooltip] + [resolveTooltip, clearTooltip, setHoveredLabel, layers] ); const handleViewerRef = useCallback( diff --git a/packages/vis/src/SpatialCanvas/renderers/labelsRenderer.ts b/packages/vis/src/SpatialCanvas/renderers/labelsRenderer.ts index 0008311f..8d93a4e5 100644 --- a/packages/vis/src/SpatialCanvas/renderers/labelsRenderer.ts +++ b/packages/vis/src/SpatialCanvas/renderers/labelsRenderer.ts @@ -29,6 +29,14 @@ export interface LabelsLayerRenderConfig { * feature state" — the layer draws every label in the channel colour. */ featureColorLut?: LabelColorLut; + /** + * The label id under the cursor, or `-1` / omitted for none. + * + * Runtime render state rather than layer config — it changes on every pointer + * move and must never reach a saved Render Stack. It becomes a shader uniform, + * so hovering re-uploads neither the LUT nor the tiles. + */ + highlightedLabelId?: number; } export function renderLabelsLayer(config: LabelsLayerRenderConfig): Layer | null { @@ -46,6 +54,7 @@ export function renderLabelsLayer(config: LabelsLayerRenderConfig): Layer | null channelStrokeWidths, selections, featureColorLut, + highlightedLabelId, } = config; if (!visible || !loader) { @@ -66,5 +75,6 @@ export function renderLabelsLayer(config: LabelsLayerRenderConfig): Layer | null channelStrokeWidths, selections, ...(featureColorLut ? { featureColorLut } : {}), + ...(highlightedLabelId !== undefined ? { highlightedLabelId } : {}), }); } diff --git a/packages/vis/src/SpatialCanvas/useLayerData.ts b/packages/vis/src/SpatialCanvas/useLayerData.ts index fea11e8f..e73a66b4 100644 --- a/packages/vis/src/SpatialCanvas/useLayerData.ts +++ b/packages/vis/src/SpatialCanvas/useLayerData.ts @@ -241,6 +241,15 @@ interface UseLayerDataResult { object: ShapeFeatureRenderDatum; } | undefined; + /** + * Set (or clear with `null`) the label drawn as hovered. + * + * Runtime render state, deliberately not part of the Render Stack: it changes on + * every pointer move and would be meaningless in a saved view. It reaches the + * shader as a uniform, so a hover re-uploads neither the colour LUT nor the + * tiles. Passing an unchanged value is free — no re-render is scheduled. + */ + setHoveredLabel: (next: { layerId: string; labelId: number } | null) => void; /** Whether any layers are currently loading */ isLoading: boolean; /** Whether any visible layer is still waiting on its first renderable resource. */ @@ -416,6 +425,29 @@ export function useLayerData( setLoadedDataRevision((revision) => revision + 1); }, []); + // Hover highlight for labels (runtime render state, never config — it changes on + // every pointer move and must not reach a saved Render Stack). + // + // ONE entry, not a per-layer map: only one thing is under the cursor at a time, so + // a single slot makes "moved to a different layer" and "moved off" the same + // transition instead of two bookkeeping cases that can disagree. + // + // A ref plus a version counter rather than plain state: deck fires hover on every + // pointer move, but the highlight only changes when the pointer crosses INTO A + // DIFFERENT LABEL, so the re-render is gated on that transition rather than on + // pointer motion. `getLayers` reads the ref during render. + const hoveredLabelRef = useRef<{ layerId: string; labelId: number } | null>(null); + const [, setHoveredLabelRevision] = useState(0); + + const setHoveredLabel = useCallback((next: { layerId: string; labelId: number } | null) => { + const prev = hoveredLabelRef.current; + if (prev?.layerId === next?.layerId && prev?.labelId === next?.labelId) { + return; + } + hoveredLabelRef.current = next; + setHoveredLabelRevision((revision) => revision + 1); + }, []); + // Build a map of element key -> AvailableElement for quick lookup. Memoised so it // only rebuilds when `availableElements` changes... const elementMapValue = useMemo(() => { @@ -1257,6 +1289,12 @@ export function useLayerData( // Fallbacks mirror `buildLabelsChannelDefaults`: the resolver always // populates these, but its `LabelsChannelDefaults` types them optional. + // Only the layer actually under the cursor highlights; a stale id from a + // layer that is no longer hovered must not tint a second element. + const hoveredLabel = hoveredLabelRef.current; + const highlightedLabelId = + hoveredLabel?.layerId === layerId ? hoveredLabel.labelId : undefined; + const layer = renderLabelsLayer({ id: layerId, loader: labelsData.loader, @@ -1265,6 +1303,7 @@ export function useLayerData( visible: config.visible, channelColors, ...(featureColorLut ? { featureColorLut } : {}), + ...(highlightedLabelId !== undefined ? { highlightedLabelId } : {}), channelsVisible: ch?.channelsVisible && ch.channelsVisible.length > 0 ? ch.channelsVisible @@ -1667,6 +1706,7 @@ export function useLayerData( getFeatureTooltip, getFeaturePickEvent, getShapePickEvent, + setHoveredLabel, isLoading, isBlocking, reloadElement, diff --git a/packages/vis/tests/featureTooltipHover.spec.ts b/packages/vis/tests/featureTooltipHover.spec.ts index 83f271e5..9b953899 100644 --- a/packages/vis/tests/featureTooltipHover.spec.ts +++ b/packages/vis/tests/featureTooltipHover.spec.ts @@ -5,6 +5,7 @@ import { normalizeDeckLayerId, resolveDeckPickLayerIds, resolveHoverFeatureTooltip, + resolveHoveredLabel, } from '../src/SpatialCanvas/featureTooltipHover.js'; describe('isHoverDuringDrag', () => { @@ -425,3 +426,60 @@ describe('featureTooltipHover', () => { expect(getFeatureTooltip).toHaveBeenCalledWith('shapes:cells', expect.any(Object)); }); }); + +describe('resolveHoveredLabel', () => { + const isLabels = (layerId: string) => layerId.startsWith('labels:'); + + it('resolves the layer and label under the cursor', () => { + expect( + resolveHoveredLabel( + { picked: true, layer: { id: 'labels:cells' }, object: { labelId: 42 } }, + isLabels + ) + ).toEqual({ layerId: 'labels:cells', labelId: 42 }); + }); + + it('strips deck’s per-viewport id suffix, so the id matches the layer config key', () => { + expect( + resolveHoveredLabel( + { picked: true, layer: { id: 'labels:cells-#detail#' }, object: { labelId: 7 } }, + isLabels + ) + ).toEqual({ layerId: 'labels:cells', labelId: 7 }); + }); + + it('resolves nothing when the pick is not on a label', () => { + // Nothing under the cursor at all — moving onto empty space or off the canvas. + expect(resolveHoveredLabel({ picked: false, layer: { id: 'labels:cells' } }, isLabels)).toBeNull(); + // A different layer kind: a shapes pick must never light up a labels layer. + expect( + resolveHoveredLabel( + { picked: true, layer: { id: 'shapes:cells' }, object: { featureId: '3' } }, + isLabels + ) + ).toBeNull(); + // Picked, on a labels layer, but the object carries no label id. + expect( + resolveHoveredLabel({ picked: true, layer: { id: 'labels:cells' }, object: {} }, isLabels) + ).toBeNull(); + // Deck can report a pick with no layer. + expect(resolveHoveredLabel({ picked: true, object: { labelId: 1 } }, isLabels)).toBeNull(); + }); + + it('rejects a label id that is not a plain integer', () => { + // Guards the shader comparison: the id reaches it as a float and is matched + // against sampled ids by proximity. + expect( + resolveHoveredLabel( + { picked: true, layer: { id: 'labels:cells' }, object: { labelId: '1e3' } }, + isLabels + ) + ).toBeNull(); + expect( + resolveHoveredLabel( + { picked: true, layer: { id: 'labels:cells' }, object: { labelId: 'abc' } }, + isLabels + ) + ).toBeNull(); + }); +}); diff --git a/packages/vis/tests/useLayerData.spec.tsx b/packages/vis/tests/useLayerData.spec.tsx index 10700e57..6d1aaf29 100644 --- a/packages/vis/tests/useLayerData.spec.tsx +++ b/packages/vis/tests/useLayerData.spec.tsx @@ -1,6 +1,6 @@ import { Matrix4 } from '@math.gl/core'; import type { PointsElement, ShapesElement, SpatialData } from '@spatialdata/core'; -import { renderHook, waitFor } from '@testing-library/react'; +import { act, renderHook, waitFor } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; import type { AvailableElement, ElementsByType, LayerConfig } from '../src/SpatialCanvas/types.js'; import { useLayerData } from '../src/SpatialCanvas/useLayerData.js'; @@ -10,11 +10,11 @@ import { useLayerData } from '../src/SpatialCanvas/useLayerData.js'; * * Until this file, nothing did. Two specs import from the module — one takes a * type, one takes two module-scope helpers — but the 1,873-line hook itself was - * never invoked by any test in the repo. Its entire public surface, seventeen + * never invoked by any test in the repo. Its entire public surface, eighteen * members that reach MDV through a `...layerData` spread, was unguarded. * * That is untenable for the Resource Resolver work, which dissolves six of the - * hook's seven kind-switch ladders and re-points all seventeen members at a + * hook's seven kind-switch ladders and re-points all eighteen members at a * resolver snapshot. This file is the net. It is written against the CURRENT * hook — it must be green before the refactor and stay green through it. * @@ -24,7 +24,7 @@ import { useLayerData } from '../src/SpatialCanvas/useLayerData.js'; * whether the shim is honest. */ -/** The seventeen members MDV consumes. This list IS the compat contract. */ +/** The eighteen members MDV consumes. This list IS the compat contract. */ const PUBLIC_SURFACE = [ 'getLayers', 'getVivLayerProps', @@ -38,6 +38,7 @@ const PUBLIC_SURFACE = [ 'getFeatureTooltip', 'getFeaturePickEvent', 'getShapePickEvent', + 'setHoveredLabel', 'isLoading', 'isBlocking', 'reloadElement', @@ -102,11 +103,11 @@ const shapesConfig = (id: string, elementKey: string): LayerConfig => ({ const render = (layers: Record, elements: ElementsByType) => renderHook(() => useLayerData(layers, Object.keys(layers), elements, null)); -describe('useLayerData — the 17-member public surface', () => { +describe('useLayerData — the 18-member public surface', () => { // ADR 0004 promises MDV that this surface survives the refactor behind a compat // shim. MDV gets it via `...layerData` in SpatialCanvasViewer, so a member that // silently vanishes is a downstream break with no local failure. - it('exposes exactly the seventeen members, and no more', () => { + it('exposes exactly the eighteen members, and no more', () => { const { result } = render({}, EMPTY_ELEMENTS); expect(Object.keys(result.current).sort()).toEqual([...PUBLIC_SURFACE].sort()); @@ -542,3 +543,80 @@ describe('useLayerData — selection show/hide + colour', () => { expect(basePreloadedCodes()?.length).toBe(3); }); }); + +describe('useLayerData — the hover highlight channel', () => { + // Hover is runtime render state, and deck fires it on every pointer move. The + // channel is a ref plus a version counter precisely so that motion WITHIN one + // label is free; only crossing into a different label may re-render. + const renderCounting = () => { + let renders = 0; + const hook = renderHook(() => { + renders += 1; + return useLayerData({}, [], EMPTY_ELEMENTS, null); + }); + return { hook, getRenders: () => renders }; + }; + + it('does not re-render when the hovered label is unchanged', async () => { + const { hook, getRenders } = renderCounting(); + + await act(async () => { + hook.result.current.setHoveredLabel({ layerId: 'labels-1', labelId: 4 }); + }); + const afterFirst = getRenders(); + + // The same label again — the pointer moved, the highlight did not. + await act(async () => { + hook.result.current.setHoveredLabel({ layerId: 'labels-1', labelId: 4 }); + }); + + expect(getRenders()).toBe(afterFirst); + }); + + it('re-renders when the hovered label changes, and when it clears', async () => { + const { hook, getRenders } = renderCounting(); + + await act(async () => { + hook.result.current.setHoveredLabel({ layerId: 'labels-1', labelId: 4 }); + }); + const afterFirst = getRenders(); + + await act(async () => { + hook.result.current.setHoveredLabel({ layerId: 'labels-1', labelId: 5 }); + }); + expect(getRenders()).toBeGreaterThan(afterFirst); + const afterSecond = getRenders(); + + await act(async () => { + hook.result.current.setHoveredLabel(null); + }); + expect(getRenders()).toBeGreaterThan(afterSecond); + }); + + it('treats the same label id on a different layer as a change', async () => { + const { hook, getRenders } = renderCounting(); + + await act(async () => { + hook.result.current.setHoveredLabel({ layerId: 'labels-1', labelId: 4 }); + }); + const afterFirst = getRenders(); + + // Two labels elements can share an id space; only the hovered LAYER highlights. + await act(async () => { + hook.result.current.setHoveredLabel({ layerId: 'labels-2', labelId: 4 }); + }); + + expect(getRenders()).toBeGreaterThan(afterFirst); + }); + + it('clearing when nothing is hovered is free', async () => { + const { hook, getRenders } = renderCounting(); + const baseline = getRenders(); + + await act(async () => { + hook.result.current.setHoveredLabel(null); + }); + + expect(getRenders()).toBe(baseline); + }); +});