From 141ad54fb54b656a2bdb24adb8f9c12ea60d5d7b Mon Sep 17 00:00:00 2001 From: Afonso Pinto Date: Fri, 10 Jul 2026 15:02:22 +0100 Subject: [PATCH 01/11] Merge pull request from MetaCell/feat/update-tool-state fix: correct edit tool state handling --- src/ui/skeleton_edit_tools.ts | 80 ++++++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 16 deletions(-) diff --git a/src/ui/skeleton_edit_tools.ts b/src/ui/skeleton_edit_tools.ts index fb9873380..566693cd4 100644 --- a/src/ui/skeleton_edit_tools.ts +++ b/src/ui/skeleton_edit_tools.ts @@ -73,10 +73,8 @@ import { getSpatialSkeletonDeleteIdleStatusText, getSpatialSkeletonDeletingStatusText, getSpatialSkeletonMergeStatusText, - getSpatialSkeletonMergingStatusText, getSpatialSkeletonMovingStatusText, getSpatialSkeletonSplitIdleStatusText, - getSpatialSkeletonSplittingStatusText, } from "#src/ui/skeleton_edit_tool_messages.js"; import type { ToolActivation } from "#src/ui/tool.js"; import { @@ -691,6 +689,33 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { // --- Mode transitions --- + // Return merge to its "waiting for the first pick" state: no anchor, no + // pinned selection, and the selected-node highlight hidden. Used both when + // entering merge (so a stale anchor from a previous, possibly interrupted + // merge can never carry over) and after a merge completes (so we never + // linger in a limbo state with a stale anchor). Merge mode itself is left + // untouched. + private resetMergeToFreshState() { + this.layer.clearSpatialSkeletonMergeAnchor(); + this.layer.clearSpatialSkeletonNodeSelection("force-unpin"); + this.layer.spatialSkeletonSuppressSelectedNodeHighlight.value = true; + // Drop any transient override so the idle "click a node to merge from" + // prompt shows again — the tool stays held, so it just waits for the next + // pick rather than lingering on a stale status. + this.statusOverride = undefined; + this.renderStatus(); + } + + // Split has no anchor, but like merge it stays active while held; reset the + // selection/highlight and status after a split so it prompts for the next + // node instead of lingering. + private resetSplitToFreshState() { + this.layer.clearSpatialSkeletonNodeSelection("force-unpin"); + this.layer.spatialSkeletonSuppressSelectedNodeHighlight.value = true; + this.statusOverride = undefined; + this.renderStatus(); + } + private enterMerge(anchorNode?: { nodeId: number; segmentId?: number; @@ -703,12 +728,15 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { } this.layer.selectSpatialSkeletonNode(anchorNode.nodeId, true, anchorNode); this.layer.setSpatialSkeletonMergeAnchor(anchorNode.nodeId); + // Entered with an explicit anchor: the first pick has effectively already + // happened, so reveal the selected-node highlight for the from node. + this.layer.spatialSkeletonSuppressSelectedNodeHighlight.value = false; + } else { + // Keyboard flow (hold M): always start from a clean slate. Clearing here + // guarantees merge activation can never begin with an old anchor set — + // e.g. after holding M through a completed merge without releasing. + this.resetMergeToFreshState(); } - // In merge mode the selected-node highlight is only shown once a from node - // has been picked (the first click). When entered with an explicit anchor, - // that first pick has effectively already happened. - this.layer.spatialSkeletonSuppressSelectedNodeHighlight.value = - anchorNode === undefined; this.layer.spatialSkeletonMergeMode.value = true; this.currentMode = SkeletonEditMode.Merge; this.updateModeAttribute(); @@ -926,7 +954,6 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { // A node was clicked: reveal the selected-node highlight for it. this.layer.spatialSkeletonSuppressSelectedNodeHighlight.value = false; this.pending = true; - this.setStatus(getSpatialSkeletonSplittingStatusText()); void (async () => { try { await executeSpatialSkeletonSplit(this.layer, { @@ -937,7 +964,9 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { showSpatialSkeletonActionError("split skeleton", error); } finally { this.pending = false; - this.renderStatus(); + // Reset to a fresh split so it prompts for the next node (the user may + // still be holding s) rather than lingering on a "splitting…" status. + this.resetSplitToFreshState(); } })(); } @@ -1035,12 +1064,28 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { const pickedNode = this.resolvePickedNodeSelectionForMerge(skeletonLayer); if (pickedNode === undefined || pickedNode.segmentId === undefined) return; - if ( - pickedNode.nodeId === anchorNodeId || - pickedNode.segmentId === firstNode.segmentId - ) { + if (pickedNode.nodeId === anchorNodeId) { + // Clicked the anchor node again — nothing to do. + return; + } + if (pickedNode.segmentId === firstNode.segmentId) { + // The second pick is on the SAME skeleton as the anchor. Rather than + // blocking the edit and leaving the old anchor armed, treat this as the + // user re-choosing the "from" node: move the merge anchor here and stay + // in merge mode so they can now pick a node on a different skeleton. + if (!this.isSpatialSkeletonSegmentVisible(pickedNode.segmentId)) { + StatusMessage.showTemporaryMessage( + `Make skeleton ${pickedNode.segmentId} visible before merging.`, + ); + return; + } + this.pinSegmentByNumber(pickedNode.segmentId); + this.layer.selectSpatialSkeletonNode(pickedNode.nodeId, true, pickedNode); + this.layer.setSpatialSkeletonMergeAnchor(pickedNode.nodeId); + this.layer.spatialSkeletonSuppressSelectedNodeHighlight.value = false; + this.renderStatus(); StatusMessage.showTemporaryMessage( - "Select a node from a different skeleton to merge with.", + "Moved the merge start here — now select a node on a different skeleton.", ); return; } @@ -1062,7 +1107,6 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { this.pinSegmentByNumber(pickedNode.segmentId); this.layer.selectSpatialSkeletonNode(pickedNode.nodeId, true, pickedNode); this.pending = true; - this.setStatus(getSpatialSkeletonMergingStatusText()); void (async () => { try { @@ -1086,7 +1130,11 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { showSpatialSkeletonActionError("merge skeletons", error); } finally { this.pending = false; - this.renderStatus(); // Keep merge mode — user may still be holding m. + // Keep merge mode active (the user may still be holding M), but clear + // the anchor + pinned selection and hide the highlight so we never + // linger in a limbo state with a stale anchor. The next click starts a + // fresh merge. Applies on both success and error. + this.resetMergeToFreshState(); } })(); } From bc1c0b37ccf55d890e0fd75188730472d4f7c9bb Mon Sep 17 00:00:00 2001 From: Afonso Pinto Date: Fri, 10 Jul 2026 16:40:15 +0100 Subject: [PATCH 02/11] Merge pull request from MetaCell/refactor/remove-unused-state feat: Correct node merge mode highlights --- src/ui/skeleton_edit_tools.ts | 92 ++++++++--------------------------- 1 file changed, 21 insertions(+), 71 deletions(-) diff --git a/src/ui/skeleton_edit_tools.ts b/src/ui/skeleton_edit_tools.ts index 566693cd4..f381665b9 100644 --- a/src/ui/skeleton_edit_tools.ts +++ b/src/ui/skeleton_edit_tools.ts @@ -293,44 +293,6 @@ abstract class SpatialSkeletonToolBase extends LayerTool }; } - protected getSelectedSpatialSkeletonNodeForTool( - skeletonLayer: SpatiallyIndexedSkeletonLayer | undefined, - ): - | { - nodeId: number; - segmentId?: number; - position?: SpatialSkeletonVector; - sourceState?: SpatialSkeletonSourceState; - } - | undefined { - const nodeId = this.layer.selectedSpatialSkeletonNodeInfo.value?.nodeId; - if ( - typeof nodeId !== "number" || - !Number.isSafeInteger(nodeId) || - nodeId <= 0 - ) { - return undefined; - } - const resolvedNodeInfo = - skeletonLayer?.getNode(nodeId) ?? - this.layer.spatialSkeletonState.getCachedNode(nodeId); - const selectedNodeInfo = this.layer.selectedSpatialSkeletonNodeInfo.value; - const layerSelectionState = - this.layer.manager.root.selectionState.value?.layers.find( - (entry) => entry.layer === this.layer, - )?.state; - return { - nodeId, - segmentId: - resolvedNodeInfo?.segmentId ?? - selectedNodeInfo?.segmentId ?? - getSegmentIdFromLayerSelectionValue(layerSelectionState), - position: resolvedNodeInfo?.position ?? selectedNodeInfo?.position, - sourceState: - resolvedNodeInfo?.sourceState ?? selectedNodeInfo?.sourceState, - }; - } - protected getSelectedSpatialSkeletonNodeSummary() { const nodeId = this.layer.selectedSpatialSkeletonNodeInfo.value?.nodeId; if (nodeId === undefined) { @@ -689,15 +651,13 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { // --- Mode transitions --- - // Return merge to its "waiting for the first pick" state: no anchor, no - // pinned selection, and the selected-node highlight hidden. Used both when - // entering merge (so a stale anchor from a previous, possibly interrupted - // merge can never carry over) and after a merge completes (so we never - // linger in a limbo state with a stale anchor). Merge mode itself is left - // untouched. + // Return merge to its "waiting for the first pick" state: no active anchor + // and the selected-node highlight hidden. Used both when entering merge and + // after a merge completes. The node selection itself is preserved (only its + // highlight is suppressed) — merge never clears the selection, it only hides + // it. Merge mode itself is left untouched. private resetMergeToFreshState() { this.layer.clearSpatialSkeletonMergeAnchor(); - this.layer.clearSpatialSkeletonNodeSelection("force-unpin"); this.layer.spatialSkeletonSuppressSelectedNodeHighlight.value = true; // Drop any transient override so the idle "click a node to merge from" // prompt shows again — the tool stays held, so it just waits for the next @@ -716,27 +676,13 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { this.renderStatus(); } - private enterMerge(anchorNode?: { - nodeId: number; - segmentId?: number; - position?: SpatialSkeletonVector; - sourceState?: SpatialSkeletonSourceState; - }) { - if (anchorNode !== undefined) { - if (anchorNode.segmentId !== undefined) { - this.pinSegmentByNumber(anchorNode.segmentId); - } - this.layer.selectSpatialSkeletonNode(anchorNode.nodeId, true, anchorNode); - this.layer.setSpatialSkeletonMergeAnchor(anchorNode.nodeId); - // Entered with an explicit anchor: the first pick has effectively already - // happened, so reveal the selected-node highlight for the from node. - this.layer.spatialSkeletonSuppressSelectedNodeHighlight.value = false; - } else { - // Keyboard flow (hold M): always start from a clean slate. Clearing here - // guarantees merge activation can never begin with an old anchor set — - // e.g. after holding M through a completed merge without releasing. - this.resetMergeToFreshState(); - } + private enterMerge() { + // Merge always starts without an active anchor — it can never begin with a + // pre-set anchor. The anchor is set solely by the first in-mode pick + // (handleMergeFirstPick), which also sets the selected node. Entering merge + // preserves the current node selection and only hides its highlight, so the + // selection reappears if the user exits merge without picking. + this.resetMergeToFreshState(); this.layer.spatialSkeletonMergeMode.value = true; this.currentMode = SkeletonEditMode.Merge; this.updateModeAttribute(); @@ -1130,11 +1076,15 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { showSpatialSkeletonActionError("merge skeletons", error); } finally { this.pending = false; - // Keep merge mode active (the user may still be holding M), but clear - // the anchor + pinned selection and hide the highlight so we never - // linger in a limbo state with a stale anchor. The next click starts a - // fresh merge. Applies on both success and error. - this.resetMergeToFreshState(); + // If the user released M while the merge was in flight, exitMerge has + // already left merge mode and restored the highlight — do not re-hide + // it here (that would leave the selection permanently hidden). Only + // reset when still in merge mode: clear the anchor and re-hide the + // highlight (the selection is kept) so the next click starts a fresh + // merge. Applies on both success and error. + if (this.currentMode === SkeletonEditMode.Merge) { + this.resetMergeToFreshState(); + } } })(); } From 10458063d329fff2d9831d079ca36f5c93b5ce50 Mon Sep 17 00:00:00 2001 From: Afonso Pinto Date: Fri, 10 Jul 2026 16:41:40 +0100 Subject: [PATCH 03/11] Merge pull request from MetaCell/fix/node-scroll feat: scroll node to center in skeleton details --- src/ui/skeleton_tab.ts | 80 +++++++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/src/ui/skeleton_tab.ts b/src/ui/skeleton_tab.ts index 9c0c26c56..c12e27ac6 100644 --- a/src/ui/skeleton_tab.ts +++ b/src/ui/skeleton_tab.ts @@ -375,7 +375,7 @@ export class SpatialSkeletonEditTab extends Tab { let nodeRerootAllowed = false; let pendingScrollToSelectedNode = false; const MAX_SCROLL_RETRY_FRAMES = 6; - const SCROLL_IN_VIEW_EPSILON = 1; + const SCROLL_CENTER_EPSILON = 2; let scrollRetryHandle: number | undefined; let scrollRetriesRemaining = 0; let scrollRetryNodeId: number | undefined; @@ -612,27 +612,46 @@ export class SpatialSkeletonEditTab extends Tab { } }; - // True when the row is fully visible below the sticky header (or is simply - // taller than the available viewport, in which case aligning its top is the - // best we can do). - const isRowFullyInView = (element: HTMLElement) => { + // The area below the sticky header in which rows are actually visible. + const getRowViewport = () => { const listRect = nodesList.element.getBoundingClientRect(); - const viewportTop = listRect.top + nodesList.header.offsetHeight; - const viewportBottom = listRect.bottom; + const top = listRect.top + nodesList.header.offsetHeight; + return { top, bottom: listRect.bottom, height: listRect.bottom - top }; + }; + + // The `anchorClientOffset` (distance from the top of the list element to the + // top of the anchored row) that vertically centers a row of the given height + // in the viewport below the sticky header. Rows taller than the viewport are + // clamped to align their top with the header. + const getCenteringAnchorClientOffset = (rowHeight: number) => { + const headerHeight = nodesList.header.offsetHeight; + const availableHeight = getRowViewport().height; + return headerHeight + Math.max(0, (availableHeight - rowHeight) / 2); + }; + + // True when the row is vertically centered in the viewport, within epsilon. + // Rows taller than the viewport count as centered once their top reaches the + // header, since they cannot be centered any better. + const isRowCentered = (element: HTMLElement) => { + const viewport = getRowViewport(); const rowRect = element.getBoundingClientRect(); - const topVisible = rowRect.top >= viewportTop - SCROLL_IN_VIEW_EPSILON; - const bottomVisible = - rowRect.bottom <= viewportBottom + SCROLL_IN_VIEW_EPSILON; - const tallerThanViewport = rowRect.height > viewportBottom - viewportTop; - return topVisible && (bottomVisible || tallerThanViewport); + if (rowRect.height >= viewport.height) { + return Math.abs(rowRect.top - viewport.top) <= SCROLL_CENTER_EPSILON; + } + const rowCenter = rowRect.top + rowRect.height / 2; + const viewportCenter = viewport.top + viewport.height / 2; + return Math.abs(rowCenter - viewportCenter) <= SCROLL_CENTER_EPSILON; }; - // Reveal the currently selected node's row in the virtual list. The virtual - // list renders asynchronously (animation-frame debounced) and positions - // unrendered rows using size *estimates*, so a single synchronous attempt is - // unreliable. We keep `pendingScrollToSelectedNode` set until the target row - // is genuinely rendered and fully in view, correcting the scroll position - // against the real measured geometry across a bounded number of frames. + // Center the currently selected node's row in the virtual list. Aligning it + // to the middle (rather than merely scrolling it barely into view) keeps it + // clear of UI overlays anchored to the bottom of the list, which could + // otherwise obscure a row revealed at the very bottom. The virtual list + // renders asynchronously (animation-frame debounced) and positions unrendered + // rows using size *estimates*, so a single synchronous attempt is unreliable. + // We keep `pendingScrollToSelectedNode` set until the target row is genuinely + // rendered and centered, correcting the scroll position against the real + // measured geometry across a bounded number of frames. const attemptScrollToSelectedNode = () => { scrollRetryHandle = undefined; const selectedNodeId = @@ -654,34 +673,31 @@ export class SpatialSkeletonEditTab extends Tab { } const renderedElement = nodesList.getItemElement(index); - if (renderedElement !== undefined && isRowFullyInView(renderedElement)) { + if (renderedElement !== undefined && isRowCentered(renderedElement)) { pendingScrollToSelectedNode = false; return; } if (scrollRetriesRemaining <= 0) { - // Found and rendered but still won't fit after several corrections; stop + // Found and rendered but still not centered after several corrections + // (e.g. the row is near a list edge and cannot be centered further); stop // retrying so `updateList` doesn't loop forever. pendingScrollToSelectedNode = false; return; } scrollRetriesRemaining--; - const headerHeight = nodesList.header.offsetHeight; nodesList.state.anchorIndex = index; if (renderedElement === undefined) { - // Not rendered: anchor its top just below the sticky header and let the - // next frame render + measure it. - nodesList.state.anchorClientOffset = headerHeight; + // Not rendered: the row's real height is unknown, so anchor its top at the + // viewport center and let the next frame render + measure it before + // correcting to a true center. + nodesList.state.anchorClientOffset = getCenteringAnchorClientOffset(0); } else { - // Rendered but out of view: correct using the real measured rect. - const listRect = nodesList.element.getBoundingClientRect(); + // Rendered but off-center: center it using the real measured height. const rowRect = renderedElement.getBoundingClientRect(); - const relTop = rowRect.top - listRect.top; - if (relTop < headerHeight) { - nodesList.state.anchorClientOffset = headerHeight; - } else { - nodesList.state.anchorClientOffset = listRect.height - rowRect.height; - } + nodesList.state.anchorClientOffset = getCenteringAnchorClientOffset( + rowRect.height, + ); } // Drives VirtualList's own debouncedUpdateView; its rAF is registered // before ours below, so it runs first and our next attempt measures the From 4b5c2df21320d8f75d8eb873900d5d7c96908fc2 Mon Sep 17 00:00:00 2001 From: Afonso Pinto Date: Tue, 14 Jul 2026 16:15:44 +0100 Subject: [PATCH 04/11] Merge pull request from MetaCell/feat/skeleton-cylinder-rendering feat: cylinder and ball rendering of skeletons --- python/tests/skeleton_options_test.py | 15 +- src/perspective_view/panel.ts | 25 +- src/skeleton/frontend.ts | 577 +++++++++++++++----------- src/skeleton/skeleton_shader_color.ts | 141 +++++++ src/webgl/raycast_cylinder.ts | 148 +++++++ src/webgl/raycast_primitive.ts | 146 +++++++ src/webgl/raycast_sphere.ts | 73 ++++ 7 files changed, 875 insertions(+), 250 deletions(-) create mode 100644 src/skeleton/skeleton_shader_color.ts create mode 100644 src/webgl/raycast_cylinder.ts create mode 100644 src/webgl/raycast_primitive.ts create mode 100644 src/webgl/raycast_sphere.ts diff --git a/python/tests/skeleton_options_test.py b/python/tests/skeleton_options_test.py index 2256af4eb..e206af973 100644 --- a/python/tests/skeleton_options_test.py +++ b/python/tests/skeleton_options_test.py @@ -63,10 +63,17 @@ def test_skeleton_options(webdriver): s.layout = "3d" s.layers[0].skeleton_rendering.line_width3d = 100 screenshot = webdriver.viewer.screenshot(size=[10, 10]).screenshot - np.testing.assert_array_equal( - screenshot.image_pixels, - np.tile(np.array([255, 0, 0, 255], dtype=np.uint8), (10, 10, 1)), - ) + # In the perspective view the skeleton is now rendered with lit + # cylinder/sphere impostors rather than flat billboards, so the red channel + # is modulated by the lighting factor instead of being a uniform 255. The + # shader emits only the red channel, so green/blue remain 0 and the + # composited alpha remains 255; some red must be present (the skeleton + # renders). + pixels = screenshot.image_pixels + np.testing.assert_array_equal(pixels[..., 1], 0) # green + np.testing.assert_array_equal(pixels[..., 2], 0) # blue + np.testing.assert_array_equal(pixels[..., 3], 255) # alpha + assert pixels[..., 0].max() > 0 # red skeleton rendered with webdriver.viewer.txn() as s: s.layers[0].source[0].subsources["default"] = False diff --git a/src/perspective_view/panel.ts b/src/perspective_view/panel.ts index 11d073d5c..6b3802dcc 100644 --- a/src/perspective_view/panel.ts +++ b/src/perspective_view/panel.ts @@ -111,15 +111,31 @@ enum TransparentRenderingState { MAX_PROJECTION = 2, } -export const glsl_perspectivePanelEmit = ` +// Fragment depth used by `emit` for the Z buffer / OIT weight. Defaults to a +// sentinel (< 0) meaning "use gl_FragCoord.z". Raycast render layers (which +// draw an analytic surface on a flat quad and write gl_FragDepth themselves) +// set `emitDepthOverride` to the true surface depth so the Z texture and OIT +// weight are consistent with the written gl_FragDepth rather than the flat quad +// depth. +export const glsl_perspectivePanelEmitDepth = ` +highp float emitDepthOverride = -1.0; +highp float getEmitDepth() { + return emitDepthOverride < 0.0 ? gl_FragCoord.z : emitDepthOverride; +} +`; + +export const glsl_perspectivePanelEmit = [ + glsl_perspectivePanelEmitDepth, + ` void emit(vec4 color, highp uint pickId) { out_color = color; - float zValue = 1.0 - gl_FragCoord.z; + float zValue = 1.0 - getEmitDepth(); out_z = vec4(zValue, zValue, zValue, 1.0); float pickIdFloat = float(pickId); out_pickId = vec4(pickIdFloat, pickIdFloat, pickIdFloat, 1.0); } -`; +`, +]; /** * http://jcgt.org/published/0002/02/09/paper.pdf @@ -137,13 +153,14 @@ float computeOITWeight(float alpha, float depth) { // Can use emitAccumAndRevealage() to emit a pre-weighted OIT result. export const glsl_perspectivePanelEmitOIT = [ glsl_computeOITWeight, + glsl_perspectivePanelEmitDepth, ` void emitAccumAndRevealage(vec4 accum, float revealage, highp uint pickId) { v4f_fragData0 = vec4(accum.rgb, revealage); v4f_fragData1 = vec4(accum.a, 0.0, 0.0, 0.0); } void emit(vec4 color, highp uint pickId) { - float weight = computeOITWeight(color.a, gl_FragCoord.z); + float weight = computeOITWeight(color.a, getEmitDepth()); vec4 accum = color * weight; emitAccumAndRevealage(accum, color.a, pickId); } diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index 0fcb81959..fbfbdef78 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -95,6 +95,12 @@ import { mergeSpatiallyIndexedSkeletonOverlaySegmentIds, retainSpatiallyIndexedSkeletonOverlaySegment, } from "#src/skeleton/segment_overlay.js"; +import type { EdgeShadingGlsl } from "#src/skeleton/skeleton_shader_color.js"; +import { + edgeColorPathsGlsl, + raycastFragmentSetup, + nodeColorPathsGlsl, +} from "#src/skeleton/skeleton_shader_color.js"; import type { SpatiallyIndexedSkeletonView } from "#src/skeleton/source_selection.js"; import { type SliceViewSourceOptions, @@ -168,6 +174,9 @@ import { drawLines, initializeLineShader, } from "#src/webgl/lines.js"; +import { drawQuads } from "#src/webgl/quad.js"; +import { defineRaycastCylinderShader } from "#src/webgl/raycast_cylinder.js"; +import { defineRaycastSphereShader } from "#src/webgl/raycast_sphere.js"; import type { ShaderModule, ShaderProgram, @@ -241,6 +250,11 @@ const SELECTED_NODE_HIGHLIGHT_COLORS: readonly vec3[] = [ // Used for debugging chunks via a different color for each chunk const tempChunkKeyToColorMap = new Map(); const tempMat4 = mat4.create(); +// Scratch matrices/vectors for raycast uniform computation in beginLayer. +const tempInvProjection = mat4.create(); +const tempInvModel = mat4.create(); +const tempNormalTransform = mat4.create(); +const tempLightVec = new Float32Array(4); interface VertexAttributeRenderInfo extends VertexAttributeInfo { name: string; @@ -275,7 +289,6 @@ interface SkeletonShaderContext { fallbackShaderParameters: WatchableValue; displayState: SkeletonLayerDisplayState; skeletonShaderParameters: WatchableValueInterface; - segmentColorAttributeIndex?: number; } interface SkeletonGPUGeometry { @@ -318,13 +331,26 @@ type SpatiallyIndexedSkeletonPickData = chunk: SpatiallyIndexedSkeletonChunk; }; +interface EdgeGeometry { + vertexMain: string; + fragmentSetup: string; + shading: EdgeShadingGlsl; +} + +interface NodeGeometry { + vertexMain: string; + fragmentSetup: string; + // Whether the legacy path premultiplies rgb by alpha before emitting (raycast + // does; the billboard preserves its original un-premultiplied behavior). + legacyPremultiply: boolean; +} + class RenderHelper extends RefCounted { private textureAccessHelper = new OneDimensionalTextureAccessHelper( "vertexData", ); private vertexIdHelper; private segmentAttributeIndex: number | undefined; - private segmentColorAttributeIndex: number | undefined; private visibleSegmentsShaderManager = new HashSetShaderManager( "visibleSegments", ); @@ -365,16 +391,34 @@ class RenderHelper extends RefCounted { if (skeletonParams.dynamicSegmentAppearance) { this.defineDynamicSegmentAppearance(builder, skeletonParams); } + // Perspective (3D) views render cylinders/spheres as raycasts; slice (2D) + // views keep the screen-space line/circle billboards. + const raycast = !this.targetIsSliceView; + if (raycast) { + builder.addUniform("highp mat4", "uInvProjection"); + builder.addUniform("highp mat4", "uNormalTransform"); + builder.addUniform("highp vec4", "uLightDirection"); + builder.addUniform("highp vec2", "uViewportSize"); + // Set per-fragment by the raycast setup; the emit bodies multiply the + // color by it. + builder.addFragmentCode("highp float raycastLightingFactor = 1.0;\n"); + } if (skeletonParams.spatialChunkCulling) { builder.addUniform("highp vec3", "uChunkOrigin"); builder.addUniform("highp vec3", "uChunkBound"); - builder.addVarying("highp vec3", "vCullPos"); builder.addFragmentCode(` -void spatialChunkCull() { - if (any(lessThan(vCullPos, uChunkOrigin)) || - any(greaterThanEqual(vCullPos, uChunkBound))) discard; +void spatialChunkCull(highp vec3 cullPos) { + if (any(lessThan(cullPos, uChunkOrigin)) || + any(greaterThanEqual(cullPos, uChunkBound))) discard; } `); + if (!raycast) { + // Billboard path culls using the interpolated per-fragment position. + builder.addVarying("highp vec3", "vCullPos"); + builder.addFragmentCode(` +void spatialChunkCull() { spatialChunkCull(vCullPos); } +`); + } } } @@ -389,6 +433,7 @@ void spatialChunkCull() { shaderBuilderState: ShaderControlsBuilderState, skeletonParams: SkeletonShaderParameters, vertexMain: string, + fragmentSetup = "", ): void { builder.addFragmentCode(glsl_COLORMAPS); const { vertexAttributes } = this; @@ -425,19 +470,9 @@ void spatialChunkCull() { shaderCodeWithLineDirective(shaderBuilderState.parseResult.code) + "\n#undef main\n", ); - builder.setFragmentMain( - skeletonParams.spatialChunkCulling - ? "spatialChunkCull();\nuserMain();" - : "userMain();", - ); - } - - private getSegmentColorExpression() { - const index = this.segmentColorAttributeIndex; - if (index === undefined) { - return "uColor"; - } - return `vCustom${index}`; + // `fragmentSetup` runs the raycast intersection (writing gl_FragDepth / + // lighting) or the billboard chunk cull before the user's fragment main. + builder.setFragmentMain(fragmentSetup + "userMain();"); } edgeShaderGetter; @@ -633,7 +668,6 @@ vec4 getSegmentAppearance(highp uint segmentValue) { ); this.segmentAttributeIndex = segmentAttrIndex >= 0 ? segmentAttrIndex : undefined; - this.segmentColorAttributeIndex = base.segmentColorAttributeIndex; const segmentationGroupState = base.displayState.segmentationGroupState.value; @@ -672,100 +706,8 @@ vec4 getSegmentAppearance(highp uint segmentValue) { .builderState, extraParameters: this.base.skeletonShaderParameters, shaderError: this.base.displayState.shaderError, - defineShader: ( - builder: ShaderBuilder, - shaderBuilderState: ShaderControlsBuilderState, - skeletonParams: SkeletonShaderParameters, - ) => { - this.defineCommonShader(builder, shaderBuilderState, skeletonParams); - defineLineShader(builder); - builder.addAttribute("highp uvec2", "aVertexIndex"); - builder.addUniform("highp float", "uLineWidth"); - let vertexMain = ` -highp uint pickOffset = uint(gl_InstanceID) * uPickInstanceStride; -vPickID = uPickID + pickOffset; -highp vec3 vertexA = readAttribute0(aVertexIndex.x); -highp vec3 vertexB = readAttribute0(aVertexIndex.y); -emitLine(uProjection, vertexA, vertexB, uLineWidth); -highp uint lineEndpointIndex = getLineEndpointIndex(); -highp uint vertexIndex = aVertexIndex.x * (1u - lineEndpointIndex) + aVertexIndex.y * lineEndpointIndex; -`; - if (skeletonParams.spatialChunkCulling) { - vertexMain += `vCullPos = mix(vertexA, vertexB, float(lineEndpointIndex));\n`; - } - if ( - skeletonParams.dynamicSegmentAppearance && - this.segmentAttributeIndex !== undefined - ) { - vertexMain += `vSegmentValue = toRaw(readAttribute${this.segmentAttributeIndex}(aVertexIndex.x));\n`; - } - - const segmentColorExpression = this.getSegmentColorExpression(); - const segmentAlphaExpression = - this.segmentColorAttributeIndex === undefined - ? "uColor.a" - : `${segmentColorExpression}.a`; - if (skeletonParams.dynamicSegmentAppearance) { - // Dynamic path (spatial skeletons): per-segment color, visibility, - // saturation and hover highlight all resolved in the shader via - // getSegmentAppearance(). uColor is unused in this path. - builder.addFragmentCode(` -vec4 segmentColor() { - return getSegmentAppearance(vSegmentValue); -} -void emitRGB(vec3 color) { - vec4 baseColor = segmentColor(); - highp float alpha = baseColor.a * getLineAlpha() * ${this.getCrossSectionFadeFactor()}; - if (alpha <= 0.0) discard; - emit(vec4(color * alpha, alpha), vPickID); -} -void emitDefault() { - vec4 baseColor = segmentColor(); - highp float alpha = baseColor.a * getLineAlpha() * ${this.getCrossSectionFadeFactor()}; - if (alpha <= 0.0) discard; - emit(vec4(baseColor.rgb * alpha, alpha), vPickID); -} -`); - } else if (this.segmentColorAttributeIndex === undefined) { - // Legacy path (non-spatial skeletons): one skeleton drawn per call; - // uColor is set per-skeleton by the CPU via getObjectColor(), which - // already incorporates saturation and hover highlighting. - builder.addFragmentCode(` -vec4 segmentColor() { - return ${segmentColorExpression}; -} -void emitRGB(vec3 color) { - emit(vec4(color * uColor.a, uColor.a * getLineAlpha() * ${this.getCrossSectionFadeFactor()}), vPickID); -} -void emitDefault() { - emit(vec4(uColor.rgb, uColor.a * getLineAlpha() * ${this.getCrossSectionFadeFactor()}), vPickID); -} -`); - } else { - // Per-vertex color attribute path: color comes from a per-vertex - // attribute; alpha is taken from uColor. - builder.addFragmentCode(` -vec4 segmentColor() { - return ${segmentColorExpression}; -} -void emitRGB(vec3 color) { - highp float alpha = ${segmentAlphaExpression} * getLineAlpha() * ${this.getCrossSectionFadeFactor()}; - emit(vec4(color * alpha, alpha), vPickID); -} -void emitDefault() { - vec4 baseColor = segmentColor(); - highp float alpha = baseColor.a * getLineAlpha() * ${this.getCrossSectionFadeFactor()}; - emit(vec4(baseColor.rgb * alpha, alpha), vPickID); -} -`); - } - this.finalizeShaderBuilder( - builder, - shaderBuilderState, - skeletonParams, - vertexMain, - ); - }, + defineShader: (builder, shaderBuilderState, skeletonParams) => + this.defineEdgeShader(builder, shaderBuilderState, skeletonParams), }, ); @@ -783,112 +725,201 @@ void emitDefault() { .builderState, extraParameters: this.base.skeletonShaderParameters, shaderError: this.base.displayState.shaderError, - defineShader: ( - builder: ShaderBuilder, - shaderBuilderState: ShaderControlsBuilderState, - skeletonParams: SkeletonShaderParameters, - ) => { - this.defineCommonShader(builder, shaderBuilderState, skeletonParams); - defineCircleShader( - builder, - /*crossSectionFade=*/ this.targetIsSliceView, - ); - builder.addUniform("highp float", "uNodeDiameter"); - let vertexMain = ` -highp uint vertexIndex = uint(gl_InstanceID); -highp uint pickOffset = vertexIndex * uPickInstanceStride; + defineShader: (builder, shaderBuilderState, skeletonParams) => + this.defineNodeShader(builder, shaderBuilderState, skeletonParams), + }, + ); + } + + private dynamicColorPath(skeletonParams: SkeletonShaderParameters): boolean { + return ( + skeletonParams.dynamicSegmentAppearance && + this.segmentAttributeIndex !== undefined + ); + } + + // Vertex-shader assignment of `vSegmentValue`, read by the dynamic color path. + private readSegmentValueGlsl( + skeletonParams: SkeletonShaderParameters, + indexExpression: string, + ): string { + if (!this.dynamicColorPath(skeletonParams)) return ""; + return `vSegmentValue = toRaw(readAttribute${this.segmentAttributeIndex}(${indexExpression}));\n`; + } + + private defineEdgeShader( + builder: ShaderBuilder, + shaderBuilderState: ShaderControlsBuilderState, + skeletonParams: SkeletonShaderParameters, + ) { + this.defineCommonShader(builder, shaderBuilderState, skeletonParams); + const geometry = this.targetIsSliceView + ? this.defineEdgeLineBillboard(builder, skeletonParams) + : this.defineEdgeRaycastCylinder(builder, skeletonParams); + const path = this.dynamicColorPath(skeletonParams) ? "dynamic" : "legacy"; + builder.addFragmentCode(edgeColorPathsGlsl(path, geometry.shading)); + this.finalizeShaderBuilder( + builder, + shaderBuilderState, + skeletonParams, + geometry.vertexMain, + geometry.fragmentSetup, + ); + } + + private defineNodeShader( + builder: ShaderBuilder, + shaderBuilderState: ShaderControlsBuilderState, + skeletonParams: SkeletonShaderParameters, + ) { + this.defineCommonShader(builder, shaderBuilderState, skeletonParams); + const geometry = this.targetIsSliceView + ? this.defineNodeCircleBillboard(builder, skeletonParams) + : this.defineNodeRaycastSphere(builder, skeletonParams); + const path = this.dynamicColorPath(skeletonParams) ? "dynamic" : "legacy"; + builder.addFragmentCode( + nodeColorPathsGlsl(path, geometry.legacyPremultiply), + ); + this.finalizeShaderBuilder( + builder, + shaderBuilderState, + skeletonParams, + geometry.vertexMain, + geometry.fragmentSetup, + ); + } + + // Slice view: screen-space anti-aliased line billboard (constant pixel width). + private defineEdgeLineBillboard( + builder: ShaderBuilder, + skeletonParams: SkeletonShaderParameters, + ): EdgeGeometry { + defineLineShader(builder); + builder.addAttribute("highp uvec2", "aVertexIndex"); + builder.addUniform("highp float", "uLineWidth"); + let vertexMain = ` +highp uint pickOffset = uint(gl_InstanceID) * uPickInstanceStride; vPickID = uPickID + pickOffset; -highp vec3 vertexPosition = readAttribute0(vertexIndex); +highp vec3 vertexA = readAttribute0(aVertexIndex.x); +highp vec3 vertexB = readAttribute0(aVertexIndex.y); +emitLine(uProjection, vertexA, vertexB, uLineWidth); +highp uint lineEndpointIndex = getLineEndpointIndex(); +highp uint vertexIndex = aVertexIndex.x * (1u - lineEndpointIndex) + aVertexIndex.y * lineEndpointIndex; `; - if (skeletonParams.spatialChunkCulling) { - vertexMain += `vCullPos = vertexPosition;\n`; - } - if ( - skeletonParams.dynamicSegmentAppearance && - this.segmentAttributeIndex !== undefined - ) { - vertexMain += `vSegmentValue = toRaw(readAttribute${this.segmentAttributeIndex}(vertexIndex));\n`; - } - vertexMain += ` -emitCircle(uProjection * vec4(vertexPosition, 1.0), uNodeDiameter, 0.0); + let fragmentSetup = ""; + if (skeletonParams.spatialChunkCulling) { + vertexMain += `vCullPos = mix(vertexA, vertexB, float(lineEndpointIndex));\n`; + fragmentSetup = `spatialChunkCull();\n`; + } + vertexMain += this.readSegmentValueGlsl(skeletonParams, "aVertexIndex.x"); + return { + vertexMain, + fragmentSetup, + shading: { + coverageAlpha: ` * getLineAlpha() * ${this.getCrossSectionFadeFactor()}`, + shadeColor: "", + legacyDefaultPremultiply: "", + }, + }; + } + + // Perspective view: raycast cylinder (2 triangles). Each end is + // clipped by the node radius so it does not overlap the node sphere (which + // would double-blend under order-independent transparency). + private defineEdgeRaycastCylinder( + builder: ShaderBuilder, + skeletonParams: SkeletonShaderParameters, + ): EdgeGeometry { + defineRaycastCylinderShader(builder, { capped: false }); + builder.addAttribute("highp uvec2", "aVertexIndex"); + builder.addUniform("highp float", "uEdgePixelRadius"); + builder.addUniform("highp float", "uNodePixelRadius"); + let vertexMain = ` +highp uint pickOffset = uint(gl_InstanceID) * uPickInstanceStride; +vPickID = uPickID + pickOffset; +highp vec3 vertexA = readAttribute0(aVertexIndex.x); +highp vec3 vertexB = readAttribute0(aVertexIndex.y); +highp uint vertexIndex = aVertexIndex.x; +highp vec3 edgeMidpoint = mix(vertexA, vertexB, 0.5); +highp float edgeRadius = getRaycastModelRadiusForPixels(edgeMidpoint, uEdgePixelRadius); +highp float clipRadiusA = getRaycastModelRadiusForPixels(vertexA, uNodePixelRadius); +highp float clipRadiusB = getRaycastModelRadiusForPixels(vertexB, uNodePixelRadius); `; - const segmentColorExpression = this.getSegmentColorExpression(); - if ( - skeletonParams.dynamicSegmentAppearance && - this.segmentAttributeIndex !== undefined - ) { - // Dynamic path (spatial skeletons): per-segment color, visibility, - // saturation and hover highlight all resolved in the shader via - // getSegmentAppearance(). uColor is unused in this path. Selected and - // hovered node highlights are drawn as DOM overlays, not in-shader. - const segmentExpression = `vSegmentValue`; - builder.addFragmentCode(` -vec4 segmentColor() { - return getSegmentAppearance(${segmentExpression}); -} -void emitRGBA(vec4 color) { - vec4 baseColor = segmentColor(); - highp float alpha = color.a * baseColor.a; - if (alpha <= 0.0) discard; - vec4 renderColor = vec4(color.rgb, alpha); - vec4 circleColor = getCircleColor(renderColor, renderColor); - emit(vec4(circleColor.rgb * circleColor.a, circleColor.a), vPickID); -} -void emitRGB(vec3 color) { - emitRGBA(vec4(color, 1.0)); -} -void emitDefault() { - emitRGBA(vec4(segmentColor().rgb, 1.0)); -} -`); - } else if (this.segmentColorAttributeIndex === undefined) { - // Legacy path (non-spatial skeletons): one skeleton drawn per call; - // uColor is set per-skeleton by the CPU via getObjectColor(), which - // already incorporates saturation and hover highlighting. - builder.addFragmentCode(` -vec4 segmentColor() { - return ${segmentColorExpression}; -} -void emitRGBA(vec4 color) { - vec4 borderColor = color; - emit(getCircleColor(color, borderColor), vPickID); -} -void emitRGB(vec3 color) { - emitRGBA(vec4(color, 1.0)); -} -void emitDefault() { - emitRGBA(uColor); + vertexMain += this.readSegmentValueGlsl(skeletonParams, "aVertexIndex.x"); + vertexMain += `emitRaycastCylinder(vertexA, vertexB, edgeRadius, clipRadiusA, clipRadiusB);\n`; + return { + vertexMain, + fragmentSetup: raycastFragmentSetup( + "intersectRaycastCylinder", + skeletonParams.spatialChunkCulling, + ), + shading: { + coverageAlpha: "", + shadeColor: " * raycastLightingFactor", + legacyDefaultPremultiply: " * uColor.a", + }, + }; + } + + // Slice view: screen-space anti-aliased circle billboard (constant pixel + // diameter). Feather/border are applied by `getCircleColor`. + private defineNodeCircleBillboard( + builder: ShaderBuilder, + skeletonParams: SkeletonShaderParameters, + ): NodeGeometry { + defineCircleShader(builder, /*crossSectionFade=*/ this.targetIsSliceView); + builder.addUniform("highp float", "uNodeDiameter"); + builder.addFragmentCode(` +vec4 finishNodeColor(vec4 color) { + return getCircleColor(color, color); } `); - } else { - // Per-vertex color attribute path: color comes from a per-vertex - // attribute; alpha is taken from the attribute's alpha component. - builder.addFragmentCode(` -vec4 segmentColor() { - return ${segmentColorExpression}; -} -void emitRGBA(vec4 color) { - vec4 renderColor = color; - vec4 circleColor = getCircleColor(renderColor, renderColor); - emit(vec4(circleColor.rgb * circleColor.a, circleColor.a), vPickID); -} -void emitRGB(vec3 color) { - emitRGBA(vec4(color, 1.0)); -} -void emitDefault() { - emitRGBA(segmentColor()); + let vertexMain = ` +highp uint vertexIndex = uint(gl_InstanceID); +highp uint pickOffset = vertexIndex * uPickInstanceStride; +vPickID = uPickID + pickOffset; +highp vec3 vertexPosition = readAttribute0(vertexIndex); +`; + let fragmentSetup = ""; + if (skeletonParams.spatialChunkCulling) { + vertexMain += `vCullPos = vertexPosition;\n`; + fragmentSetup = `spatialChunkCull();\n`; + } + vertexMain += this.readSegmentValueGlsl(skeletonParams, "vertexIndex"); + vertexMain += `emitCircle(uProjection * vec4(vertexPosition, 1.0), uNodeDiameter, 0.0);\n`; + // The legacy path emits the circle color un-premultiplied (preserved). + return { vertexMain, fragmentSetup, legacyPremultiply: false }; + } + + // Perspective view: raycast sphere (2 triangles). + private defineNodeRaycastSphere( + builder: ShaderBuilder, + skeletonParams: SkeletonShaderParameters, + ): NodeGeometry { + defineRaycastSphereShader(builder); + builder.addUniform("highp float", "uNodePixelRadius"); + builder.addFragmentCode(` +vec4 finishNodeColor(vec4 color) { + return vec4(color.rgb * raycastLightingFactor, color.a); } `); - } - this.finalizeShaderBuilder( - builder, - shaderBuilderState, - skeletonParams, - vertexMain, - ); - }, - }, - ); + let vertexMain = ` +highp uint vertexIndex = uint(gl_InstanceID); +highp uint pickOffset = vertexIndex * uPickInstanceStride; +vPickID = uPickID + pickOffset; +highp vec3 vertexPosition = readAttribute0(vertexIndex); +highp float nodeRadius = getRaycastModelRadiusForPixels(vertexPosition, uNodePixelRadius); +`; + vertexMain += this.readSegmentValueGlsl(skeletonParams, "vertexIndex"); + vertexMain += `emitRaycastSphere(vertexPosition, nodeRadius);\n`; + return { + vertexMain, + fragmentSetup: raycastFragmentSetup( + "intersectRaycastSphere", + skeletonParams.spatialChunkCulling, + ), + legacyPremultiply: true, + }; } defineAttributeAccess(builder: ShaderBuilder) { @@ -935,6 +966,40 @@ void emitDefault() { const { viewProjectionMat } = renderContext.projectionParameters; const mat = mat4.multiply(tempMat4, viewProjectionMat, modelMatrix); gl.uniformMatrix4fv(shader.uniform("uProjection"), false, mat); + if (!this.targetIsSliceView) { + // Raycast uniforms (perspective view). Intersection is done in model + // space, so we provide clip->model and the model-normal->display normal + // transform (inverse-transpose of the model matrix), plus the light and + // viewport size. Mirrors src/annotation/ellipsoid.ts. + const invProjection = mat4.invert(tempInvProjection, mat); + if (invProjection !== null) { + gl.uniformMatrix4fv( + shader.uniform("uInvProjection"), + false, + invProjection, + ); + } + const invModel = mat4.invert(tempInvModel, modelMatrix); + if (invModel !== null) { + const normalTransform = mat4.transpose(tempNormalTransform, invModel); + gl.uniformMatrix4fv( + shader.uniform("uNormalTransform"), + false, + normalTransform, + ); + } + const { width, height } = renderContext.projectionParameters; + gl.uniform2f(shader.uniform("uViewportSize"), width, height); + const perspectiveContext = renderContext as PerspectiveViewRenderContext; + const lightVec = tempLightVec as unknown as vec3; + vec3.scale( + lightVec, + perspectiveContext.lightDirection, + perspectiveContext.directionalLighting, + ); + tempLightVec[3] = perspectiveContext.ambientLighting; + gl.uniform4fv(shader.uniform("uLightDirection"), tempLightVec); + } this.vertexIdHelper.enable(); } @@ -960,6 +1025,39 @@ void emitDefault() { gl.uniform3fv(shader.uniform("uChunkBound"), upperBound); } + // Sets the edge-size uniforms for whichever edge shader variant is active: + // the billboard line width (slice view) or the raycast-cylinder pixel radius + // (perspective view). + setEdgeSizeUniforms( + gl: GL, + shader: ShaderProgram, + lineWidth: number, + pointDiameter: number, + ) { + if (this.targetIsSliceView) { + gl.uniform1f(shader.uniform("uLineWidth"), lineWidth); + gl.uniform1f( + shader.uniform("uLineEndpointClipRadius"), + pointDiameter / 2, + ); + } else { + gl.uniform1f(shader.uniform("uEdgePixelRadius"), lineWidth * 0.5); + // Node radius, used to clip the cylinder ends against the node spheres. + gl.uniform1f(shader.uniform("uNodePixelRadius"), pointDiameter * 0.5); + } + } + + // Sets the node-size uniforms for whichever node shader variant is active: + // the billboard circle diameter (slice view) or the raycast-sphere pixel + // radius (perspective view). + setNodeSizeUniforms(gl: GL, shader: ShaderProgram, pointDiameter: number) { + if (this.targetIsSliceView) { + gl.uniform1f(shader.uniform("uNodeDiameter"), pointDiameter); + } else { + gl.uniform1f(shader.uniform("uNodePixelRadius"), pointDiameter * 0.5); + } + } + drawSkeletons( gl: GL, edgeShader: ShaderProgram, @@ -985,7 +1083,10 @@ void emitDefault() { ); } - // Draw edges + const raycast = !this.targetIsSliceView; + + // Draw edges: lines (slice) or raycast cylinders (perspective). Both are + // instanced quads whose per-instance endpoint pair comes from `aVertexIndex`. { edgeShader.bind(); const aVertexIndex = edgeShader.attribute("aVertexIndex"); @@ -995,23 +1096,28 @@ void emitDefault() { WebGL2RenderingContext.UNSIGNED_INT, ); gl.vertexAttribDivisor(aVertexIndex, 1); - initializeLineShader( - edgeShader, - projectionParameters, - this.targetIsSliceView ? 1.0 : 0.0, - ); - drawLines(gl, 1, skeletonGpuGeometry.numIndices / 2); + if (raycast) { + drawQuads(gl, 1, skeletonGpuGeometry.numIndices / 2); + } else { + initializeLineShader(edgeShader, projectionParameters, 1.0); + drawLines(gl, 1, skeletonGpuGeometry.numIndices / 2); + } gl.vertexAttribDivisor(aVertexIndex, 0); gl.disableVertexAttribArray(aVertexIndex); } - // Draw nodes + // Draw nodes: circles (slice) or raycast spheres (perspective). Position + // is pulled per-instance from the position texture by gl_InstanceID. { nodeShader.bind(); - initializeCircleShader(nodeShader, projectionParameters, { - featherWidthInPixels: this.targetIsSliceView ? 1.0 : 0.0, - }); - drawCircles(nodeShader.gl, 1, skeletonGpuGeometry.numVertices); + if (raycast) { + drawQuads(gl, 1, skeletonGpuGeometry.numVertices); + } else { + initializeCircleShader(nodeShader, projectionParameters, { + featherWidthInPixels: 1.0, + }); + drawCircles(nodeShader.gl, 1, skeletonGpuGeometry.numVertices); + } } } @@ -1302,7 +1408,6 @@ export class SkeletonLayer extends RefCounted implements SkeletonShaderContext { redrawNeeded = new NullarySignal(); private sharedObject: SegmentationLayerSharedObject; vertexAttributes: VertexAttributeRenderInfo[]; - segmentColorAttributeIndex: number | undefined = undefined; // Non-spatial skeletons iterate segments individually and pass color/alpha via // uniforms (getObjectColor), so the dynamic per-vertex segment appearance path // is not needed. Stated colors and default color are likewise handled upstream @@ -1423,15 +1528,11 @@ export class SkeletonLayer extends RefCounted implements SkeletonShaderContext { shaderControlState, edgeShaderParameters.parseResult, ); - gl.uniform1f(edgeShader.uniform("uLineWidth"), lineWidth!); - gl.uniform1f( - edgeShader.uniform("uLineEndpointClipRadius"), - pointDiameter / 2, - ); + renderHelper.setEdgeSizeUniforms(gl, edgeShader, lineWidth!, pointDiameter); nodeShader.bind(); renderHelper.beginLayer(gl, nodeShader, renderContext, modelMatrix); - gl.uniform1f(nodeShader.uniform("uNodeDiameter"), pointDiameter); + renderHelper.setNodeSizeUniforms(gl, nodeShader, pointDiameter); renderHelper.setPickInstanceStride(gl, nodeShader, 0); setControlsInShader( gl, @@ -2089,7 +2190,6 @@ export class SpatiallyIndexedSkeletonLayer layerChunkProgressInfo = new LayerChunkProgressInfo(); redrawNeeded = new NullarySignal(); vertexAttributes: VertexAttributeRenderInfo[]; - segmentColorAttributeIndex: number | undefined; readonly browsePassLayerView: SkeletonShaderContext; readonly skeletonShaderParameters: WatchableValue; readonly browsePassSkeletonShaderParameters: WatchableValueInterface; @@ -2572,11 +2672,8 @@ export class SpatiallyIndexedSkeletonLayer ), ); - // Browse pass uses uniform-based dynamic segment color (not per-vertex attribute), - // so segmentColorAttributeIndex is intentionally undefined here. this.browsePassLayerView = { vertexAttributes: this.source.vertexAttributes, - segmentColorAttributeIndex: undefined, gl: this.gl, fallbackShaderParameters: this.fallbackShaderParameters, displayState: this.displayState, @@ -3090,11 +3187,7 @@ export class SpatiallyIndexedSkeletonLayer edgeShader.bind(); renderHelper.beginLayer(gl, edgeShader, renderContext, modelMatrix); - gl.uniform1f(edgeShader.uniform("uLineWidth"), lineWidth); - gl.uniform1f( - edgeShader.uniform("uLineEndpointClipRadius"), - pointDiameter / 2, - ); + renderHelper.setEdgeSizeUniforms(gl, edgeShader, lineWidth, pointDiameter); renderHelper.setPickInstanceStride(gl, edgeShader, 0); setControlsInShader( gl, @@ -3112,7 +3205,7 @@ export class SpatiallyIndexedSkeletonLayer nodeShader.bind(); renderHelper.beginLayer(gl, nodeShader, renderContext, modelMatrix); - gl.uniform1f(nodeShader.uniform("uNodeDiameter"), pointDiameter); + renderHelper.setNodeSizeUniforms(gl, nodeShader, pointDiameter); renderHelper.setPickInstanceStride(gl, nodeShader, 0); setControlsInShader( gl, diff --git a/src/skeleton/skeleton_shader_color.ts b/src/skeleton/skeleton_shader_color.ts new file mode 100644 index 000000000..3479aa513 --- /dev/null +++ b/src/skeleton/skeleton_shader_color.ts @@ -0,0 +1,141 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file GLSL for the skeleton color paths, shared by the billboard and raycast + * primitives so the color logic is defined once. + */ + +// `dynamic`: per-segment appearance resolved in the shader (spatial skeletons). +// `legacy`: one skeleton drawn per call with a CPU-supplied `uColor`. +export type SkeletonColorPath = "dynamic" | "legacy"; + +// Per-primitive substitutions for the edge color paths. Filling in the +// billboard values reproduces the original line GLSL exactly. +export interface EdgeShadingGlsl { + // Multiplied into the coverage alpha: ` * getLineAlpha() * ...` (billboard) or + // `` (raycast, whose silhouette is already exact). + coverageAlpha: string; + // Multiplied into the emitted rgb: `` (billboard) or ` * raycastLightingFactor`. + shadeColor: string; + // Extra premultiply for the legacy `emitDefault` rgb: `` (billboard emits + // un-premultiplied) or ` * uColor.a` (raycast). + legacyDefaultPremultiply: string; +} + +// GLSL run at the top of the fragment shader before the user's main: intersect +// the raycast surface, discard on a miss, publish the surface depth (for +// gl_FragDepth and the OIT emit depth) and the lighting factor, and cull against +// the chunk bounds using the true surface point. The primitive defines +// `intersectFunction`. +export function raycastFragmentSetup( + intersectFunction: string, + spatialChunkCulling: boolean, +): string { + return ( + ` +RaycastHit raycastHit = ${intersectFunction}(); +if (!raycastHit.hit) discard; +// Discard hits outside the frustum depth range (positive-form test, so it also +// rejects any residual NaN) before they can influence the OIT weight. +if (!(raycastHit.windowDepth >= 0.0 && raycastHit.windowDepth <= 1.0)) discard; +gl_FragDepth = raycastHit.windowDepth; +emitDepthOverride = raycastHit.windowDepth; +raycastLightingFactor = raycastHit.lightingFactor; +` + (spatialChunkCulling ? `spatialChunkCull(raycastHit.surfacePoint);\n` : "") + ); +} + +export function edgeColorPathsGlsl( + path: SkeletonColorPath, + shading: EdgeShadingGlsl, +): string { + const { coverageAlpha, shadeColor, legacyDefaultPremultiply } = shading; + if (path === "dynamic") { + return ` +vec4 segmentColor() { + return getSegmentAppearance(vSegmentValue); +} +void emitRGB(vec3 color) { + vec4 baseColor = segmentColor(); + highp float alpha = baseColor.a${coverageAlpha}; + if (alpha <= 0.0) discard; + emit(vec4(color${shadeColor} * alpha, alpha), vPickID); +} +void emitDefault() { + vec4 baseColor = segmentColor(); + highp float alpha = baseColor.a${coverageAlpha}; + if (alpha <= 0.0) discard; + emit(vec4(baseColor.rgb${shadeColor} * alpha, alpha), vPickID); +} +`; + } + return ` +vec4 segmentColor() { + return uColor; +} +void emitRGB(vec3 color) { + emit(vec4(color${shadeColor} * uColor.a, uColor.a${coverageAlpha}), vPickID); +} +void emitDefault() { + emit(vec4(uColor.rgb${shadeColor}${legacyDefaultPremultiply}, uColor.a${coverageAlpha}), vPickID); +} +`; +} + +export function nodeColorPathsGlsl( + path: SkeletonColorPath, + legacyPremultiply: boolean, +): string { + if (path === "dynamic") { + return ` +vec4 segmentColor() { + return getSegmentAppearance(vSegmentValue); +} +void emitRGBA(vec4 color) { + vec4 baseColor = segmentColor(); + highp float alpha = color.a * baseColor.a; + if (alpha <= 0.0) discard; + vec4 finished = finishNodeColor(vec4(color.rgb, alpha)); + emit(vec4(finished.rgb * finished.a, finished.a), vPickID); +} +void emitRGB(vec3 color) { + emitRGBA(vec4(color, 1.0)); +} +void emitDefault() { + emitRGBA(vec4(segmentColor().rgb, 1.0)); +} +`; + } + const legacyEmit = legacyPremultiply + ? "emit(vec4(finished.rgb * finished.a, finished.a), vPickID);" + : "emit(finished, vPickID);"; + return ` +vec4 segmentColor() { + return uColor; +} +void emitRGBA(vec4 color) { + vec4 finished = finishNodeColor(color); + ${legacyEmit} +} +void emitRGB(vec3 color) { + emitRGBA(vec4(color, 1.0)); +} +void emitDefault() { + emitRGBA(uColor); +} +`; +} diff --git a/src/webgl/raycast_cylinder.ts b/src/webgl/raycast_cylinder.ts new file mode 100644 index 000000000..c12202cc0 --- /dev/null +++ b/src/webgl/raycast_cylinder.ts @@ -0,0 +1,148 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file Raycast (raycast) finite cylinder drawn on a camera-facing quad; see + * `raycast_primitive.ts` for the shared conventions. + * + * The ray/cylinder intersection is adapted from Inigo Quilez's capped-cylinder + * intersector (https://iquilezles.org/articles/intersectors/), MIT licensed: + * + * The MIT License. Copyright (c) 2016 Inigo Quilez. + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: the above copyright + * notice and this permission notice shall be included in all copies or + * substantial portions of the Software. THE SOFTWARE IS PROVIDED "AS IS". + */ + +import { defineRaycastPrimitiveCommon } from "#src/webgl/raycast_primitive.js"; +import type { ShaderBuilder } from "#src/webgl/shader.js"; + +export interface RaycastCylinderOptions { + capped: boolean; +} + +/** + * Adds `emitRaycastCylinder(endpointA, endpointB, radius, clipRadiusA, + * clipRadiusB)` (vertex) and `intersectRaycastCylinder()` (fragment). All + * arguments are in model space. Surface points within `clipRadiusA/B` of an + * endpoint are discarded so an abutting sphere covers the joint without + * overlapping (which would double-blend under order-independent transparency); + * pass 0 to disable clipping at that end. + * TODO: support per-endpoint radii for tapered/conical segments. + */ +export function defineRaycastCylinderShader( + builder: ShaderBuilder, + options: RaycastCylinderOptions, +) { + defineRaycastPrimitiveCommon(builder); + builder.addVarying("highp vec3", "vCylinderEndpointA", "flat"); + builder.addVarying("highp vec3", "vCylinderEndpointB", "flat"); + builder.addVarying("highp float", "vCylinderRadius", "flat"); + builder.addVarying("highp float", "vCylinderClipRadiusA", "flat"); + builder.addVarying("highp float", "vCylinderClipRadiusB", "flat"); + builder.addVertexCode(` +void emitRaycastCylinder(highp vec3 endpointA, highp vec3 endpointB, + highp float radius, + highp float clipRadiusA, highp float clipRadiusB) { + vCylinderEndpointA = endpointA; + vCylinderEndpointB = endpointB; + vCylinderRadius = radius; + vCylinderClipRadiusA = clipRadiusA; + vCylinderClipRadiusB = clipRadiusB; + highp vec3 axisVector = endpointB - endpointA; + highp float axisLength = length(axisVector); + highp vec3 axisDirection = axisLength > 1e-6 ? axisVector / axisLength : vec3(0.0, 1.0, 0.0); + highp vec3 referenceVector = + abs(axisDirection.y) < 0.99 ? vec3(0.0, 1.0, 0.0) : vec3(1.0, 0.0, 0.0); + highp vec3 radialU = normalize(cross(referenceVector, axisDirection)) * radius; + highp vec3 radialV = normalize(cross(axisDirection, radialU)) * radius; + RaycastBounds bounds = beginRaycastBounds(); + for (int corner = 0; corner < 8; ++corner) { + highp vec3 ringCenter = (corner & 4) == 0 ? endpointA : endpointB; + highp float signU = (corner & 1) == 0 ? -1.0 : 1.0; + highp float signV = (corner & 2) == 0 ? -1.0 : 1.0; + accumulateRaycastCorner(bounds, ringCenter + signU * radialU + signV * radialV); + } + gl_Position = getRaycastQuadPosition(bounds); +} +`); + if (options.capped) { + builder.addFragmentCode("#define RAYCAST_CYLINDER_CAPPED\n"); + } + builder.addFragmentCode(` +bool cylinderPointClipped(highp vec3 surfacePoint) { + return distance(surfacePoint, vCylinderEndpointA) < vCylinderClipRadiusA || + distance(surfacePoint, vCylinderEndpointB) < vCylinderClipRadiusB; +} +RaycastHit intersectRaycastCylinder() { + RaycastHit result; + result.hit = false; + RaycastRay ray = getRaycastEyeRay(); + highp vec3 axis = vCylinderEndpointB - vCylinderEndpointA; + highp vec3 originToBase = ray.origin - vCylinderEndpointA; + highp float axisLengthSq = dot(axis, axis); + highp float axisDotDirection = dot(axis, ray.direction); + highp float axisDotOrigin = dot(axis, originToBase); + highp float quadA = axisLengthSq - axisDotDirection * axisDotDirection; + highp float quadB = axisLengthSq * dot(originToBase, ray.direction) - axisDotOrigin * axisDotDirection; + highp float quadC = axisLengthSq * dot(originToBase, originToBase) - + axisDotOrigin * axisDotOrigin - vCylinderRadius * vCylinderRadius * axisLengthSq; + highp float discriminant = quadB * quadB - quadA * quadC; + // Positive-form guard so a NaN ray (degenerate projection) misses rather than + // slipping through (NaN < 0.0 is false). + if (!(discriminant >= 0.0)) return result; + highp float sqrtDiscriminant = sqrt(discriminant); + // Lateral surface (near root). + highp float hitDistance = (-quadB - sqrtDiscriminant) / quadA; + highp float axialCoord = axisDotOrigin + hitDistance * axisDotDirection; + if (hitDistance >= 0.0 && axialCoord >= 0.0 && axialCoord <= axisLengthSq) { + highp vec3 surfacePoint = ray.origin + hitDistance * ray.direction; + if (!cylinderPointClipped(surfacePoint)) { + highp vec3 surfaceNormal = + (originToBase + hitDistance * ray.direction - axis * (axialCoord / axisLengthSq)) / + vCylinderRadius; + result.hit = true; + result.surfacePoint = surfacePoint; + result.windowDepth = getRaycastWindowDepth(surfacePoint); + result.lightingFactor = getRaycastSurfaceLighting(normalize(surfaceNormal)); + return result; + } + } +#ifdef RAYCAST_CYLINDER_CAPPED + // End caps at axialCoord == 0 (endpoint A) and axialCoord == axisLengthSq (B). + highp float capDistance = + ((axialCoord < 0.0 ? 0.0 : axisLengthSq) - axisDotOrigin) / axisDotDirection; + if (capDistance >= 0.0 && abs(quadB + quadA * capDistance) < sqrtDiscriminant) { + highp vec3 surfacePoint = ray.origin + capDistance * ray.direction; + if (!cylinderPointClipped(surfacePoint)) { + highp vec3 surfaceNormal = axis * sign(axialCoord) / sqrt(axisLengthSq); + result.hit = true; + result.surfacePoint = surfacePoint; + result.windowDepth = getRaycastWindowDepth(surfacePoint); + result.lightingFactor = getRaycastSurfaceLighting(normalize(surfaceNormal)); + return result; + } + } +#endif + return result; +} +`); +} diff --git a/src/webgl/raycast_primitive.ts b/src/webgl/raycast_primitive.ts new file mode 100644 index 000000000..0f8156106 --- /dev/null +++ b/src/webgl/raycast_primitive.ts @@ -0,0 +1,146 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file Shared GLSL for raycast (raycast) primitives: a camera-facing quad (2 + * triangles) whose fragment shader ray-intersects the true 3D surface, writes + * `gl_FragDepth`, and shades a normal. Intersection is done in model space, so + * the (possibly anisotropic/sheared) `uProjection` deforms the model-space + * sphere/cylinder into the correct display-space shape, as in + * `src/annotation/ellipsoid.ts`. + * + * The consumer must declare these uniforms (see `frontend.ts` + * `defineCommonShader`): + * uniform highp mat4 uProjection; // model -> clip + * uniform highp mat4 uInvProjection; // clip -> model + * uniform highp mat4 uNormalTransform; // model-normal -> display, = (M^-1)^T + * uniform highp vec4 uLightDirection; // xyz = dir*intensity, w = ambient + * uniform highp vec2 uViewportSize; // device pixels + */ + +import { glsl_getQuadVertexPosition } from "#src/webgl/quad.js"; +import type { ShaderBuilder } from "#src/webgl/shader.js"; + +// Fragment helpers: eye-ray reconstruction, model->window depth, and the Lambert +// lighting factor. `RaycastHit` is returned by each intersection routine. +export const glsl_raycastPrimitiveFragmentUtil = ` +struct RaycastRay { + highp vec3 origin; + highp vec3 direction; +}; +struct RaycastHit { + bool hit; + highp vec3 surfacePoint; + highp float windowDepth; + highp float lightingFactor; +}; +RaycastRay getRaycastEyeRay() { + highp vec2 normalizedDeviceCoord = (gl_FragCoord.xy / uViewportSize) * 2.0 - 1.0; + highp vec4 nearClip = uInvProjection * vec4(normalizedDeviceCoord, -1.0, 1.0); + highp vec4 farClip = uInvProjection * vec4(normalizedDeviceCoord, 1.0, 1.0); + highp vec3 nearModel = nearClip.xyz / nearClip.w; + highp vec3 farModel = farClip.xyz / farClip.w; + RaycastRay ray; + ray.origin = nearModel; + ray.direction = normalize(farModel - nearModel); + return ray; +} +highp float getRaycastWindowDepth(highp vec3 modelPoint) { + // Assumes the default depth range [0, 1] and NDC z in [-1, 1]. + highp vec4 clip = uProjection * vec4(modelPoint, 1.0); + return 0.5 * (clip.z / clip.w) + 0.5; +} +highp float getRaycastSurfaceLighting(highp vec3 modelNormal) { + highp vec3 displayNormal = normalize((uNormalTransform * vec4(modelNormal, 0.0)).xyz); + return abs(dot(displayNormal, uLightDirection.xyz)) + uLightDirection.w; +} +`; + +// Accumulates the projected NDC bounding box of the primitive's model-space AABB +// corners, then emits the current quad corner covering it. A corner on/behind +// the near plane must not be dropped (that would under-cover a primitive +// straddling the near plane, leaving it undrawn); instead its w is clamped +// positive so it projects off-screen and expands the bounds, and the fragment +// ray test trims the excess. NDC is bounded so the expansion stays finite. +export const glsl_raycastPrimitiveVertexUtil = ` +const highp float RAYCAST_NDC_BOUND = 2.0; +struct RaycastBounds { + highp vec2 ndcMin; + highp vec2 ndcMax; + highp float ndcNearZ; + bool anyCornerValid; +}; +RaycastBounds beginRaycastBounds() { + RaycastBounds bounds; + bounds.ndcMin = vec2(0.0); + bounds.ndcMax = vec2(0.0); + bounds.ndcNearZ = 0.0; + bounds.anyCornerValid = false; + return bounds; +} +void accumulateRaycastCorner(inout RaycastBounds bounds, highp vec3 modelCorner) { + highp vec4 clip = uProjection * vec4(modelCorner, 1.0); + highp float clipW = max(clip.w, 1e-4); + highp vec2 ndcXY = + clamp(clip.xy / clipW, vec2(-RAYCAST_NDC_BOUND), vec2(RAYCAST_NDC_BOUND)); + highp float ndcZ = clip.z / clipW; + if (!bounds.anyCornerValid) { + bounds.ndcMin = ndcXY; + bounds.ndcMax = ndcXY; + bounds.ndcNearZ = ndcZ; + bounds.anyCornerValid = true; + } else { + bounds.ndcMin = min(bounds.ndcMin, ndcXY); + bounds.ndcMax = max(bounds.ndcMax, ndcXY); + bounds.ndcNearZ = min(bounds.ndcNearZ, ndcZ); + } +} +vec4 getRaycastQuadPosition(RaycastBounds bounds) { + if (!bounds.anyCornerValid) { + // Cull: position outside the clip volume. + return vec4(2.0, 2.0, 2.0, 1.0); + } + // Expand slightly so the analytic silhouette is never clipped by the quad. + highp vec2 margin = (bounds.ndcMax - bounds.ndcMin) * 0.02 + 2.0 / uViewportSize; + highp vec2 lowerCorner = bounds.ndcMin - margin; + highp vec2 upperCorner = bounds.ndcMax + margin; + highp vec2 quadCorner = getQuadVertexPosition(lowerCorner, upperCorner); + return vec4(quadCorner, clamp(bounds.ndcNearZ, -1.0, 1.0), 1.0); +} +`; + +// Model-space radius that projects to `radiusInPixels` device px at `modelPoint` +// (using the vertical viewport extent), giving raycasts a constant on-screen +// size like the billboards. +export const glsl_raycastPrimitivePixelRadius = ` +highp float getRaycastModelRadiusForPixels(highp vec3 modelPoint, highp float radiusInPixels) { + highp vec4 clip = uProjection * vec4(modelPoint, 1.0); + highp float clipW = max(clip.w, 1e-6); + highp float ndcPerPixel = 2.0 / uViewportSize.y; + highp vec4 clipDelta = vec4(0.0, ndcPerPixel * clipW * radiusInPixels, 0.0, 0.0); + highp vec3 modelDelta = (uInvProjection * clipDelta).xyz; + return length(modelDelta); +} +`; + +// Convenience: pull in the fragment- and vertex-stage raycast helpers. The +// consumer must separately declare the uniforms documented in the file header. +export function defineRaycastPrimitiveCommon(builder: ShaderBuilder) { + builder.addVertexCode(glsl_getQuadVertexPosition); + builder.addVertexCode(glsl_raycastPrimitiveVertexUtil); + builder.addVertexCode(glsl_raycastPrimitivePixelRadius); + builder.addFragmentCode(glsl_raycastPrimitiveFragmentUtil); +} diff --git a/src/webgl/raycast_sphere.ts b/src/webgl/raycast_sphere.ts new file mode 100644 index 000000000..e31a6ec14 --- /dev/null +++ b/src/webgl/raycast_sphere.ts @@ -0,0 +1,73 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file Raycast (raycast) sphere drawn on a camera-facing quad; see + * `raycast_primitive.ts` for the shared conventions. + */ + +import { defineRaycastPrimitiveCommon } from "#src/webgl/raycast_primitive.js"; +import type { ShaderBuilder } from "#src/webgl/shader.js"; + +/** + * Adds `emitRaycastSphere(center, radius)` (vertex) and + * `intersectRaycastSphere()` (fragment); `center`/`radius` are in model space. + */ +export function defineRaycastSphereShader(builder: ShaderBuilder) { + defineRaycastPrimitiveCommon(builder); + builder.addVarying("highp vec3", "vSphereCenter", "flat"); + builder.addVarying("highp float", "vSphereRadius", "flat"); + builder.addVertexCode(` +void emitRaycastSphere(highp vec3 center, highp float radius) { + vSphereCenter = center; + vSphereRadius = radius; + RaycastBounds bounds = beginRaycastBounds(); + for (int corner = 0; corner < 8; ++corner) { + highp float signX = (corner & 1) == 0 ? -1.0 : 1.0; + highp float signY = (corner & 2) == 0 ? -1.0 : 1.0; + highp float signZ = (corner & 4) == 0 ? -1.0 : 1.0; + accumulateRaycastCorner(bounds, center + radius * vec3(signX, signY, signZ)); + } + gl_Position = getRaycastQuadPosition(bounds); +} +`); + builder.addFragmentCode(` +RaycastHit intersectRaycastSphere() { + RaycastHit result; + result.hit = false; + RaycastRay ray = getRaycastEyeRay(); + highp vec3 originToCenter = ray.origin - vSphereCenter; + highp float projectedDistance = dot(originToCenter, ray.direction); + highp float centerDistanceSq = + dot(originToCenter, originToCenter) - vSphereRadius * vSphereRadius; + highp float discriminant = projectedDistance * projectedDistance - centerDistanceSq; + // Positive-form guards so a NaN ray (degenerate projection) is treated as a + // miss rather than slipping through (NaN < 0.0 is false). + if (!(discriminant >= 0.0)) return result; + highp float sqrtDiscriminant = sqrt(discriminant); + highp float hitDistance = -projectedDistance - sqrtDiscriminant; + if (hitDistance < 0.0) hitDistance = -projectedDistance + sqrtDiscriminant; + if (!(hitDistance >= 0.0)) return result; + highp vec3 surfacePoint = ray.origin + hitDistance * ray.direction; + result.hit = true; + result.surfacePoint = surfacePoint; + result.windowDepth = getRaycastWindowDepth(surfacePoint); + result.lightingFactor = + getRaycastSurfaceLighting((surfacePoint - vSphereCenter) / vSphereRadius); + return result; +} +`); +} From 6c95308f45abaefddef052be338355a9b3b0fa2f Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Wed, 15 Jul 2026 15:12:48 +0200 Subject: [PATCH 05/11] Merge pull request from MetaCell/feature/NGLASS-2012 NGLASS-2012 Improved Neuroglancer banner for clearer information when in a tool --- src/ui/images/mouse.svg | 0 src/ui/skeleton_edit_tool_messages.spec.ts | 176 ++++++++++++--------- src/ui/skeleton_edit_tool_messages.ts | 105 ++++++++---- src/ui/skeleton_edit_tools.css | 53 ++++--- src/ui/skeleton_edit_tools.ts | 40 ++--- 5 files changed, 222 insertions(+), 152 deletions(-) create mode 100644 src/ui/images/mouse.svg diff --git a/src/ui/images/mouse.svg b/src/ui/images/mouse.svg new file mode 100644 index 000000000..e69de29bb diff --git a/src/ui/skeleton_edit_tool_messages.spec.ts b/src/ui/skeleton_edit_tool_messages.spec.ts index 312279460..5372d1c87 100644 --- a/src/ui/skeleton_edit_tool_messages.spec.ts +++ b/src/ui/skeleton_edit_tool_messages.spec.ts @@ -1,10 +1,5 @@ import { describe, expect, it } from "vitest"; -import { - SPATIAL_SKELETON_ROTATE_PAN_HINT, - formatSpatialSkeletonToolPoint, - getSpatialSkeletonCreateIdleStatusText, - getSpatialSkeletonCreatingStatusText, getSpatialSkeletonDefaultStatusText, getSpatialSkeletonDeleteIdleStatusText, getSpatialSkeletonDeletingStatusText, @@ -14,8 +9,23 @@ import { getSpatialSkeletonSplitIdleStatusText, getSpatialSkeletonSplittingStatusText, getSpatialSkeletonToolPointSummaryRow, - getSpatialSkeletonToolPointStatusFields, -} from "#src/ui/skeleton_edit_tool_messages.js"; +import { + ADD_NODE_ACTION, + DELETE_ACTION, + DELETE_CLICK_ACTION, + EXIT_CREATE_ACTION, + EXIT_DELETE_ACTION, + EXIT_MERGE_ACTION, + EXIT_SPLIT_ACTION, + MERGE_ACTION, + MOVE_ACTION, + NEW_SKELETON_ACTION, + PLACE_ACTION, + SELECT_ACTION, + SHOW_SKELETON_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + SPLIT_ACTION, +} from "#src/ui/skeleton_edit_tool_shortcuts.js"; describe("spatial_skeleton_tool_messages", () => { it("formats tool points with node and segment ids", () => { @@ -54,181 +64,201 @@ describe("spatial_skeleton_tool_messages", () => { describe("getSpatialSkeletonDefaultStatusText", () => { it("no selection", () => { - expect(getSpatialSkeletonDefaultStatusText("none", false)).toEqual({ - status: "No selection", - actions: `Click to select · drag to move · hold m to merge · hold s to split · hold n for new skeleton · hold d to delete · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SELECT_ACTION, + MOVE_ACTION, + MERGE_ACTION, + SPLIT_ACTION, + NEW_SKELETON_ACTION, + DELETE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); it("selected, visible skeleton", () => { expect( getSpatialSkeletonDefaultStatusText("selected-visible", false), - ).toEqual({ - status: "Node selected", - actions: `Click to select · drag to move · shift+click to add node · hold m to merge · hold s to split · hold n for new skeleton · hold d to delete · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SELECT_ACTION, + MOVE_ACTION, + ADD_NODE_ACTION, + MERGE_ACTION, + SPLIT_ACTION, + NEW_SKELETON_ACTION, + DELETE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); it("selected, visible skeleton, shift held", () => { expect( getSpatialSkeletonDefaultStatusText("selected-visible", true), - ).toEqual({ - status: "Ready to place new node", - actions: `Click to select · drag to move · shift+click to add node · hold m to merge · hold s to split · hold n for new skeleton · hold d to delete · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SELECT_ACTION, + MOVE_ACTION, + ADD_NODE_ACTION, + MERGE_ACTION, + SPLIT_ACTION, + NEW_SKELETON_ACTION, + DELETE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); it("selected, non-visible skeleton", () => { expect( getSpatialSkeletonDefaultStatusText("selected-hidden", false), - ).toEqual({ - status: "Node selected from non-visible skeleton", - actions: `Double-click skeleton to show it · hold m to merge · hold s to split · hold n for new skeleton · hold d to delete · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SHOW_SKELETON_ACTION, + MERGE_ACTION, + SPLIT_ACTION, + NEW_SKELETON_ACTION, + DELETE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); it("selected, non-visible skeleton, shift held — unaffected by shift", () => { expect( getSpatialSkeletonDefaultStatusText("selected-hidden", true), - ).toEqual({ - status: "Node selected from non-visible skeleton", - actions: `Double-click skeleton to show it · hold m to merge · hold s to split · hold n for new skeleton · hold d to delete · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SHOW_SKELETON_ACTION, + MERGE_ACTION, + SPLIT_ACTION, + NEW_SKELETON_ACTION, + DELETE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); }); it("returns a static moving-node status", () => { - expect(getSpatialSkeletonMovingStatusText()).toEqual({ - status: "Moving node", - actions: SPATIAL_SKELETON_ROTATE_PAN_HINT, + actions: [SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); describe("getSpatialSkeletonMergeStatusText", () => { it("no from node, key held", () => { - expect(getSpatialSkeletonMergeStatusText("no-from-node", true)).toEqual({ - status: "Merge · click a node to merge from", - actions: `Click to select node · release m to exit merge · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SELECT_ACTION, + EXIT_MERGE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); it("no from node, key not held", () => { - expect(getSpatialSkeletonMergeStatusText("no-from-node", false)).toEqual({ - status: "Merge · click a node to merge from", - actions: `Click to select node · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [SELECT_ACTION, SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); it("from node selected on a visible skeleton, key held", () => { expect( getSpatialSkeletonMergeStatusText("from-node-visible", true), - ).toEqual({ - status: "Merge · click a node to merge to", - actions: `Click to select node · release m to exit merge · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SELECT_ACTION, + EXIT_MERGE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); it("from node selected on a visible skeleton, key not held", () => { expect( getSpatialSkeletonMergeStatusText("from-node-visible", false), - ).toEqual({ - status: "Merge · click a node to merge to", - actions: `Click to select node · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [SELECT_ACTION, SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); it("from node on a non-visible skeleton, key held", () => { expect( getSpatialSkeletonMergeStatusText("from-node-hidden", true), - ).toEqual({ - status: "Merge · make the from-node skeleton visible", - actions: `Double-click skeleton to show it · release m to exit merge · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SHOW_SKELETON_ACTION, + EXIT_MERGE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); it("from node on a non-visible skeleton, key not held", () => { expect( getSpatialSkeletonMergeStatusText("from-node-hidden", false), - ).toEqual({ - status: "Merge · make the from-node skeleton visible", - actions: `Double-click skeleton to show it · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [SHOW_SKELETON_ACTION, SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); }); it("returns a static merging status", () => { - expect(getSpatialSkeletonMergingStatusText()).toEqual({ - status: "Merge · merging nodes…", - actions: SPATIAL_SKELETON_ROTATE_PAN_HINT, + actions: [SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); describe("getSpatialSkeletonSplitIdleStatusText", () => { it("key held", () => { - expect(getSpatialSkeletonSplitIdleStatusText(true)).toEqual({ - status: "Split · click a node to form the root of a new skeleton", - actions: `Click to select node · release s to exit split · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SELECT_ACTION, + EXIT_SPLIT_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); it("key not held", () => { - expect(getSpatialSkeletonSplitIdleStatusText(false)).toEqual({ - status: "Split · click a node to form the root of a new skeleton", - actions: `Click to select node · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [SELECT_ACTION, SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); }); it("returns a static splitting status", () => { - expect(getSpatialSkeletonSplittingStatusText()).toEqual({ - status: "Split · splitting node…", - actions: SPATIAL_SKELETON_ROTATE_PAN_HINT, + actions: [SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); describe("getSpatialSkeletonDeleteIdleStatusText", () => { it("key held", () => { - expect(getSpatialSkeletonDeleteIdleStatusText(true)).toEqual({ - status: "Delete · no selected nodes", - actions: `Click a node to delete · release d to exit delete · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + DELETE_CLICK_ACTION, + EXIT_DELETE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); it("key not held", () => { - expect(getSpatialSkeletonDeleteIdleStatusText(false)).toEqual({ - status: "Delete · no selected nodes", - actions: `Click a node to delete · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [DELETE_CLICK_ACTION, SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); }); it("returns a static deleting status", () => { - expect(getSpatialSkeletonDeletingStatusText()).toEqual({ - status: "Delete · deleting node…", - actions: SPATIAL_SKELETON_ROTATE_PAN_HINT, + actions: [SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); describe("getSpatialSkeletonCreateIdleStatusText", () => { it("key held", () => { - expect(getSpatialSkeletonCreateIdleStatusText(true)).toEqual({ - status: "Create · ready to place", - actions: `Click to place a new skeleton · release n to exit create · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + PLACE_ACTION, + EXIT_CREATE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }); }); it("key not held", () => { - expect(getSpatialSkeletonCreateIdleStatusText(false)).toEqual({ - status: "Create · ready to place", - actions: `Click to place a new skeleton · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [PLACE_ACTION, SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); }); it("returns a static creating status", () => { - expect(getSpatialSkeletonCreatingStatusText()).toEqual({ - status: "Create · creating skeleton…", - actions: SPATIAL_SKELETON_ROTATE_PAN_HINT, + actions: [SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); }); diff --git a/src/ui/skeleton_edit_tool_messages.ts b/src/ui/skeleton_edit_tool_messages.ts index 83c714698..571ce0713 100644 --- a/src/ui/skeleton_edit_tool_messages.ts +++ b/src/ui/skeleton_edit_tool_messages.ts @@ -14,6 +14,28 @@ * limitations under the License. */ +import type { + SpatialSkeletonShortcut, + SpatialSkeletonToolStatusText, +} from "#src/ui/skeleton_edit_tool_shortcuts.js"; +import { + ADD_NODE_ACTION, + DELETE_ACTION, + DELETE_CLICK_ACTION, + EXIT_CREATE_ACTION, + EXIT_DELETE_ACTION, + EXIT_MERGE_ACTION, + EXIT_SPLIT_ACTION, + MERGE_ACTION, + MOVE_ACTION, + NEW_SKELETON_ACTION, + PLACE_ACTION, + SELECT_ACTION, + SHOW_SKELETON_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + SPLIT_ACTION, +} from "#src/ui/skeleton_edit_tool_shortcuts.js"; + export interface SpatialSkeletonToolPointInfo { nodeId: number; segmentId?: number; @@ -97,8 +119,7 @@ export function getSpatialSkeletonToolPointStatusFields( // yet (e.g. shift+click, which requires an existing selection). // // User-facing copy says "from node" rather than "merge anchor" — the -// internal name (mergeAnchorNodeId, etc.) is unaffected. - +<<<<<<< HEAD export interface SpatialSkeletonToolStatusText { status: string; actions: string; @@ -108,6 +129,8 @@ export const SPATIAL_SKELETON_EDIT_TOOL_NAME = "Skeleton editing"; export const SPATIAL_SKELETON_ROTATE_PAN_HINT = "middle-click or ctrl+click to rotate/pan"; +======= +>>>>>>> 209548a32 (Merge pull request #285 from MetaCell/feature/NGLASS-2012) export type SpatialSkeletonDefaultSelectionState = | "none" | "selected-visible" @@ -121,23 +144,50 @@ export function getSpatialSkeletonDefaultStatusText( case "none": return { status: "No selection", - actions: `Click to select · drag to move · hold m to merge · hold s to split · hold n for new skeleton · hold d to delete · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SELECT_ACTION, + MOVE_ACTION, + MERGE_ACTION, + SPLIT_ACTION, + NEW_SKELETON_ACTION, + DELETE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }; case "selected-visible": return { status: shiftHeld ? "Ready to place new node" : "Node selected", - actions: `Click to select · drag to move · shift+click to add node · hold m to merge · hold s to split · hold n for new skeleton · hold d to delete · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SELECT_ACTION, + MOVE_ACTION, + ADD_NODE_ACTION, + MERGE_ACTION, + SPLIT_ACTION, + NEW_SKELETON_ACTION, + DELETE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }; case "selected-hidden": return { status: "Node selected from non-visible skeleton", - actions: `Double-click skeleton to show it · hold m to merge · hold s to split · hold n for new skeleton · hold d to delete · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`, + actions: [ + SHOW_SKELETON_ACTION, + MERGE_ACTION, + SPLIT_ACTION, + NEW_SKELETON_ACTION, + DELETE_ACTION, + SPATIAL_SKELETON_ROTATE_PAN_ACTION, + ], }; } } export function getSpatialSkeletonMovingStatusText(): SpatialSkeletonToolStatusText { - return { status: "Moving node", actions: SPATIAL_SKELETON_ROTATE_PAN_HINT }; + return { + status: "Moving node", + actions: [SPATIAL_SKELETON_ROTATE_PAN_ACTION], + }; } export type SpatialSkeletonMergeState = @@ -146,38 +196,37 @@ export type SpatialSkeletonMergeState = | "from-node-hidden"; function withExitHint( - action: string, + action: SpatialSkeletonShortcut, canExitWithKey: boolean, - exitHint: string, -) { + exitAction: SpatialSkeletonShortcut, +): SpatialSkeletonShortcut[] { return canExitWithKey - ? `${action} · ${exitHint} · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}` - : `${action} · ${SPATIAL_SKELETON_ROTATE_PAN_HINT}`; + ? [action, exitAction, SPATIAL_SKELETON_ROTATE_PAN_ACTION] + : [action, SPATIAL_SKELETON_ROTATE_PAN_ACTION]; } export function getSpatialSkeletonMergeStatusText( state: SpatialSkeletonMergeState, canExitWithKey: boolean, ): SpatialSkeletonToolStatusText { - const exitHint = "release m to exit merge"; switch (state) { case "no-from-node": return { status: "Merge · click a node to merge from", - actions: withExitHint("Click to select node", canExitWithKey, exitHint), + actions: withExitHint(SELECT_ACTION, canExitWithKey, EXIT_MERGE_ACTION), }; case "from-node-visible": return { status: "Merge · click a node to merge to", - actions: withExitHint("Click to select node", canExitWithKey, exitHint), + actions: withExitHint(SELECT_ACTION, canExitWithKey, EXIT_MERGE_ACTION), }; case "from-node-hidden": return { status: "Merge · make the from-node skeleton visible", actions: withExitHint( - "Double-click skeleton to show it", + SHOW_SKELETON_ACTION, canExitWithKey, - exitHint, + EXIT_MERGE_ACTION, ), }; } @@ -186,7 +235,7 @@ export function getSpatialSkeletonMergeStatusText( export function getSpatialSkeletonMergingStatusText(): SpatialSkeletonToolStatusText { return { status: "Merge · merging nodes…", - actions: SPATIAL_SKELETON_ROTATE_PAN_HINT, + actions: [SPATIAL_SKELETON_ROTATE_PAN_ACTION], }; } @@ -195,18 +244,14 @@ export function getSpatialSkeletonSplitIdleStatusText( ): SpatialSkeletonToolStatusText { return { status: "Split · click a node to form the root of a new skeleton", - actions: withExitHint( - "Click to select node", - canExitWithKey, - "release s to exit split", - ), + actions: withExitHint(SELECT_ACTION, canExitWithKey, EXIT_SPLIT_ACTION), }; } export function getSpatialSkeletonSplittingStatusText(): SpatialSkeletonToolStatusText { return { status: "Split · splitting node…", - actions: SPATIAL_SKELETON_ROTATE_PAN_HINT, + actions: [SPATIAL_SKELETON_ROTATE_PAN_ACTION], }; } @@ -216,9 +261,9 @@ export function getSpatialSkeletonDeleteIdleStatusText( return { status: "Delete · no selected nodes", actions: withExitHint( - "Click a node to delete", + DELETE_CLICK_ACTION, canExitWithKey, - "release d to exit delete", + EXIT_DELETE_ACTION, ), }; } @@ -226,7 +271,7 @@ export function getSpatialSkeletonDeleteIdleStatusText( export function getSpatialSkeletonDeletingStatusText(): SpatialSkeletonToolStatusText { return { status: "Delete · deleting node…", - actions: SPATIAL_SKELETON_ROTATE_PAN_HINT, + actions: [SPATIAL_SKELETON_ROTATE_PAN_ACTION], }; } @@ -235,17 +280,13 @@ export function getSpatialSkeletonCreateIdleStatusText( ): SpatialSkeletonToolStatusText { return { status: "Create · ready to place", - actions: withExitHint( - "Click to place a new skeleton", - canExitWithKey, - "release n to exit create", - ), + actions: withExitHint(PLACE_ACTION, canExitWithKey, EXIT_CREATE_ACTION), }; } export function getSpatialSkeletonCreatingStatusText(): SpatialSkeletonToolStatusText { return { status: "Create · creating skeleton…", - actions: SPATIAL_SKELETON_ROTATE_PAN_HINT, + actions: [SPATIAL_SKELETON_ROTATE_PAN_ACTION], }; } diff --git a/src/ui/skeleton_edit_tools.css b/src/ui/skeleton_edit_tools.css index a6f15e5ae..ff817d2a6 100644 --- a/src/ui/skeleton_edit_tools.css +++ b/src/ui/skeleton_edit_tools.css @@ -20,42 +20,57 @@ align-items: center; gap: 0.75rem; min-width: 0; -} -/* Sits right after the divider, immediately beside the tool name in the - header, sized to its own content (not flex-grow) rather than stretching - across the bar. */ -.neuroglancer-skeleton-tool-status-divider { - flex: 0 0 auto; - color: #6b6c6f; - line-height: 1.25rem; -} - -.neuroglancer-skeleton-tool-status-text { - flex: 0 1 auto; - min-width: 0; overflow: hidden; text-overflow: ellipsis; - white-space: nowrap; - line-height: 1.25rem; + color: #999ca0; } /* margin-left: auto consumes all remaining space on the main axis before justify-content is applied, so the actions list is pushed to the far right regardless of the status text's width or the parent's justify-content (neuroglass-theme.css sets justify-content: flex-end on - this same element via a higher-specificity nested rule). */ -.neuroglancer-skeleton-tool-status-actions { + display: flex; flex: 0 1 auto; + align-items: center; + gap: 1rem; margin-left: auto; min-width: 0; overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; color: #999ca0; font-size: 0.8125rem; line-height: 1.25rem; text-align: right; +.neuroglancer-skeleton-tool-shortcut { + display: inline-flex; + align-items: center; + gap: 0.5rem; + flex: 0 0 auto; + white-space: nowrap; +} + +.neuroglancer-skeleton-tool-shortcut-label { + color: #fff; +} + +.neuroglancer-skeleton-tool-shortcut-combo { + display: inline-flex; + align-items: center; + gap: 2px; + padding: 2px 4px; + border-radius: 0.25rem; + background: #333; + font-size: 0.75rem; + color: #999CA0; + line-height: 1rem; +} + +.neuroglancer-skeleton-tool-shortcut-icon { + display: inline-flex; + align-items: center; + min-width: 1rem !important; + max-height: 1rem; + min-height: 1rem; } /* Per-mode cursor indicators — driven by data-skeleton-edit-mode on the panel element */ diff --git a/src/ui/skeleton_edit_tools.ts b/src/ui/skeleton_edit_tools.ts index f381665b9..ff8ee75ce 100644 --- a/src/ui/skeleton_edit_tools.ts +++ b/src/ui/skeleton_edit_tools.ts @@ -62,20 +62,16 @@ import { StatusMessage } from "#src/status.js"; import { getDefaultSkeletonEditAuxBindings, getDefaultSkeletonEditNodeBindings, - getDefaultSkeletonEditToolBindings, -} from "#src/ui/default_input_event_bindings.js"; -import type { SpatialSkeletonToolStatusText } from "#src/ui/skeleton_edit_tool_messages.js"; -import { - SPATIAL_SKELETON_EDIT_TOOL_NAME, - getSpatialSkeletonCreateIdleStatusText, - getSpatialSkeletonCreatingStatusText, - getSpatialSkeletonDefaultStatusText, + getSpatialSkeletonDeleteIdleStatusText, getSpatialSkeletonDeletingStatusText, getSpatialSkeletonMergeStatusText, getSpatialSkeletonMovingStatusText, - getSpatialSkeletonSplitIdleStatusText, -} from "#src/ui/skeleton_edit_tool_messages.js"; +import { + SPATIAL_SKELETON_EDIT_TOOL_NAME, + renderSpatialSkeletonShortcut, +} from "#src/ui/skeleton_edit_tool_shortcuts.js"; +import type { SpatialSkeletonToolStatusText } from "#src/ui/skeleton_edit_tool_shortcuts.js"; import type { ToolActivation } from "#src/ui/tool.js"; import { LayerTool, @@ -149,22 +145,14 @@ function renderSpatialSkeletonToolStatus( body: HTMLElement, text: SpatialSkeletonToolStatusText, ) { - removeChildren(body); - body.classList.add("neuroglancer-skeleton-tool-status"); - const dividerElement = document.createElement("span"); - dividerElement.className = "neuroglancer-skeleton-tool-status-divider"; - dividerElement.textContent = "—"; - body.appendChild(dividerElement); - const statusElement = document.createElement("span"); - statusElement.className = "neuroglancer-skeleton-tool-status-text"; - statusElement.textContent = text.status; + body.appendChild(statusElement); if (text.actions.length === 0) { return; } - const actionsElement = document.createElement("span"); - actionsElement.className = "neuroglancer-skeleton-tool-status-actions"; - actionsElement.textContent = text.actions; + for (const shortcut of text.actions) { + actionsElement.appendChild(renderSpatialSkeletonShortcut(shortcut)); + } body.appendChild(actionsElement); } @@ -1395,17 +1383,13 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { ); if (disabledReason !== undefined) { StatusMessage.showTemporaryMessage(disabledReason); - renderSpatialSkeletonToolStatus(body, { - status: disabledReason, - actions: "", + actions: [], }); queueMicrotask(() => activation.cancel()); return; } if (this.getActiveSpatiallyIndexedSkeletonLayer() === undefined) { - const msg = "No spatially indexed skeleton source is currently loaded."; - StatusMessage.showTemporaryMessage(msg); - renderSpatialSkeletonToolStatus(body, { status: msg, actions: "" }); + renderSpatialSkeletonToolStatus(body, { status: msg, actions: [] }); queueMicrotask(() => activation.cancel()); return; } From ac9b44ffc8fcb5b20a4d75ed7c9e6ec84124e2b3 Mon Sep 17 00:00:00 2001 From: Afonso Pinto Date: Wed, 15 Jul 2026 16:09:42 +0100 Subject: [PATCH 06/11] Merge pull request from MetaCell/perf/skeleton-memory perf: improve skeleton memory management for faster move edits and less allocations per draw loop --- src/layer/index.ts | 4 + src/layer/segmentation/index.ts | 4 + src/picking_indicator_overlay.ts | 7 +- src/skeleton/frontend.ts | 436 ++++++++++++++++++++------- src/skeleton/segment_overlay.spec.ts | 22 ++ src/skeleton/segment_overlay.ts | 34 ++- src/ui/skeleton_edit_tools.ts | 19 +- 7 files changed, 398 insertions(+), 128 deletions(-) diff --git a/src/layer/index.ts b/src/layer/index.ts index e3c035924..be383f5e8 100644 --- a/src/layer/index.ts +++ b/src/layer/index.ts @@ -1156,6 +1156,10 @@ export class MouseSelectionState implements PickState { position: Float32Array = kEmptyFloat32Vec; unsnappedPosition: Float32Array = kEmptyFloat32Vec; active = false; + // When true, the global picking-indicator ring is hidden even though the mouse + // state is active. Set during a skeleton node move, where the on-screen node is + // driven by the drag preview rather than by picking. + pickingIndicatorSuppressed = false; displayDimensions: DisplayDimensions | undefined = undefined; pickedRenderLayer: RenderLayer | null = null; pickedValue = 0n; diff --git a/src/layer/segmentation/index.ts b/src/layer/segmentation/index.ts index 2e1b57ff8..eeb1d36bb 100644 --- a/src/layer/segmentation/index.ts +++ b/src/layer/segmentation/index.ts @@ -1672,6 +1672,8 @@ export class SegmentationUserLayer extends Base { this.spatialSkeletonState.pendingNodePositionVersion, getPendingNodePosition: (nodeId) => this.spatialSkeletonState.getPendingNodePosition(nodeId), + getPendingNodeIds: () => + this.spatialSkeletonState.getPendingNodeIds(), getCachedNode: (nodeId) => this.spatialSkeletonState.getCachedNode(nodeId), resolveGlobalPosition: (modelPosition) => @@ -1712,6 +1714,8 @@ export class SegmentationUserLayer extends Base { this.spatialSkeletonState.pendingNodePositionVersion, getPendingNodePosition: (nodeId) => this.spatialSkeletonState.getPendingNodePosition(nodeId), + getPendingNodeIds: () => + this.spatialSkeletonState.getPendingNodeIds(), getCachedNode: (nodeId) => this.spatialSkeletonState.getCachedNode(nodeId), resolveGlobalPosition: (modelPosition) => diff --git a/src/picking_indicator_overlay.ts b/src/picking_indicator_overlay.ts index 454bf4555..0ad070142 100644 --- a/src/picking_indicator_overlay.ts +++ b/src/picking_indicator_overlay.ts @@ -45,9 +45,10 @@ export class PickingIndicatorOverlay implements PanelOverlaySource { updatePanelOverlays(ctx: PanelOverlayContext): void { const { container } = ctx; const { mouseState } = this; - const pos = mouseState.active - ? ctx.project(mouseState.position) - : undefined; + const pos = + mouseState.active && !mouseState.pickingIndicatorSuppressed + ? ctx.project(mouseState.position) + : undefined; let element = container.firstElementChild as HTMLElement | null; if (pos === undefined) { if (element !== null) element.style.display = "none"; diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index fbfbdef78..d068280c5 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -388,6 +388,16 @@ class RenderHelper extends RefCounted { builder.addVarying("highp uint", "vPickID", "flat"); builder.addUniform("highp uint", "uPickInstanceStride"); this.defineAttributeAccess(builder); + // Live node drag: override a single vertex's position via a uniform instead + // of re-uploading the position texture. `uOverrideVertexIndex` is -1 when no + // node is being dragged (set in beginLayer), so this is a no-op fast path. + builder.addUniform("highp int", "uOverrideVertexIndex"); + builder.addUniform("highp vec3", "uOverridePosition"); + builder.addVertexCode(` +highp vec3 applyNodePositionOverride(highp uint vertexIndex, highp vec3 position) { + return (int(vertexIndex) == uOverrideVertexIndex) ? uOverridePosition : position; +} +`); if (skeletonParams.dynamicSegmentAppearance) { this.defineDynamicSegmentAppearance(builder, skeletonParams); } @@ -800,8 +810,8 @@ vec4 getSegmentAppearance(highp uint segmentValue) { let vertexMain = ` highp uint pickOffset = uint(gl_InstanceID) * uPickInstanceStride; vPickID = uPickID + pickOffset; -highp vec3 vertexA = readAttribute0(aVertexIndex.x); -highp vec3 vertexB = readAttribute0(aVertexIndex.y); +highp vec3 vertexA = applyNodePositionOverride(aVertexIndex.x, readAttribute0(aVertexIndex.x)); +highp vec3 vertexB = applyNodePositionOverride(aVertexIndex.y, readAttribute0(aVertexIndex.y)); emitLine(uProjection, vertexA, vertexB, uLineWidth); highp uint lineEndpointIndex = getLineEndpointIndex(); highp uint vertexIndex = aVertexIndex.x * (1u - lineEndpointIndex) + aVertexIndex.y * lineEndpointIndex; @@ -837,8 +847,8 @@ highp uint vertexIndex = aVertexIndex.x * (1u - lineEndpointIndex) + aVertexInde let vertexMain = ` highp uint pickOffset = uint(gl_InstanceID) * uPickInstanceStride; vPickID = uPickID + pickOffset; -highp vec3 vertexA = readAttribute0(aVertexIndex.x); -highp vec3 vertexB = readAttribute0(aVertexIndex.y); +highp vec3 vertexA = applyNodePositionOverride(aVertexIndex.x, readAttribute0(aVertexIndex.x)); +highp vec3 vertexB = applyNodePositionOverride(aVertexIndex.y, readAttribute0(aVertexIndex.y)); highp uint vertexIndex = aVertexIndex.x; highp vec3 edgeMidpoint = mix(vertexA, vertexB, 0.5); highp float edgeRadius = getRaycastModelRadiusForPixels(edgeMidpoint, uEdgePixelRadius); @@ -878,7 +888,7 @@ vec4 finishNodeColor(vec4 color) { highp uint vertexIndex = uint(gl_InstanceID); highp uint pickOffset = vertexIndex * uPickInstanceStride; vPickID = uPickID + pickOffset; -highp vec3 vertexPosition = readAttribute0(vertexIndex); +highp vec3 vertexPosition = applyNodePositionOverride(vertexIndex, readAttribute0(vertexIndex)); `; let fragmentSetup = ""; if (skeletonParams.spatialChunkCulling) { @@ -907,7 +917,7 @@ vec4 finishNodeColor(vec4 color) { highp uint vertexIndex = uint(gl_InstanceID); highp uint pickOffset = vertexIndex * uPickInstanceStride; vPickID = uPickID + pickOffset; -highp vec3 vertexPosition = readAttribute0(vertexIndex); +highp vec3 vertexPosition = applyNodePositionOverride(vertexIndex, readAttribute0(vertexIndex)); highp float nodeRadius = getRaycastModelRadiusForPixels(vertexPosition, uNodePixelRadius); `; vertexMain += this.readSegmentValueGlsl(skeletonParams, "vertexIndex"); @@ -1000,6 +1010,10 @@ highp float nodeRadius = getRaycastModelRadiusForPixels(vertexPosition, uNodePix tempLightVec[3] = perspectiveContext.ambientLighting; gl.uniform4fv(shader.uniform("uLightDirection"), tempLightVec); } + // Default: no live-drag position override. Must be set for every pass — + // including the shared browse pass — since a uniform left at its 0 default + // would wrongly override vertex 0. The overlay pass sets a real value below. + gl.uniform1i(shader.uniform("uOverrideVertexIndex"), -1); this.vertexIdHelper.enable(); } @@ -1007,6 +1021,25 @@ highp float nodeRadius = getRaycastModelRadiusForPixels(vertexPosition, uNodePix gl.uniform4fv(shader.uniform("uColor"), color); } + // Overrides a single vertex's position (a live node drag) without touching the + // position texture. `vertexIndex < 0` disables the override. + setNodePositionOverride( + gl: GL, + shader: ShaderProgram, + vertexIndex: number, + position: ArrayLike, + ) { + gl.uniform1i(shader.uniform("uOverrideVertexIndex"), vertexIndex); + if (vertexIndex >= 0) { + gl.uniform3f( + shader.uniform("uOverridePosition"), + Number(position[0] ?? 0), + Number(position[1] ?? 0), + Number(position[2] ?? 0), + ); + } + } + setPickID(gl: GL, shader: ShaderProgram, pickID: number) { gl.uniform1ui(shader.uniform("uPickID"), pickID); } @@ -1988,6 +2021,8 @@ interface SpatiallyIndexedSkeletonLayerOptions { >; pendingNodePositionVersion?: WatchableValueInterface; getPendingNodePosition?: (nodeId: number) => ArrayLike | undefined; + // Node ids that currently have a pending (drag) position override. + getPendingNodeIds?: () => Iterable; getCachedNode?: (nodeId: number) => SpatiallyIndexedSkeletonNode | undefined; // Transforms a node's model-space position into the global coordinate space // used by the panels, so node highlights can be projected to screen. @@ -2020,6 +2055,9 @@ class SkeletonOverlayChunk implements SkeletonGPUGeometry { readonly pickNodePositions: Float32Array; readonly pickSegmentIds: Uint32Array; readonly pickEdgeSegmentIds: Uint32Array; + // Maps nodeId to packed vertex index, used to target a live node drag's + // position override to the correct vertex via a shader uniform (no rebuild). + readonly nodeIndex: ReadonlyMap; constructor( gl: GL, @@ -2060,6 +2098,7 @@ class SkeletonOverlayChunk implements SkeletonGPUGeometry { this.pickNodePositions = geometry.positions; this.pickSegmentIds = geometry.pickSegmentIds; this.pickEdgeSegmentIds = geometry.pickEdgeSegmentIds; + this.nodeIndex = geometry.nodeIndex; } dispose(gl: GL) { @@ -2080,67 +2119,6 @@ const seenChunkKeysPerFrame = new WeakMap< const SPATIAL_SKELETON_RESOLUTION_INDICATOR_BAR_HEIGHT = 10; -function updateSpatialSkeletonSpacingHistogram( - histogram: RenderScaleHistogram, - frameNumber: number, - transformedSources: readonly TransformedSource[][], - projectionParameters: ProjectionParameters, - localPosition: Float32Array, - spacingTarget: number, -) { - histogram.begin(frameNumber); - if (transformedSources.length === 0) { - return; - } - let seen = seenChunkKeysPerFrame.get(histogram); - if (seen === undefined || seen.frameNumber !== frameNumber) { - seen = { frameNumber, keys: new Set() }; - seenChunkKeysPerFrame.set(histogram, seen); - } - const seenKeys = seen.keys; - for (const scales of transformedSources) { - forEachSpatialSkeletonSourceScale( - projectionParameters, - spacingTarget, - scales, - (tsource, _, physicalSpacing, pixelSpacing, selected) => { - if (selected) return; - const source = tsource.source as SpatiallyIndexedSkeletonSource; - const indicatorKey = `indicator:${getObjectId(source)}`; - if (seenKeys.has(indicatorKey)) return; - seenKeys.add(indicatorKey); - histogram.add( - physicalSpacing, - pixelSpacing, - 0, - SPATIAL_SKELETON_RESOLUTION_INDICATOR_BAR_HEIGHT, - true, - ); - }, - ); - forEachVisibleSpatialSkeletonChunk( - projectionParameters, - localPosition, - spacingTarget, - scales, - () => {}, - (tsource, _, physicalSpacing, pixelSpacing) => { - const source = tsource.source as SpatiallyIndexedSkeletonSource; - const chunkKey = tsource.curPositionInChunks.join(); - const seenKey = `${getObjectId(source)}:${chunkKey}`; - if (seenKeys.has(seenKey)) return; - seenKeys.add(seenKey); - const chunk = source.chunks.get(chunkKey); - if (chunk?.state === ChunkState.GPU_MEMORY) { - histogram.add(physicalSpacing, pixelSpacing, 1, 0); - } else { - histogram.add(physicalSpacing, pixelSpacing, 0, 1); - } - }, - ); - } -} - export interface SpatiallyIndexedSkeletonLayerDisplayState extends SkeletonLayerDisplayState { spatialSkeletonSpacingTarget2d: WatchableValueInterface; @@ -2219,12 +2197,12 @@ export class SpatiallyIndexedSkeletonLayer private hoveredNodeInfo: | WatchableValueInterface | undefined; - private pendingNodePositionVersion: - | WatchableValueInterface - | undefined; private getPendingNodePositionOverride: | ((nodeId: number) => ArrayLike | undefined) | undefined; + // Node ids with a live pending (drag) position. Used to target the shader + // position override without scanning all nodes; one entry during a drag. + private getPendingNodeIds: (() => Iterable) | undefined; private getCachedNodeInfo: | ((nodeId: number) => SpatiallyIndexedSkeletonNode | undefined) | undefined; @@ -2236,14 +2214,29 @@ export class SpatiallyIndexedSkeletonLayer readonly highlightMarkersChanged = new NullarySignal(); private inspectionState: SpatiallyIndexedSkeletonInspectionState | undefined; private overlayChunk: SkeletonOverlayChunk | undefined; - private overlayGeometryKey: string | undefined; + // Identifies the overlay geometry topology (which segments are loaded plus the + // node-data version). A change forces a full rebuild. Live-drag position + // changes do not affect it — they are applied per-draw via a shader uniform. + private overlayTopologyKey: string | undefined; private overlayRebuildFrame = -1; private pendingOverlaySegmentLoads = new Set(); private browseExcludedSegments = new Uint64Set(); private gpuBrowseExcludedSegmentsHashTable: GPUHashTable; private browseExcludedSegmentsKey: string | undefined; private readonly editedSegmentIds = new Set(); + // Bumped on every mutation of `editedSegmentIds` so the per-frame browse + // excluded-segments computation can be skipped when nothing changed. + private editedSegmentIdsVersion = 0; + private cachedBrowseExcludedResult: Uint64Set | undefined; + private cachedBrowseExcludedVersion = -1; private retainedOverlaySegmentIds: number[] = []; + // Bumped whenever `retainedOverlaySegmentIds` is replaced, so the merged + // overlay render segment-id list can be cached across frames. + private retainedOverlaySegmentIdsVersion = 0; + private cachedOverlayRenderSegmentIds: number[] = []; + private cachedOverlayRenderVisibleSet: Uint64Set | undefined; + private cachedOverlayRenderVisibleGeneration = -1; + private cachedOverlayRenderRetainedVersion = -1; private maxRetainedOverlaySegments: number; private readonly selectedNodeOutlineColor = vec3.clone( ACTIVE_NODE_BORDER_FALLBACK_COLOR, @@ -2257,9 +2250,63 @@ export class SpatiallyIndexedSkeletonLayer private cachedNodeOutlineColorGeneration = -1; private disposeOverlayChunk() { + const changed = + this.overlayChunk !== undefined || this.overlayTopologyKey !== undefined; this.overlayChunk?.dispose(this.gl); this.overlayChunk = undefined; - this.overlayGeometryKey = undefined; + this.overlayTopologyKey = undefined; + return changed; + } + + getUniqueChunkSources() { + const sources = new Set(); + for (const sourceEntry of [...this.sources, ...this.sources2d]) { + sources.add(sourceEntry.chunkSource); + } + return sources; + } + + private clearOverlayRuntimeState() { + let changed = this.disposeOverlayChunk(); + if (this.pendingOverlaySegmentLoads.size !== 0) { + this.pendingOverlaySegmentLoads.clear(); + changed = true; + } + if (this.editedSegmentIds.size !== 0) { + this.editedSegmentIds.clear(); + ++this.editedSegmentIdsVersion; + changed = true; + } + if (this.retainedOverlaySegmentIds.length !== 0) { + this.retainedOverlaySegmentIds = []; + ++this.retainedOverlaySegmentIdsVersion; + changed = true; + } + if (this.browseExcludedSegments.size !== 0) { + this.browseExcludedSegments.clear(); + changed = true; + } + if (this.browseExcludedSegmentsKey !== undefined) { + this.browseExcludedSegmentsKey = undefined; + changed = true; + } + this.overlayRebuildFrame = -1; + return changed; + } + + disposeRuntimeState( + options: SpatiallyIndexedSkeletonSourceRuntimeDisposalOptions = {}, + ) { + const overlayChanged = this.clearOverlayRuntimeState(); + const sourceChanged = disposeSpatiallyIndexedSkeletonSourceRuntimeState( + this.getUniqueChunkSources(), + options, + ); + const changed = overlayChanged || sourceChanged; + if (changed) { + this.redrawNeeded.dispatch(); + } + return changed; } private requestOverlaySegmentLoad(segmentId: number) { @@ -2280,10 +2327,9 @@ export class SpatiallyIndexedSkeletonLayer }); } - private getOverlayGeometryKey(segmentIds: readonly number[]) { + private getOverlayTopologyKey(segmentIds: readonly number[]) { return [ segmentIds.join(","), - `pending:${this.pendingNodePositionVersion?.value ?? ""}`, `data:${this.inspectionState?.nodeDataVersion.value ?? ""}`, ].join("|"); } @@ -2401,6 +2447,7 @@ export class SpatiallyIndexedSkeletonLayer return false; } this.retainedOverlaySegmentIds = nextRetainedOverlaySegmentIds; + ++this.retainedOverlaySegmentIdsVersion; this.redrawNeeded.dispatch(); return true; } @@ -2415,15 +2462,37 @@ export class SpatiallyIndexedSkeletonLayer return false; } this.editedSegmentIds.add(normalizedSegmentId); + ++this.editedSegmentIdsVersion; this.redrawNeeded.dispatch(); return true; } private getOverlayRenderSegmentIds() { - return mergeSpatiallyIndexedSkeletonOverlaySegmentIds( + // The merged list depends only on the visible-segment set and the retained + // list; both expose a cheap version (the hash-table generation and a counter + // bumped on replacement), so the sort/merge can be skipped when unchanged. + const visibleSet = getVisibleSegments( + this.displayState.segmentationGroupState.value, + ); + const visibleGeneration = visibleSet.hashTable.generation; + if ( + this.cachedOverlayRenderVisibleSet === visibleSet && + this.cachedOverlayRenderVisibleGeneration === visibleGeneration && + this.cachedOverlayRenderRetainedVersion === + this.retainedOverlaySegmentIdsVersion + ) { + return this.cachedOverlayRenderSegmentIds; + } + const result = mergeSpatiallyIndexedSkeletonOverlaySegmentIds( this.getActiveEditableSegmentIds(), this.retainedOverlaySegmentIds, ); + this.cachedOverlayRenderVisibleSet = visibleSet; + this.cachedOverlayRenderVisibleGeneration = visibleGeneration; + this.cachedOverlayRenderRetainedVersion = + this.retainedOverlaySegmentIdsVersion; + this.cachedOverlayRenderSegmentIds = result; + return result; } private getNormalizedBrowsePassExcludedSegmentIds() { @@ -2431,12 +2500,20 @@ export class SpatiallyIndexedSkeletonLayer } private getBrowsePassExcludedSegments() { + // Called once per browse pass per panel per frame. `editedSegmentIds` only + // changes on edit operations, so skip the sort/join/set rebuild entirely + // while it is unchanged. + if (this.cachedBrowseExcludedVersion === this.editedSegmentIdsVersion) { + return this.cachedBrowseExcludedResult; + } + this.cachedBrowseExcludedVersion = this.editedSegmentIdsVersion; const segmentIds = this.getNormalizedBrowsePassExcludedSegmentIds(); if (segmentIds.length === 0) { if (this.browseExcludedSegments.size !== 0) { this.browseExcludedSegments.clear(); } this.browseExcludedSegmentsKey = undefined; + this.cachedBrowseExcludedResult = undefined; return undefined; } const excludedSegmentsKey = segmentIds.join(","); @@ -2451,6 +2528,7 @@ export class SpatiallyIndexedSkeletonLayer ); this.browseExcludedSegmentsKey = excludedSegmentsKey; } + this.cachedBrowseExcludedResult = this.browseExcludedSegments; return this.browseExcludedSegments; } @@ -2489,17 +2567,20 @@ export class SpatiallyIndexedSkeletonLayer return undefined; } - const overlayGeometryKey = this.getOverlayGeometryKey(loadedSegmentIds); + const topologyKey = this.getOverlayTopologyKey(loadedSegmentIds); - if (this.overlayChunk !== undefined) { - if (this.overlayGeometryKey === overlayGeometryKey) { - // Geometry unchanged — selection/hover highlights are DOM overlays, so no - // GPU rebuild is needed when selection changes. - return this.overlayChunk; - } + if ( + this.overlayChunk !== undefined && + this.overlayTopologyKey === topologyKey + ) { + // Topology unchanged, so no rebuild. Live node-drag position changes are + // applied per-draw via a shader uniform (see applyOverlayNodePositionOverride), + // and selection/hover highlights are DOM overlays — none of these rebuild + // the GPU geometry. + return this.overlayChunk; } - // Geometry cache miss — collect node sets and rebuild. + // Topology cache miss — collect node sets and rebuild. const segmentNodeSets: (readonly SpatiallyIndexedSkeletonNode[])[] = []; for (const segmentId of loadedSegmentIds) { const segmentNodes = @@ -2518,7 +2599,7 @@ export class SpatiallyIndexedSkeletonLayer geometry, this.overlayAttributeTextureFormats, ); - this.overlayGeometryKey = overlayGeometryKey; + this.overlayTopologyKey = topologyKey; return this.overlayChunk; } @@ -2579,8 +2660,8 @@ export class SpatiallyIndexedSkeletonLayer this.selectedNodeInfo = options.selectedNodeInfo; this.suppressSelectedNodeHighlight = options.suppressSelectedNodeHighlight; this.hoveredNodeInfo = options.hoveredNodeInfo; - this.pendingNodePositionVersion = options.pendingNodePositionVersion; this.getPendingNodePositionOverride = options.getPendingNodePosition; + this.getPendingNodeIds = options.getPendingNodeIds; this.getCachedNodeInfo = options.getCachedNode; this.resolveGlobalPosition = options.resolveGlobalPosition; this.inspectionState = options.inspectionState; @@ -3061,24 +3142,94 @@ export class SpatiallyIndexedSkeletonLayer } } - getVisibleChunksInCurrentView( + // Walks the visible chunk set once per panel per frame: collects GPU-resident + // chunks into the reused `out` array (pooled to avoid per-frame allocation of a + // fresh array plus one object per visible chunk) and updates the resolution + // histogram (present/absent bars plus unselected-scale indicator bars) in the + // same traversal, rather than walking the chunk set a second time. Returns + // `out`, whose length is set to the number of collected chunks. + updateVisibleChunksAndHistogram( transformedSources: readonly TransformedSource[][], projectionParameters: ProjectionParameters, spacingTarget: number, + histogram: RenderScaleHistogram, + frameNumber: number, + out: VisibleChunk[], ): VisibleChunk[] { - const result: VisibleChunk[] = []; - this.forEachVisibleChunkSlot( - transformedSources, - projectionParameters, - spacingTarget, - (chunkKey, chunkSource, chunkLayout) => { - const chunk = chunkSource.chunks.get(chunkKey); - if (chunk?.state === ChunkState.GPU_MEMORY) { - result.push({ chunk, chunkLayout }); - } - }, - ); - return result; + histogram.begin(frameNumber); + let count = 0; + if (transformedSources.length === 0) { + out.length = 0; + return out; + } + let seen = seenChunkKeysPerFrame.get(histogram); + if (seen === undefined || seen.frameNumber !== frameNumber) { + seen = { frameNumber, keys: new Set() }; + seenChunkKeysPerFrame.set(histogram, seen); + } + const seenKeys = seen.keys; + const localPosition = this.localPosition.value; + for (const scales of transformedSources) { + forEachSpatialSkeletonSourceScale( + projectionParameters, + spacingTarget, + scales, + (tsource, _, physicalSpacing, pixelSpacing, selected) => { + if (selected) return; + const source = tsource.source as SpatiallyIndexedSkeletonSource; + const indicatorKey = `indicator:${getObjectId(source)}`; + if (seenKeys.has(indicatorKey)) return; + seenKeys.add(indicatorKey); + histogram.add( + physicalSpacing, + pixelSpacing, + 0, + SPATIAL_SKELETON_RESOLUTION_INDICATOR_BAR_HEIGHT, + true, + ); + }, + ); + forEachVisibleSpatialSkeletonChunk( + projectionParameters, + localPosition, + spacingTarget, + scales, + () => {}, + (tsource, _, physicalSpacing, pixelSpacing) => { + const source = tsource.source as SpatiallyIndexedSkeletonSource; + const chunkKey = tsource.curPositionInChunks.join(); + const chunk = source.chunks.get(chunkKey); + const isGpuResident = chunk?.state === ChunkState.GPU_MEMORY; + if (isGpuResident) { + // Collect for drawing. Pooled: reuse existing entry objects in place, + // growing the array only when this frame has more chunks than the last. + const entry = out[count]; + if (entry === undefined) { + out[count] = { chunk: chunk!, chunkLayout: tsource.chunkLayout }; + } else { + entry.chunk = chunk!; + entry.chunkLayout = tsource.chunkLayout; + } + ++count; + } + // Histogram present/absent accounting is deduplicated across panels via + // the per-frame seen set so a chunk visible in more than one panel is + // counted once; the draw list above is intentionally per-panel and not + // deduplicated. + const seenKey = `${getObjectId(source)}:${chunkKey}`; + if (!seenKeys.has(seenKey)) { + seenKeys.add(seenKey); + if (isGpuResident) { + histogram.add(physicalSpacing, pixelSpacing, 1, 0); + } else { + histogram.add(physicalSpacing, pixelSpacing, 0, 1); + } + } + }, + ); + } + out.length = count; + return out; } private areVisibleChunksReady( @@ -3371,6 +3522,49 @@ export class SpatiallyIndexedSkeletonLayer ); } + // Sets the shader position override to the one node currently being dragged + // (if any) that belongs to `overlayChunk`. Reads the live pending state each + // draw; leaves the override disabled (as beginLayer set it) when no dragged + // node maps into this chunk. + private applyOverlayNodePositionOverride( + gl: GL, + renderHelper: RenderHelper, + edgeShader: ShaderProgram, + nodeShader: ShaderProgram, + overlayChunk: SkeletonOverlayChunk, + ) { + const getPendingNodeIds = this.getPendingNodeIds; + const getPendingNodePosition = this.getPendingNodePositionOverride; + if ( + getPendingNodeIds === undefined || + getPendingNodePosition === undefined + ) { + return; + } + for (const nodeId of getPendingNodeIds()) { + const vertexIndex = overlayChunk.nodeIndex.get(nodeId); + if (vertexIndex === undefined) continue; + const position = getPendingNodePosition(nodeId); + if (position === undefined) continue; + edgeShader.bind(); + renderHelper.setNodePositionOverride( + gl, + edgeShader, + vertexIndex, + position, + ); + nodeShader.bind(); + renderHelper.setNodePositionOverride( + gl, + nodeShader, + vertexIndex, + position, + ); + // Exactly one node is dragged at a time. + return; + } + } + private drawInspectionOverlayPass( renderContext: SliceViewPanelRenderContext | PerspectiveViewRenderContext, layer: RenderLayer, @@ -3442,6 +3636,18 @@ export class SpatiallyIndexedSkeletonLayer ); } + // Live node drag: override just the moving vertex's position via a uniform, + // instead of re-uploading the position texture. `beginSkeletonRenderPass` + // left the override disabled (-1); set it here for the one dragged node that + // belongs to this overlay chunk. Exactly one node moves at a time. + this.applyOverlayNodePositionOverride( + gl, + renderHelper, + edgeShader, + nodeShader, + overlayChunk, + ); + renderHelper.drawSkeletons( gl, edgeShader, @@ -3666,6 +3872,9 @@ export class PerspectiveViewSpatiallyIndexedSkeletonLayer private browseRenderHelper: RenderHelper; private renderOptions: ViewSpecificSkeletonRenderingOptions; transformedSources: TransformedSource[][] = []; + // Reused across frames to avoid allocating a fresh array plus one object per + // visible chunk on every draw. Consumed synchronously within draw(). + private readonly visibleChunksScratch: VisibleChunk[] = []; backend: ChunkRenderLayerFrontend; constructor(public base: SpatiallyIndexedSkeletonLayer) { @@ -3769,21 +3978,16 @@ export class PerspectiveViewSpatiallyIndexedSkeletonLayer } const { displayState } = this.base; const spacingTarget = displayState.spatialSkeletonSpacingTarget3d.value; - const visibleChunks = this.base.getVisibleChunksInCurrentView( - this.transformedSources, - renderContext.projectionParameters, - spacingTarget, - ); const histogram = displayState.spatialSkeletonSpacingHistogram3d; const frameNumber = this.base.chunkManager.chunkQueueManager.frameNumberCounter.frameNumber; - updateSpatialSkeletonSpacingHistogram( - histogram, - frameNumber, + const visibleChunks = this.base.updateVisibleChunksAndHistogram( this.transformedSources, renderContext.projectionParameters, - this.base.localPosition.value, spacingTarget, + histogram, + frameNumber, + this.visibleChunksScratch, ); const modelMatrix = update3dRenderLayerAttachment( displayState.transform.value, @@ -3861,6 +4065,9 @@ export class SliceViewPanelSpatiallyIndexedSkeletonLayer private browseRenderHelper: RenderHelper; private renderOptions: ViewSpecificSkeletonRenderingOptions; transformedSources: TransformedSource[][] = []; + // Reused across frames to avoid allocating a fresh array plus one object per + // visible chunk on every draw. Consumed synchronously within draw(). + private readonly visibleChunksScratch: VisibleChunk[] = []; backend: ChunkRenderLayerFrontend; constructor(public base: SpatiallyIndexedSkeletonLayer) { super(); @@ -3951,21 +4158,16 @@ export class SliceViewPanelSpatiallyIndexedSkeletonLayer ) { const { displayState } = this.base; const spacingTarget = displayState.spatialSkeletonSpacingTarget2d.value; - const visibleChunks = this.base.getVisibleChunksInCurrentView( - this.transformedSources, - renderContext.sliceView.projectionParameters.value, - spacingTarget, - ); const histogram = displayState.spatialSkeletonSpacingHistogram2d; const frameNumber = this.base.chunkManager.chunkQueueManager.frameNumberCounter.frameNumber; - updateSpatialSkeletonSpacingHistogram( - histogram, - frameNumber, + const visibleChunks = this.base.updateVisibleChunksAndHistogram( this.transformedSources, renderContext.sliceView.projectionParameters.value, - this.base.localPosition.value, spacingTarget, + histogram, + frameNumber, + this.visibleChunksScratch, ); const modelMatrix = update3dRenderLayerAttachment( displayState.transform.value, diff --git a/src/skeleton/segment_overlay.spec.ts b/src/skeleton/segment_overlay.spec.ts index c49532db5..453951afe 100644 --- a/src/skeleton/segment_overlay.spec.ts +++ b/src/skeleton/segment_overlay.spec.ts @@ -67,6 +67,28 @@ describe("buildSpatiallyIndexedSkeletonOverlayGeometry", () => { expect([...geometry.indices]).toEqual([1, 0]); expect([...geometry.pickEdgeSegmentIds]).toEqual([11]); }); + + it("returns a nodeIndex map aligned with the packed vertex order", () => { + const geometry = buildSpatiallyIndexedSkeletonOverlayGeometry([ + [ + { nodeId: 5, segmentId: 11, position: new Float32Array([1, 2, 3]) }, + { + nodeId: 6, + segmentId: 11, + position: new Float32Array([4, 5, 6]), + parentNodeId: 5, + }, + ], + // Duplicate of node 5 in a second segment must not create a new vertex. + [{ nodeId: 5, segmentId: 13, position: new Float32Array([9, 9, 9]) }], + ]); + expect(geometry.numVertices).toBe(2); + // The nodeIndex maps each nodeId to its packed vertex index, so a live drag + // can override the right vertex's position via a shader uniform. + expect(geometry.nodeIndex.get(5)).toBe(0); + expect(geometry.nodeIndex.get(6)).toBe(1); + expect([...geometry.nodeIds]).toEqual([5, 6]); + }); }); describe("mergeSpatiallyIndexedSkeletonOverlaySegmentIds", () => { diff --git a/src/skeleton/segment_overlay.ts b/src/skeleton/segment_overlay.ts index 822875f23..0665c054a 100644 --- a/src/skeleton/segment_overlay.ts +++ b/src/skeleton/segment_overlay.ts @@ -58,6 +58,29 @@ export interface SpatiallyIndexedSkeletonOverlayGeometry { pickEdgeSegmentIds: Uint32Array; indices: Uint32Array; numVertices: number; + // Maps nodeId to its packed vertex index. Retained by the overlay chunk so a + // live node drag can override just the moving vertex's position via a shader + // uniform, rather than rebuilding or re-uploading the geometry. + nodeIndex: ReadonlyMap; +} + +// Writes xyz node positions (one vertex per node, in `orderedNodes` order) into +// `positions`, applying any pending (dragged) position override so the built +// texture is correct at build time. Live-drag position changes between builds +// are applied at render time via a shader uniform, not here. +function writeSpatiallyIndexedSkeletonOverlayNodePositions( + orderedNodes: readonly SpatiallyIndexedSkeletonOverlayNodeLike[], + positions: Float32Array, + getPendingNodePosition?: (nodeId: number) => ArrayLike | undefined, +) { + for (let index = 0; index < orderedNodes.length; ++index) { + const node = orderedNodes[index]; + const position = getPendingNodePosition?.(node.nodeId) ?? node.position; + const baseOffset = index * 3; + positions[baseOffset] = Number(position[0] ?? 0); + positions[baseOffset + 1] = Number(position[1] ?? 0); + positions[baseOffset + 2] = Number(position[2] ?? 0); + } } export function buildSpatiallyIndexedSkeletonOverlayGeometry( @@ -92,12 +115,12 @@ export function buildSpatiallyIndexedSkeletonOverlayGeometry( const scratch = ensureGpuScratch(numVertices); const { segmentIds, edgeIndices, edgeSegIds } = scratch; + writeSpatiallyIndexedSkeletonOverlayNodePositions( + orderedNodes, + positions, + getPendingNodePosition, + ); orderedNodes.forEach((node, index) => { - const position = getPendingNodePosition?.(node.nodeId) ?? node.position; - const baseOffset = index * 3; - positions[baseOffset] = Number(position[0] ?? 0); - positions[baseOffset + 1] = Number(position[1] ?? 0); - positions[baseOffset + 2] = Number(position[2] ?? 0); segmentIds[index] = Math.max(0, Math.round(Number(node.segmentId))); pickSegmentIds[index] = segmentIds[index]; nodeIds[index] = Math.round(Number(node.nodeId)); @@ -134,6 +157,7 @@ export function buildSpatiallyIndexedSkeletonOverlayGeometry( // Subarray view: consumed immediately by GLBuffer.fromData. indices: edgeIndices.subarray(0, edgeCount * 2), numVertices, + nodeIndex, }; } diff --git a/src/ui/skeleton_edit_tools.ts b/src/ui/skeleton_edit_tools.ts index ff8ee75ce..3c46920a8 100644 --- a/src/ui/skeleton_edit_tools.ts +++ b/src/ui/skeleton_edit_tools.ts @@ -810,7 +810,7 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { return; } dragStarted = true; - this.dragInProgress = true; + this.setNodeMoveActive(true); skeletonLayer!.markSegmentEdited(nodeInfo!.segmentId); panel.element.dataset.skeletonPressMode = "move"; this.setStatus(getSpatialSkeletonMovingStatusText()); @@ -846,7 +846,7 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { if (finished) return; finished = true; if (this.dragInProgress) { - this.dragInProgress = false; + this.setNodeMoveActive(false); delete panel.element.dataset.skeletonPressMode; this.clearStatus(); } @@ -1352,13 +1352,26 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { }); } + // Keeps `dragInProgress` and the global picking-indicator suppression in + // lockstep. While a node is being moved, the on-screen node is driven by the + // drag preview (a shader uniform), not by picking, so the picking-indicator + // ring — which tracks the (now stale) pick buffer — is hidden. + private setNodeMoveActive(active: boolean) { + this.dragInProgress = active; + const { mouseState } = this; + if (mouseState.pickingIndicatorSuppressed !== active) { + mouseState.pickingIndicatorSuppressed = active; + mouseState.changed.dispatch(); + } + } + activate(activation: ToolActivation) { const { layer } = this; const rawInputEventMapBinder = activation.inputEventMapBinder; // 1. Reset all activation-scoped state. this.currentMode = SkeletonEditMode.Default; - this.dragInProgress = false; + this.setNodeMoveActive(false); this.pending = false; this.createPlacedThisHold = false; this.mergeKeyHeld = false; From 8f62c3553f97e193bb2eadc8b63d13fcb5f4c233 Mon Sep 17 00:00:00 2001 From: Afonso Pinto Date: Wed, 22 Jul 2026 14:58:13 +0100 Subject: [PATCH 07/11] Merge pull request from MetaCell/fix/overlay-pool-recency fix: change overlay retained segs to be longest ago edited is first out --- src/skeleton/frontend.spec.ts | 2 + src/skeleton/frontend.ts | 84 ++++++++++++++++++---------- src/skeleton/segment_overlay.spec.ts | 31 ++++++---- src/skeleton/segment_overlay.ts | 49 +++++++++++----- 4 files changed, 114 insertions(+), 52 deletions(-) diff --git a/src/skeleton/frontend.spec.ts b/src/skeleton/frontend.spec.ts index 9d9c26f62..16882f52f 100644 --- a/src/skeleton/frontend.spec.ts +++ b/src/skeleton/frontend.spec.ts @@ -394,6 +394,8 @@ describe("SpatiallyIndexedSkeletonLayer browse exclusions", () => { editedSegmentIds: new Set(), browseExcludedSegments: new Uint64Set(), browseExcludedSegmentsKey: undefined, + retainedOverlaySegments: new Map(), + overlaySegmentTouchCounter: 0, redrawNeeded: { dispatch: vi.fn() }, }, ); diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index d068280c5..95e4e2443 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -2229,9 +2229,14 @@ export class SpatiallyIndexedSkeletonLayer private editedSegmentIdsVersion = 0; private cachedBrowseExcludedResult: Uint64Set | undefined; private cachedBrowseExcludedVersion = -1; - private retainedOverlaySegmentIds: number[] = []; - // Bumped whenever `retainedOverlaySegmentIds` is replaced, so the merged - // overlay render segment-id list can be cached across frames. + // Segment id -> last-touched sequence number; doubles as pool membership + // (key) and recency (value). + private retainedOverlaySegments: Map = new Map(); + // Shared sequence counter assigned to each touch. + private overlaySegmentTouchCounter = 0; + // Bumped when the set of keys in `retainedOverlaySegments` changes, so the + // merged render segment-id list can be cached across frames. Recency-only + // touches don't bump this, since the merged, sorted list is unaffected. private retainedOverlaySegmentIdsVersion = 0; private cachedOverlayRenderSegmentIds: number[] = []; private cachedOverlayRenderVisibleSet: Uint64Set | undefined; @@ -2277,8 +2282,8 @@ export class SpatiallyIndexedSkeletonLayer ++this.editedSegmentIdsVersion; changed = true; } - if (this.retainedOverlaySegmentIds.length !== 0) { - this.retainedOverlaySegmentIds = []; + if (this.retainedOverlaySegments.size !== 0) { + this.retainedOverlaySegments = new Map(); ++this.retainedOverlaySegmentIdsVersion; changed = true; } @@ -2425,46 +2430,69 @@ export class SpatiallyIndexedSkeletonLayer } getRetainedOverlaySegmentIds() { - return this.retainedOverlaySegmentIds; + return [...this.retainedOverlaySegments.keys()]; } - retainOverlaySegment(segmentId: number) { - this.markSegmentEdited(segmentId); - const nextRetainedOverlaySegmentIds = - retainSpatiallyIndexedSkeletonOverlaySegment( - this.retainedOverlaySegmentIds, - segmentId, - { maxRetained: this.maxRetainedOverlaySegments }, - ); + /** + * Stores `nextRetainedOverlaySegments` and reports whether the set of keys + * changed. A recency-only touch still updates the stored map, but only a + * membership change bumps `retainedOverlaySegmentIdsVersion` and warrants + * a redraw. Shared by `retainOverlaySegment` and `markSegmentEdited`. + */ + private applyRetainedOverlaySegments( + nextRetainedOverlaySegments: Map, + ): boolean { + const previousRetainedOverlaySegments = this.retainedOverlaySegments; + this.retainedOverlaySegments = nextRetainedOverlaySegments; if ( - nextRetainedOverlaySegmentIds.length === - this.retainedOverlaySegmentIds.length && - nextRetainedOverlaySegmentIds.every( - (candidateSegmentId, index) => - candidateSegmentId === this.retainedOverlaySegmentIds[index], + nextRetainedOverlaySegments.size === + previousRetainedOverlaySegments.size && + [...nextRetainedOverlaySegments.keys()].every((candidateSegmentId) => + previousRetainedOverlaySegments.has(candidateSegmentId), ) ) { return false; } - this.retainedOverlaySegmentIds = nextRetainedOverlaySegmentIds; ++this.retainedOverlaySegmentIdsVersion; - this.redrawNeeded.dispatch(); return true; } + retainOverlaySegment(segmentId: number) { + return this.markSegmentEdited(segmentId); + } + markSegmentEdited(segmentId: number) { const normalizedSegmentId = Math.round(Number(segmentId)); if ( !Number.isSafeInteger(normalizedSegmentId) || - normalizedSegmentId <= 0 || - this.editedSegmentIds.has(normalizedSegmentId) + normalizedSegmentId <= 0 ) { return false; } - this.editedSegmentIds.add(normalizedSegmentId); - ++this.editedSegmentIdsVersion; - this.redrawNeeded.dispatch(); - return true; + let changed = false; + if (!this.editedSegmentIds.has(normalizedSegmentId)) { + this.editedSegmentIds.add(normalizedSegmentId); + ++this.editedSegmentIdsVersion; + changed = true; + } + // Refresh recency on every edit, not just the first, so a segment under + // continuous editing doesn't age out of the pool between retains. + if ( + this.applyRetainedOverlaySegments( + retainSpatiallyIndexedSkeletonOverlaySegment( + this.retainedOverlaySegments, + normalizedSegmentId, + ++this.overlaySegmentTouchCounter, + { maxRetained: this.maxRetainedOverlaySegments }, + ), + ) + ) { + changed = true; + } + if (changed) { + this.redrawNeeded.dispatch(); + } + return changed; } private getOverlayRenderSegmentIds() { @@ -2485,7 +2513,7 @@ export class SpatiallyIndexedSkeletonLayer } const result = mergeSpatiallyIndexedSkeletonOverlaySegmentIds( this.getActiveEditableSegmentIds(), - this.retainedOverlaySegmentIds, + [...this.retainedOverlaySegments.keys()], ); this.cachedOverlayRenderVisibleSet = visibleSet; this.cachedOverlayRenderVisibleGeneration = visibleGeneration; diff --git a/src/skeleton/segment_overlay.spec.ts b/src/skeleton/segment_overlay.spec.ts index 453951afe..f63dd2b02 100644 --- a/src/skeleton/segment_overlay.spec.ts +++ b/src/skeleton/segment_overlay.spec.ts @@ -106,27 +106,38 @@ describe("mergeSpatiallyIndexedSkeletonOverlaySegmentIds", () => { }); describe("retainSpatiallyIndexedSkeletonOverlaySegment", () => { - it("moves retained segments to the most recent position", () => { - expect(retainSpatiallyIndexedSkeletonOverlaySegment([2, 4, 6], 4)).toEqual([ - 2, 6, 4, + it("sets the touched segment's recency counter", () => { + const retained = retainSpatiallyIndexedSkeletonOverlaySegment( + new Map([ + [2, 0], + [4, 1], + [6, 2], + ]), + 4, + 10, + ); + expect([...retained.entries()]).toEqual([ + [2, 0], + [4, 10], + [6, 2], ]); }); - it("keeps only the most recent retained segments", () => { - const retained: number[] = []; + it("keeps only the most recently touched segments", () => { + let retained = new Map(); for ( let segmentId = 1; segmentId <= DEFAULT_MAX_RETAINED_OVERLAY_SEGMENTS + 2; ++segmentId ) { - retained.splice( - 0, - retained.length, - ...retainSpatiallyIndexedSkeletonOverlaySegment(retained, segmentId), + retained = retainSpatiallyIndexedSkeletonOverlaySegment( + retained, + segmentId, + segmentId, ); } const firstRetainedSegmentId = 3; - expect(retained).toEqual( + expect([...retained.keys()].sort((a, b) => a - b)).toEqual( Array.from( { length: DEFAULT_MAX_RETAINED_OVERLAY_SEGMENTS }, (_, index) => firstRetainedSegmentId + index, diff --git a/src/skeleton/segment_overlay.ts b/src/skeleton/segment_overlay.ts index 0665c054a..833847a92 100644 --- a/src/skeleton/segment_overlay.ts +++ b/src/skeleton/segment_overlay.ts @@ -161,7 +161,7 @@ export function buildSpatiallyIndexedSkeletonOverlayGeometry( }; } -export const DEFAULT_MAX_RETAINED_OVERLAY_SEGMENTS = 16; +export const DEFAULT_MAX_RETAINED_OVERLAY_SEGMENTS = 24; function normalizeSegmentId(segmentId: number) { const normalizedSegmentId = Math.round(Number(segmentId)); @@ -184,29 +184,50 @@ export function mergeSpatiallyIndexedSkeletonOverlaySegmentIds( return [...mergedSegmentIds].sort((a, b) => a - b); } +/** + * Trims to `maxRetained` entries, evicting the oldest (smallest counter) + * entries first. + */ +function trimRetainedOverlaySegments( + retainedSegments: Map, + maxRetained: number, +): Map { + const excess = retainedSegments.size - maxRetained; + if (excess <= 0) { + return retainedSegments; + } + const oldestSegmentIdsFirst = [...retainedSegments.entries()] + .sort((a, b) => a[1] - b[1]) + .map(([candidateSegmentId]) => candidateSegmentId); + const nextRetainedSegments = new Map(retainedSegments); + for (const candidateSegmentId of oldestSegmentIdsFirst.slice(0, excess)) { + nextRetainedSegments.delete(candidateSegmentId); + } + return nextRetainedSegments; +} + +/** + * Adds or refreshes `segmentId` at recency `touchCounter`, then trims to + * `maxRetained` by evicting the oldest-touched entries. Only the relative + * order of `touchCounter` values across entries matters. + */ export function retainSpatiallyIndexedSkeletonOverlaySegment( - retainedSegmentIds: readonly number[], + retainedSegments: ReadonlyMap, segmentId: number, + touchCounter: number, options: { maxRetained?: number; } = {}, -) { +): Map { const normalizedSegmentId = normalizeSegmentId(segmentId); if (normalizedSegmentId === undefined) { - return [...retainedSegmentIds]; + return new Map(retainedSegments); } - const nextRetainedSegmentIds = retainedSegmentIds.filter( - (candidateSegmentId) => candidateSegmentId !== normalizedSegmentId, - ); - nextRetainedSegmentIds.push(normalizedSegmentId); + const nextRetainedSegments = new Map(retainedSegments); + nextRetainedSegments.set(normalizedSegmentId, touchCounter); const maxRetained = Math.max( 1, Math.round(options.maxRetained ?? DEFAULT_MAX_RETAINED_OVERLAY_SEGMENTS), ); - if (nextRetainedSegmentIds.length <= maxRetained) { - return nextRetainedSegmentIds; - } - return nextRetainedSegmentIds.slice( - nextRetainedSegmentIds.length - maxRetained, - ); + return trimRetainedOverlaySegments(nextRetainedSegments, maxRetained); } From b75bad0d61dc1203a74e83647db9f8362614166c Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Tue, 11 Aug 2026 14:25:04 +0200 Subject: [PATCH 08/11] chore: restore deleted color test --- src/util/color.browser_test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/util/color.browser_test.ts b/src/util/color.browser_test.ts index 4915ddde3..b296eb073 100644 --- a/src/util/color.browser_test.ts +++ b/src/util/color.browser_test.ts @@ -20,6 +20,7 @@ import { parseRGBColorSpecification, packColor, serializeColor, + useWhiteBackground, } from "#src/util/color.js"; import { vec3, vec4 } from "#src/util/geom.js"; @@ -81,3 +82,13 @@ describe("color", () => { expect(packColor(vec4.fromValues(0.4, 4.4, -0.4, 4))).toEqual(0xff00ff66); }); }); + +describe("useWhiteBackground", () => { + it("works for simple cases", () => { + expect(useWhiteBackground(vec3.fromValues(0, 0, 0))).toBe(true); + expect(useWhiteBackground(vec3.fromValues(1, 1, 1))).toBe(false); + expect(useWhiteBackground(vec3.fromValues(1, 0, 0))).toBe(false); + expect(useWhiteBackground(vec3.fromValues(0, 1, 0))).toBe(false); + expect(useWhiteBackground(vec3.fromValues(0, 0, 1))).toBe(true); + }); +}); From 47b3cb4e72281f43675f4151d2a29089ba47e5a4 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Tue, 11 Aug 2026 21:32:51 +0200 Subject: [PATCH 09/11] Merge pull request from MetaCell/fix/canary fix: correct canary tests and usage of removed option on makeIcon --- src/chunk_manager/backend.spec.ts | 100 ++++++---- src/chunk_manager/backend.ts | 44 ++--- src/chunk_manager/base.ts | 4 +- src/chunk_manager/frontend.ts | 36 ++-- src/datasource/catmaid/api.ts | 26 ++- src/layer/segmentation/index.ts | 13 +- src/layer/segmentation/selection.spec.ts | 4 +- src/layer/segmentation/selection.ts | 4 +- src/skeleton/frontend.spec.ts | 96 ++++------ src/skeleton/frontend.ts | 60 +++--- src/sliceview/backend.ts | 3 +- src/sliceview/base.ts | 11 ++ src/ui/skeleton_edit_tool_shortcuts.ts | 171 ++++++++++++++++++ src/ui/skeleton_edit_tools.spec.ts | 75 +++++--- ...skeleton_optimistic_edit_queue_tab.spec.ts | 20 +- 15 files changed, 455 insertions(+), 212 deletions(-) create mode 100644 src/ui/skeleton_edit_tool_shortcuts.ts diff --git a/src/chunk_manager/backend.spec.ts b/src/chunk_manager/backend.spec.ts index f4dd3f815..610838fa2 100644 --- a/src/chunk_manager/backend.spec.ts +++ b/src/chunk_manager/backend.spec.ts @@ -2,29 +2,14 @@ import { describe, expect, it, vi } from "vitest"; import { ChunkQueueManager } from "#src/chunk_manager/backend.js"; import { ChunkState } from "#src/chunk_manager/base.js"; +import { getChunkKey } from "#src/sliceview/base.js"; describe("ChunkQueueManager targeted source invalidation", () => { - it("invalidates only chunks whose keys match requested cell prefixes", () => { - const matchingWorkerChunk = { - key: "13,9,5:0", - state: ChunkState.SYSTEM_MEMORY_WORKER, - freeSystemMemory: vi.fn(), - }; - const matchingSystemChunk = { - key: "13,9,5:1", - state: ChunkState.SYSTEM_MEMORY, - freeSystemMemory: vi.fn(), - }; - const adjacentCellChunk = { - key: "13,9,50:0", - state: ChunkState.SYSTEM_MEMORY, - freeSystemMemory: vi.fn(), - }; - const otherCellChunk = { - key: "13,9,6:0", - state: ChunkState.SYSTEM_MEMORY, - freeSystemMemory: vi.fn(), - }; + function makeChunk(key: string, state: ChunkState) { + return { key, state, freeSystemMemory: vi.fn() }; + } + + function makeQueueManager() { const rpc = { invoke: vi.fn() }; const queueManager = Object.assign( Object.create(ChunkQueueManager.prototype), @@ -38,39 +23,84 @@ describe("ChunkQueueManager targeted source invalidation", () => { ), }, ); + return { rpc, queueManager }; + } + + it("requeues only the identified chunks", () => { + const workerChunk = makeChunk( + getChunkKey([13, 9, 5]), + ChunkState.SYSTEM_MEMORY_WORKER, + ); + const systemChunk = makeChunk( + getChunkKey([13, 9, 6]), + ChunkState.SYSTEM_MEMORY, + ); + // Would have been caught by a bare `startsWith` test against key "13,9,5". + const untouchedChunk = makeChunk( + getChunkKey([13, 9, 50]), + ChunkState.SYSTEM_MEMORY, + ); + const { rpc, queueManager } = makeQueueManager(); const source = { rpcId: 7, chunks: new Map([ - [matchingWorkerChunk.key, matchingWorkerChunk], - [matchingSystemChunk.key, matchingSystemChunk], - [adjacentCellChunk.key, adjacentCellChunk], - [otherCellChunk.key, otherCellChunk], + [workerChunk.key, workerChunk], + [systemChunk.key, systemChunk], + [untouchedChunk.key, untouchedChunk], ]), }; - queueManager.invalidateSourceCacheKeyPrefixes(source, ["13,9,5:"]); + queueManager.invalidateSourceCacheKeys(source, [ + workerChunk.key, + systemChunk.key, + ]); - expect(matchingWorkerChunk.freeSystemMemory).toHaveBeenCalledTimes(1); + expect(workerChunk.freeSystemMemory).toHaveBeenCalledTimes(1); expect(queueManager.updateChunkState).toHaveBeenCalledWith( - matchingWorkerChunk, + workerChunk, ChunkState.QUEUED, ); expect(queueManager.updateChunkState).toHaveBeenCalledWith( - matchingSystemChunk, - ChunkState.QUEUED, - ); - expect(queueManager.updateChunkState).not.toHaveBeenCalledWith( - adjacentCellChunk, + systemChunk, ChunkState.QUEUED, ); expect(queueManager.updateChunkState).not.toHaveBeenCalledWith( - otherCellChunk, + untouchedChunk, ChunkState.QUEUED, ); expect(rpc.invoke).toHaveBeenCalledWith("Chunk.update", { source: 7, - keyPrefixes: ["13,9,5:"], + keys: [workerChunk.key, systemChunk.key], }); + // Marking chunks QUEUED only takes effect once the queue is processed. expect(queueManager.scheduleUpdate).toHaveBeenCalledTimes(1); }); + + it("tells the frontend only about the keys it actually invalidated", () => { + const chunk = makeChunk(getChunkKey([13, 9, 5]), ChunkState.SYSTEM_MEMORY); + const { rpc, queueManager } = makeQueueManager(); + const source = { rpcId: 7, chunks: new Map([[chunk.key, chunk]]) }; + + queueManager.invalidateSourceCacheKeys(source, [ + chunk.key, + getChunkKey([99, 99, 99]), + ]); + + expect(rpc.invoke).toHaveBeenCalledWith("Chunk.update", { + source: 7, + keys: [chunk.key], + }); + }); + + it("does not notify the frontend when no key matched", () => { + const chunk = makeChunk(getChunkKey([13, 9, 6]), ChunkState.SYSTEM_MEMORY); + const { rpc, queueManager } = makeQueueManager(); + const source = { rpcId: 7, chunks: new Map([[chunk.key, chunk]]) }; + + queueManager.invalidateSourceCacheKeys(source, [getChunkKey([13, 9, 5])]); + + expect(queueManager.updateChunkState).not.toHaveBeenCalled(); + expect(rpc.invoke).not.toHaveBeenCalled(); + expect(queueManager.scheduleUpdate).not.toHaveBeenCalled(); + }); }); diff --git a/src/chunk_manager/backend.ts b/src/chunk_manager/backend.ts index f514eb60b..7fa4ec58c 100644 --- a/src/chunk_manager/backend.ts +++ b/src/chunk_manager/backend.ts @@ -23,7 +23,7 @@ import { CHUNK_LAYER_STATISTICS_RPC_ID, CHUNK_MANAGER_RPC_ID, CHUNK_QUEUE_MANAGER_RPC_ID, - CHUNK_SOURCE_INVALIDATE_KEY_PREFIXES_RPC_ID, + CHUNK_SOURCE_INVALIDATE_KEYS_RPC_ID, CHUNK_SOURCE_INVALIDATE_RPC_ID, ChunkDownloadStatistics, ChunkMemoryStatistics, @@ -61,10 +61,6 @@ import { const DEBUG_CHUNK_UPDATES = false; -function keyMatchesAnyPrefix(key: string, keyPrefixes: readonly string[]) { - return keyPrefixes.some((keyPrefix) => key.startsWith(keyPrefix)); -} - export interface ChunkStateListener { (chunk: Chunk, oldState: ChunkState): void; } @@ -1132,16 +1128,15 @@ export class ChunkQueueManager extends SharedObjectCounterpart { this.scheduleUpdate(); } - invalidateSourceCacheKeyPrefixes( - source: ChunkSource, - keyPrefixes: readonly string[], - ) { - let invalidated = false; - for (const chunk of source.chunks.values()) { - const key = chunk.key; - if (key === null || !keyMatchesAnyPrefix(key, keyPrefixes)) { - continue; - } + /** + * Like {@link invalidateSourceCache}, but limited to the chunks named by `keys`. Keys are the + * `Chunk.key` values the source assigns; keys naming no cached chunk are ignored. + */ + invalidateSourceCacheKeys(source: ChunkSource, keys: readonly string[]) { + const invalidatedKeys: string[] = []; + for (const key of keys) { + const chunk = source.chunks.get(key); + if (chunk === undefined) continue; switch (chunk.state) { case ChunkState.DOWNLOADING: cancelChunkDownload(chunk); @@ -1152,14 +1147,14 @@ export class ChunkQueueManager extends SharedObjectCounterpart { } // Note: After calling this, chunk may no longer be valid. this.updateChunkState(chunk, ChunkState.QUEUED); - invalidated = true; + invalidatedKeys.push(key); } - if (!invalidated) { + if (invalidatedKeys.length === 0) { return; } this.rpc!.invoke("Chunk.update", { source: source.rpcId, - keyPrefixes: [...keyPrefixes], + keys: invalidatedKeys, }); this.scheduleUpdate(); } @@ -1415,18 +1410,9 @@ registerRPC(CHUNK_SOURCE_INVALIDATE_RPC_ID, function (x) { source.chunkManager.queueManager.invalidateSourceCache(source); }); -registerRPC(CHUNK_SOURCE_INVALIDATE_KEY_PREFIXES_RPC_ID, function (x) { +registerRPC(CHUNK_SOURCE_INVALIDATE_KEYS_RPC_ID, function (x) { const source = this.get(x.id); - const keyPrefixes = Array.isArray(x.keyPrefixes) - ? x.keyPrefixes.filter( - (keyPrefix: unknown): keyPrefix is string => - typeof keyPrefix === "string" && keyPrefix.length !== 0, - ) - : []; - source.chunkManager.queueManager.invalidateSourceCacheKeyPrefixes( - source, - keyPrefixes, - ); + source.chunkManager.queueManager.invalidateSourceCacheKeys(source, x.keys); }); registerPromiseRPC( diff --git a/src/chunk_manager/base.ts b/src/chunk_manager/base.ts index 438d8f178..1f9f89e62 100644 --- a/src/chunk_manager/base.ts +++ b/src/chunk_manager/base.ts @@ -100,8 +100,8 @@ export const PREFETCH_PRIORITY_MULTIPLIER = 1e13; export const CHUNK_QUEUE_MANAGER_RPC_ID = "ChunkQueueManager"; export const CHUNK_MANAGER_RPC_ID = "ChunkManager"; export const CHUNK_SOURCE_INVALIDATE_RPC_ID = "ChunkSource.invalidate"; -export const CHUNK_SOURCE_INVALIDATE_KEY_PREFIXES_RPC_ID = - "ChunkSource.invalidateKeyPrefixes"; +export const CHUNK_SOURCE_INVALIDATE_KEYS_RPC_ID = + "ChunkSource.invalidateKeys"; export const REQUEST_CHUNK_STATISTICS_RPC_ID = "ChunkQueueManager.requestChunkStatistics"; diff --git a/src/chunk_manager/frontend.ts b/src/chunk_manager/frontend.ts index 496e5914c..27f064184 100644 --- a/src/chunk_manager/frontend.ts +++ b/src/chunk_manager/frontend.ts @@ -22,7 +22,7 @@ import { CHUNK_LAYER_STATISTICS_RPC_ID, CHUNK_MANAGER_RPC_ID, CHUNK_QUEUE_MANAGER_RPC_ID, - CHUNK_SOURCE_INVALIDATE_KEY_PREFIXES_RPC_ID, + CHUNK_SOURCE_INVALIDATE_KEYS_RPC_ID, CHUNK_SOURCE_INVALIDATE_RPC_ID, ChunkState, REQUEST_CHUNK_STATISTICS_RPC_ID, @@ -46,10 +46,6 @@ import { const DEBUG_CHUNK_UPDATES = false; -function keyMatchesAnyPrefix(key: string, keyPrefixes: readonly string[]) { - return keyPrefixes.some((keyPrefix) => key.startsWith(keyPrefix)); -} - export class Chunk { state = ChunkState.SYSTEM_MEMORY; constructor(public source: ChunkSource) {} @@ -247,15 +243,13 @@ export class ChunkQueueManager extends SharedObject { } if (update.promise !== undefined) { this.handleFetch_(source, update); - } else if (update.keyPrefixes !== undefined) { - const keyPrefixes = update.keyPrefixes as string[]; - const chunkKeysToDelete = [...source.chunks.keys()].filter((chunkKey) => - keyMatchesAnyPrefix(chunkKey, keyPrefixes), - ); - for (const chunkKey of chunkKeysToDelete) { - source.deleteChunk(chunkKey); + } else if (update.keys !== undefined) { + for (const chunkKey of update.keys as string[]) { + if (source.chunks.has(chunkKey)) { + source.deleteChunk(chunkKey); + visibleChunksChanged = true; + } } - visibleChunksChanged = chunkKeysToDelete.length !== 0; } else if (update.id === undefined) { // Invalidate source. for (const chunkKey of source.chunks.keys()) { @@ -496,19 +490,17 @@ export class ChunkSource extends SharedObject { } /** - * Invalidates cached chunks whose backend keys match any of the specified prefixes. - * Operates asynchronously. + * Invalidates the cached chunks named by `keys`, leaving the rest of the cache intact. Keys are + * the `Chunk.key` values the source assigns. Operates asynchronously. */ - invalidateCacheKeyPrefixes(keyPrefixes: Iterable): void { - const normalizedKeyPrefixes = [...new Set(keyPrefixes)].filter( - (keyPrefix) => keyPrefix.length !== 0, - ); - if (normalizedKeyPrefixes.length === 0) { + invalidateCacheKeys(keys: Iterable): void { + const uniqueKeys = [...new Set(keys)]; + if (uniqueKeys.length === 0) { return; } - this.rpc!.invoke(CHUNK_SOURCE_INVALIDATE_KEY_PREFIXES_RPC_ID, { + this.rpc!.invoke(CHUNK_SOURCE_INVALIDATE_KEYS_RPC_ID, { id: this.rpcId, - keyPrefixes: normalizedKeyPrefixes, + keys: uniqueKeys, }); } diff --git a/src/datasource/catmaid/api.ts b/src/datasource/catmaid/api.ts index abd9126be..4f8adbe99 100644 --- a/src/datasource/catmaid/api.ts +++ b/src/datasource/catmaid/api.ts @@ -1709,7 +1709,31 @@ export class CatmaidClient implements CatmaidSpatialSkeletonEditApi { "CATMAID skeleton/reroot did not return the requested new root.", ); } - return {}; + // The `nocheck` path is used for optimistic compensation, where no cached revision tokens are + // being reconciled, so CATMAID is not asked for an edition time either. + if (options.nocheck === true) { + return {}; + } + // Rerooting rewrites the parent links along the path from the old root to the new one, bumping + // the edition time of every node on that path. CATMAID reports a single edition time for the + // operation, which applies to all of them; without it the cached revision tokens for those nodes + // would go stale and the next edit would be rejected as out of date. + const revisionToken = normalizeCatmaidRevisionToken(response?.edition_time); + if (revisionToken === undefined) { + throw new Error( + "CATMAID skeleton/reroot did not return the new root edition_time.", + ); + } + const sourceState = makeCatmaidNodeSourceState(revisionToken)!; + const nodeSourceStateUpdates = (editContext?.nodes ?? []).map( + ({ nodeId: affectedNodeId }) => ({ + nodeId: affectedNodeId, + sourceState, + }), + ); + return nodeSourceStateUpdates.length === 0 + ? {} + : { nodeSourceStateUpdates }; } async deleteNode( diff --git a/src/layer/segmentation/index.ts b/src/layer/segmentation/index.ts index eeb1d36bb..7c9a49a9d 100644 --- a/src/layer/segmentation/index.ts +++ b/src/layer/segmentation/index.ts @@ -2668,12 +2668,13 @@ export class SegmentationUserLayer extends Base { iconFilterType !== undefined ? getSpatialSkeletonNodeFilterLabel(iconFilterType) : nodeTypeLabel; - icon.appendChild( - makeIcon({ - svg: - iconFilterType === SpatialSkeletonNodeFilterType.TRUE_END - ? svg_flag - : iconFilterType === SpatialSkeletonNodeFilterType.VIRTUAL_END + const nodeTypeIcon = makeIcon({ + svg: + iconFilterType === SpatialSkeletonNodeFilterType.TRUE_END + ? svg_flag + : iconFilterType === SpatialSkeletonNodeFilterType.VIRTUAL_END + ? svg_circle + : nodeType === undefined ? svg_circle : nodeType === undefined ? svg_circle diff --git a/src/layer/segmentation/selection.spec.ts b/src/layer/segmentation/selection.spec.ts index 4d28fc209..d8736acb3 100644 --- a/src/layer/segmentation/selection.spec.ts +++ b/src/layer/segmentation/selection.spec.ts @@ -169,13 +169,15 @@ describe("layer/segmentation/selection", () => { ); const trigger = () => handlers.forEach((h) => h()); + // A node id alone is not a usable hover: consumers derive the outline colour from the hovered + // segment, so both ids have to resolve before a hover is reported. mouseState = { active: true, pickedRenderLayer: renderLayerA, pickedSpatialSkeleton: { nodeId: 31 }, }; trigger(); - expect(hoverState.value).toEqual({ nodeId: 31 }); + expect(hoverState.value).toBeUndefined(); mouseState = { active: true, diff --git a/src/layer/segmentation/selection.ts b/src/layer/segmentation/selection.ts index 26e7a433d..b1288de71 100644 --- a/src/layer/segmentation/selection.ts +++ b/src/layer/segmentation/selection.ts @@ -31,7 +31,9 @@ interface SpatialSkeletonViewerHoverMouseStateLike { export interface SpatialSkeletonHoverInfo { readonly nodeId: number; - readonly segmentId?: number; + // A hover is only reported once both ids resolve, so consumers (e.g. outline colouring, which + // derives from the hovered segment) can rely on the segment being known. + readonly segmentId: number; // Model-space position of the picked node, carried so the highlight overlay can // still be placed when the node's skeleton is not currently loaded/cached. readonly position?: Float32Array; diff --git a/src/skeleton/frontend.spec.ts b/src/skeleton/frontend.spec.ts index 16882f52f..24d953f36 100644 --- a/src/skeleton/frontend.spec.ts +++ b/src/skeleton/frontend.spec.ts @@ -16,6 +16,7 @@ import { describe, expect, it, vi } from "vitest"; +import type { SliceViewChunkSpecification } from "#src/sliceview/base.js"; import { Uint64Set } from "#src/uint64_set.js"; import { getContrastRatio } from "#src/util/color.js"; import { vec3 } from "#src/util/geom.js"; @@ -36,7 +37,7 @@ if (!("WebGL2RenderingContext" in globalThis)) { const { SpatiallyIndexedSkeletonLayer, - getSpatialSkeletonCellKeyPrefix, + getSpatialSkeletonChunkKey, resolveSpatiallyIndexedSkeletonSegmentPick, } = await import("#src/skeleton/frontend.js"); @@ -240,48 +241,10 @@ describe("SpatiallyIndexedSkeletonLayer selected node outline color", () => { expect(computeSegmentColor).toHaveBeenCalledTimes(2); }); - it("derives the hovered-node outline color from the hovered segment when nothing is selected", () => { - const sourceColor = vec3.fromValues(1, 1, 1); - const displayState = { - segmentationColorGroupState: { - value: { - segmentStatedColors: new Map(), - segmentDefaultColor: { value: sourceColor }, - segmentColorHash: { compute: vi.fn() }, - }, - }, - saturation: { value: 0 }, - hoverHighlight: { value: true }, - segmentSelectionState: { isSelected: vi.fn(() => false), baseValue: 0n }, - }; - const layer = Object.assign( - Object.create(SpatiallyIndexedSkeletonLayer.prototype), - { - selectedNodeInfo: { value: undefined }, - hoveredNodeInfo: { value: { nodeId: 303, segmentId: 202 } }, - selectedNodeOutlineColor: vec3.create(), - highlightedNodeOutlineColor: vec3.create(), - nodeOutlineColorGeneration: 0, - cachedNodeOutlineColorGeneration: -1, - displayState, - }, - ); - - (layer as any).updateNodeOutlineColorPair(); - const highlightedColor = (layer as any).highlightedNodeOutlineColor; - - // The hovered outline is chosen for high contrast against its own (white) - // segment color. - expect( - getContrastRatio(highlightedColor, sourceColor), - ).toBeGreaterThanOrEqual(3); - }); - it("derives each outline from its own segment when selected and hovered nodes belong to different segments", () => { // Selected node on a dark segment, hovered node on a bright segment, as // happens when hovering a merge target on a differently colored skeleton. const selectedSegmentColor = vec3.fromValues(0, 0, 0); - const hoveredSegmentColor = vec3.fromValues(1, 1, 1); const displayState = { segmentationColorGroupState: { value: { @@ -314,48 +277,63 @@ describe("SpatiallyIndexedSkeletonLayer selected node outline color", () => { const selectedColor = (layer as any).selectedNodeOutlineColor; const highlightedColor = (layer as any).highlightedNodeOutlineColor; - // Each outline contrasts against its own segment color... + // The selected outline is picked from the contrast palette, so it stands off its own segment + // color. The hovered outline is instead a saturation adjustment of its own segment color, which + // carries no contrast guarantee, so only the selected one is checked here. expect( getContrastRatio(selectedColor, selectedSegmentColor), ).toBeGreaterThanOrEqual(3); - expect( - getContrastRatio(highlightedColor, hoveredSegmentColor), - ).toBeGreaterThanOrEqual(3); - // ...and the two outlines are different colors. + // Each outline still derives from its own segment, so the two differ. expect([...selectedColor]).not.toEqual([...highlightedColor]); }); }); describe("SpatiallyIndexedSkeletonLayer targeted source invalidation", () => { - it("computes absolute half-open cell prefixes without lower-bound offsets", () => { + const spec3d = { + rank: 3, + chunkDataSize: new Float32Array([100, 100, 100]), + lowerChunkBound: new Float32Array([10, 20, 30]), + } as unknown as SliceViewChunkSpecification; + + it("derives chunk keys from the origin, without lower-bound offsets", () => { expect( - getSpatialSkeletonCellKeyPrefix( - new Float32Array([100, 200, 300]), - new Float32Array([100, 100, 100]), - ), + getSpatialSkeletonChunkKey(spec3d, new Float32Array([100, 200, 300])), ).toBe("1,2,3"); expect( - getSpatialSkeletonCellKeyPrefix( + getSpatialSkeletonChunkKey( + spec3d, new Float32Array([99.999, 199.999, 299.999]), - new Float32Array([100, 100, 100]), ), ).toBe("0,1,2"); }); - it("dedupes cell prefixes per unique source entry", () => { - const invalidateCacheKeyPrefixes = vi.fn(); + it("names no chunk when the grid is not 3D", () => { + // A 3D node position spans every combination of the extra dimensions, so no single key applies. + const spec4d = { + rank: 4, + chunkDataSize: new Float32Array([100, 100, 100, 1]), + } as unknown as SliceViewChunkSpecification; + expect( + getSpatialSkeletonChunkKey(spec4d, new Float32Array([100, 200, 300])), + ).toBeUndefined(); + }); + + it("dedupes chunk keys per unique source entry", () => { + const invalidateCacheKeys = vi.fn(); const source = { spec: { + rank: 3, chunkDataSize: new Float32Array([100, 100, 100]), lowerChunkBound: new Float32Array([10, 20, 30]), }, - invalidateCacheKeyPrefixes, + invalidateCacheKeys, }; const source2d = { spec: { + rank: 3, chunkDataSize: new Float32Array([50, 50, 50]), }, - invalidateCacheKeyPrefixes: vi.fn(), + invalidateCacheKeys: vi.fn(), }; const redrawNeeded = { dispatch: vi.fn() }; const layer = { @@ -375,10 +353,10 @@ describe("SpatiallyIndexedSkeletonLayer targeted source invalidation", () => { ); expect(invalidated).toBe(true); - expect(invalidateCacheKeyPrefixes).toHaveBeenCalledTimes(1); - expect([...invalidateCacheKeyPrefixes.mock.calls[0][0]]).toEqual(["1,2,3"]); - expect(source2d.invalidateCacheKeyPrefixes).toHaveBeenCalledTimes(1); - expect([...source2d.invalidateCacheKeyPrefixes.mock.calls[0][0]]).toEqual([ + expect(invalidateCacheKeys).toHaveBeenCalledTimes(1); + expect([...invalidateCacheKeys.mock.calls[0][0]]).toEqual(["1,2,3"]); + expect(source2d.invalidateCacheKeys).toHaveBeenCalledTimes(1); + expect([...source2d.invalidateCacheKeys.mock.calls[0][0]]).toEqual([ "2,4,6", "3,4,6", ]); diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index 95e4e2443..2766d16ec 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -103,6 +103,8 @@ import { } from "#src/skeleton/skeleton_shader_color.js"; import type { SpatiallyIndexedSkeletonView } from "#src/skeleton/source_selection.js"; import { + getChunkKey, + type SliceViewChunkSpecification, type SliceViewSourceOptions, type TransformedSource, } from "#src/sliceview/base.js"; @@ -1961,24 +1963,31 @@ export const SPATIAL_SKELETON_SOURCE_OPTIONS: SliceViewSourceOptions = { modelChannelDimensionIndices: [], }; -export function getSpatialSkeletonCellKeyPrefix( +/** + * Returns the key of the chunk containing `position`, given in the source's own voxel coordinates, + * or undefined if no single chunk can be named. + * + * A skeleton node position is 3D, so it identifies exactly one chunk only while the grid is also 3D. + * Every spatial skeleton source today is (see `CatmaidMultiscaleSpatiallyIndexedSkeletonSource`); a + * higher-rank grid would spread one 3D cell over every combination of the extra dimensions, which + * cannot be named without enumerating the source's chunks, so this reports undefined rather than + * guessing. The grid is anchored at the origin rather than at the source's lower bound, matching the + * chunk index computation in `updateFixedCurPositionInChunks`. + */ +export function getSpatialSkeletonChunkKey( + spec: SliceViewChunkSpecification, position: ArrayLike, - chunkDataSize: ArrayLike, -) { - const cell = new Array(3); - for (let i = 0; i < 3; ++i) { - const coordinate = Number(position[i]); - const chunkSize = Number(chunkDataSize[i]); - if ( - !Number.isFinite(coordinate) || - !Number.isFinite(chunkSize) || - chunkSize <= 0 - ) { - return undefined; - } - cell[i] = Math.floor(coordinate / chunkSize); - } - return `${cell[0]},${cell[1]},${cell[2]}`; +): string | undefined { + const { rank, chunkDataSize } = spec; + if (rank !== 3) return undefined; + const chunkGridPosition = new Array(rank); + for (let i = 0; i < rank; ++i) { + const coordinate = position[i]; + const chunkSize = chunkDataSize[i]; + if (!Number.isFinite(coordinate) || !(chunkSize > 0)) return undefined; + chunkGridPosition[i] = Math.floor(coordinate / chunkSize); + } + return getChunkKey(chunkGridPosition); } export abstract class MultiscaleSpatiallyIndexedSkeletonSource extends MultiscaleSliceViewChunkSource { @@ -3044,23 +3053,20 @@ export class SpatiallyIndexedSkeletonLayer const sourceId = getObjectId(chunkSource); if (seenSourceIds.has(sourceId)) continue; seenSourceIds.add(sourceId); - const keyPrefixes = new Set(); - const { chunkDataSize } = chunkSource.spec; + const chunkKeys = new Set(); + const { spec } = chunkSource; for (const position of positionList) { // Spatial skeleton node positions are already source/model coordinates; // render-layer transforms do not apply to CATMAID grid-cell keys. - const keyPrefix = getSpatialSkeletonCellKeyPrefix( - position, - chunkDataSize, - ); - if (keyPrefix !== undefined) { - keyPrefixes.add(keyPrefix); + const chunkKey = getSpatialSkeletonChunkKey(spec, position); + if (chunkKey !== undefined) { + chunkKeys.add(chunkKey); } } - if (keyPrefixes.size === 0) { + if (chunkKeys.size === 0) { continue; } - chunkSource.invalidateCacheKeyPrefixes(keyPrefixes); + chunkSource.invalidateCacheKeys(chunkKeys); invalidated = true; } if (!invalidated) { diff --git a/src/sliceview/backend.ts b/src/sliceview/backend.ts index 19b0aef1e..342b0b5a7 100644 --- a/src/sliceview/backend.ts +++ b/src/sliceview/backend.ts @@ -39,6 +39,7 @@ import type { import { filterVisibleSources, forEachPlaneIntersectingVolumetricChunk, + getChunkKey, getNormalizedChunkLayout, SLICEVIEW_ADD_VISIBLE_LAYER_RPC_ID, SLICEVIEW_REMOVE_VISIBLE_LAYER_RPC_ID, @@ -405,7 +406,7 @@ export class SliceViewChunkSourceBackend< } getChunk(chunkGridPosition: Float32Array) { - const key = chunkGridPosition.join(); + const key = getChunkKey(chunkGridPosition); let chunk = this.chunks.get(key); if (chunk === undefined) { chunk = this.getNewChunk_(this.chunkConstructor) as ChunkType; diff --git a/src/sliceview/base.ts b/src/sliceview/base.ts index 7b2e5a785..1e61a5de5 100644 --- a/src/sliceview/base.ts +++ b/src/sliceview/base.ts @@ -656,6 +656,17 @@ export interface SliceViewChunkSpecification< upperVoxelBound: Float32Array; } +/** + * Returns the key identifying the chunk at `chunkGridPosition` within its source. + * + * This is the single definition of the chunk key format, for a grid position of any rank: + * `SliceViewChunkSourceBackend.getChunk` creates chunks under these keys, so anything naming an + * existing chunk (e.g. to invalidate it) must derive its key here rather than formatting one itself. + */ +export function getChunkKey(chunkGridPosition: ArrayLike): string { + return Array.prototype.join.call(chunkGridPosition, ","); +} + export function makeSliceViewChunkSpecification< ChunkDataSize extends Uint32Array | Float32Array, >( diff --git a/src/ui/skeleton_edit_tool_shortcuts.ts b/src/ui/skeleton_edit_tool_shortcuts.ts new file mode 100644 index 000000000..6843a43e1 --- /dev/null +++ b/src/ui/skeleton_edit_tool_shortcuts.ts @@ -0,0 +1,171 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import svg_mouse from "#src/ui/images/mouse.svg?raw"; +import { makeIcon } from "#src/widget/icon.js"; + +// A shortcut is shown as a plain-text label followed by one or more combo chip +// A combo's parts render together inside a single chip, consecutive key parts are joined with "+", +// while an icon sits directly beside the key it modifies with no separator. + +export type SpatialSkeletonShortcutComboPart = + | { type: "icon"; value: string } + | { type: "key"; value: string }; + +export type SpatialSkeletonShortcutCombo = SpatialSkeletonShortcutComboPart[]; + +export interface SpatialSkeletonShortcut { + label: string; + combos: SpatialSkeletonShortcutCombo[]; +} + +export interface SpatialSkeletonToolStatusText { + status: string; + actions: SpatialSkeletonShortcut[]; +} + +export const SPATIAL_SKELETON_EDIT_TOOL_NAME = "Skeleton editing"; + +const mouseIcon: SpatialSkeletonShortcutComboPart = { + type: "icon", + value: svg_mouse, +}; +const key = (value: string): SpatialSkeletonShortcutComboPart => ({ + type: "key", + value, +}); + +const combo = ( + ...parts: SpatialSkeletonShortcutComboPart[] +): SpatialSkeletonShortcutCombo => parts; + +// Reusable combo building blocks, shared across the shortcut entries below. +const COMBO = { + click: combo(mouseIcon, key("click")), + drag: combo(mouseIcon, key("drag")), + shiftClick: combo(key("shift"), mouseIcon, key("click")), + doubleClick: combo(mouseIcon, key("double-click")), + holdM: combo(key("hold"), key("m")), + holdS: combo(key("hold"), key("s")), + holdN: combo(key("hold"), key("n")), + holdD: combo(key("hold"), key("d")), + releaseM: combo(key("release"), key("m")), + releaseS: combo(key("release"), key("s")), + releaseN: combo(key("release"), key("n")), + releaseD: combo(key("release"), key("d")), + middleClick: combo(key("middle click")), + navModifierClick: combo(key("ctrl"), key("click")), +}; + +export const SELECT_ACTION: SpatialSkeletonShortcut = { + label: "Select", + combos: [COMBO.click], +}; +export const MOVE_ACTION: SpatialSkeletonShortcut = { + label: "Move", + combos: [COMBO.drag], +}; +export const ADD_NODE_ACTION: SpatialSkeletonShortcut = { + label: "Add node", + combos: [COMBO.shiftClick], +}; +export const MERGE_ACTION: SpatialSkeletonShortcut = { + label: "Merge", + combos: [COMBO.holdM], +}; +export const SPLIT_ACTION: SpatialSkeletonShortcut = { + label: "Split", + combos: [COMBO.holdS], +}; +export const NEW_SKELETON_ACTION: SpatialSkeletonShortcut = { + label: "New skeleton", + combos: [COMBO.holdN], +}; +export const DELETE_ACTION: SpatialSkeletonShortcut = { + label: "Delete", + combos: [COMBO.holdD], +}; +export const SHOW_SKELETON_ACTION: SpatialSkeletonShortcut = { + label: "Show", + combos: [COMBO.doubleClick], +}; +export const PLACE_ACTION: SpatialSkeletonShortcut = { + label: "Place", + combos: [COMBO.click], +}; +export const DELETE_CLICK_ACTION: SpatialSkeletonShortcut = { + label: "Delete", + combos: [COMBO.click], +}; +export const EXIT_MERGE_ACTION: SpatialSkeletonShortcut = { + label: "Exit merge", + combos: [COMBO.releaseM], +}; +export const EXIT_SPLIT_ACTION: SpatialSkeletonShortcut = { + label: "Exit split", + combos: [COMBO.releaseS], +}; +export const EXIT_CREATE_ACTION: SpatialSkeletonShortcut = { + label: "Exit create", + combos: [COMBO.releaseN], +}; +export const EXIT_DELETE_ACTION: SpatialSkeletonShortcut = { + label: "Exit delete", + combos: [COMBO.releaseD], +}; +export const SPATIAL_SKELETON_ROTATE_PAN_ACTION: SpatialSkeletonShortcut = { + label: "Rotate/pan", + combos: [COMBO.middleClick, COMBO.navModifierClick], +}; + +export function renderSpatialSkeletonShortcutCombo( + combo: SpatialSkeletonShortcutCombo, +) { + const comboElement = document.createElement("span"); + comboElement.className = "neuroglancer-skeleton-tool-shortcut-combo"; + combo.forEach((part, i) => { + const prevPart = combo[i - 1]; + if (i > 0 && part.type === "key" && prevPart.type === "key") { + comboElement.append(" + "); + } + if (part.type === "icon") { + const shortcutIcon = makeIcon({ svg: part.value, clickable: false }); + shortcutIcon.classList.add("neuroglancer-skeleton-tool-shortcut-icon"); + comboElement.appendChild(shortcutIcon); + } else { + comboElement.append(part.value); + } + }); + return comboElement; +} + +export function renderSpatialSkeletonShortcut( + shortcut: SpatialSkeletonShortcut, +) { + const shortcutElement = document.createElement("span"); + shortcutElement.className = "neuroglancer-skeleton-tool-shortcut"; + const labelElement = document.createElement("span"); + labelElement.className = "neuroglancer-skeleton-tool-shortcut-label"; + labelElement.textContent = shortcut.label; + shortcutElement.appendChild(labelElement); + shortcut.combos.forEach((combo, i) => { + if (i > 0) { + shortcutElement.append("/"); + } + shortcutElement.appendChild(renderSpatialSkeletonShortcutCombo(combo)); + }); + return shortcutElement; +} diff --git a/src/ui/skeleton_edit_tools.spec.ts b/src/ui/skeleton_edit_tools.spec.ts index 0044f74bd..76bae6348 100644 --- a/src/ui/skeleton_edit_tools.spec.ts +++ b/src/ui/skeleton_edit_tools.spec.ts @@ -18,6 +18,12 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { makeCatmaidNodeSourceState } from "#src/datasource/catmaid/api.js"; import { CatmaidSpatialSkeletonEditCommands } from "#src/datasource/catmaid/spatial_skeleton_commands.js"; +import { + SKELETON_ADD_NODE, + SKELETON_CLEAR_SELECTION, + SKELETON_ENTER_MERGE_MODE, + SKELETON_ENTER_SPLIT_MODE, +} from "#src/skeleton/actions.js"; import type { SpatiallyIndexedSkeletonNode } from "#src/skeleton/api.js"; import { SpatialSkeletonCommandHistory } from "#src/skeleton/command_history.js"; import { @@ -150,6 +156,7 @@ function suppressStatusMessages() { function makeChangedSignal() { return { add: vi.fn((_listener: () => void) => () => {}), + dispatch: vi.fn(), }; } @@ -536,6 +543,7 @@ describe("spatial_skeleton_edit_tool", () => { directionAdjusted: true, }); const invalidateCachedSegments = vi.fn(); + const refreshCachedSegments = vi.fn(async () => true); const getFullSegmentNodes = vi.fn(async () => []); const selectSegment = vi.fn(); const selectSpatialSkeletonNode = vi.fn(); @@ -580,6 +588,9 @@ describe("spatial_skeleton_edit_tool", () => { }), getFullSegmentNodes, invalidateCachedSegments, + // Post-merge topology refresh re-fetches the surviving segments in place rather than + // dropping them from the cache; a truthy result means the cache changed. + refreshCachedSegments, }, getSpatiallyIndexedSkeletonLayer: () => skeletonLayer, selectSegment, @@ -613,8 +624,14 @@ describe("spatial_skeleton_edit_tool", () => { ]), }), ); - expect(invalidateCachedSegments).toHaveBeenCalledWith([17, 11]); - expect(getFullSegmentNodes).toHaveBeenCalledTimes(2); + // The surviving and absorbed segments are re-fetched in place rather than dropped, so renderers + // never observe a cache with them missing. + expect(refreshCachedSegments).toHaveBeenCalledWith( + skeletonLayer, + [17, 11], + { notify: false }, + ); + expect(invalidateCachedSegments).not.toHaveBeenCalled(); expect(selectSegment).toHaveBeenCalledWith(17n, false); expect(selectSpatialSkeletonNode).toHaveBeenCalledWith(101, true, { segmentId: 17, @@ -641,7 +658,7 @@ describe("spatial_skeleton_edit_tool", () => { let clearSelectionHandler: ((event: any) => void) | undefined; const activation = { bindAction: vi.fn((action: string, handler: (event: any) => void) => { - if (action === "spatial-skeleton-clear-node-selection") { + if (action === SKELETON_CLEAR_SELECTION) { clearSelectionHandler = handler; } }), @@ -685,7 +702,7 @@ describe("spatial_skeleton_edit_tool", () => { expect(unpin).not.toHaveBeenCalled(); }); - it("enters merge mode from the hovered node when the merge action fires", () => { + it("enters merge mode without selecting a node or setting an anchor", () => { suppressStatusMessages(); const hoveredNode = { nodeId: 101, @@ -721,6 +738,10 @@ describe("spatial_skeleton_edit_tool", () => { }, updateUnconditionally: vi.fn(() => true), active: true, + // Mirrors MouseSelectionState: the edit tool suppresses the picking indicator while a node is + // being dragged, and dispatches `changed` when it toggles. + pickingIndicatorSuppressed: false, + changed: makeChangedSignal(), }; const layer = { displayState: { @@ -769,23 +790,21 @@ describe("spatial_skeleton_edit_tool", () => { SpatialSkeletonEditTool.prototype.activate.call(tool, activation as any); // Fire the merge action (simulates pressing "m" while hovering node 101). - actions.get("spatial-skeleton-enter-merge")?.({}); + actions.get(SKELETON_ENTER_MERGE_MODE)?.({}); - expect(selectSpatialSkeletonNode).toHaveBeenCalledWith( - hoveredNode.nodeId, - true, - expect.objectContaining({ nodeId: hoveredNode.nodeId }), - ); - expect(setSpatialSkeletonMergeAnchor).toHaveBeenCalledWith( - hoveredNode.nodeId, - ); expect(layer.spatialSkeletonMergeMode.value).toBe(true); + // Entering merge preserves the existing selection and only hides its highlight; the anchor is + // set solely by the first in-mode pick, so hovering a node while pressing "m" must not select + // it or anchor to it. + expect(selectSpatialSkeletonNode).not.toHaveBeenCalled(); + expect(setSpatialSkeletonMergeAnchor).not.toHaveBeenCalled(); + expect(mergeAnchorNodeId.value).toBeUndefined(); } finally { dispose(); } }); - it("executes a split on the hovered node when the split action fires", () => { + it("arms split mode without splitting when the split action fires", () => { suppressStatusMessages(); const hoveredNode = { nodeId: 77, @@ -814,6 +833,10 @@ describe("spatial_skeleton_edit_tool", () => { }, updateUnconditionally: vi.fn(() => true), active: true, + // Mirrors MouseSelectionState: the edit tool suppresses the picking indicator while a node is + // being dragged, and dispatches `changed` when it toggles. + pickingIndicatorSuppressed: false, + changed: makeChangedSignal(), }; const selectSegment = vi.fn(); const selectSpatialSkeletonNode = vi.fn(); @@ -861,19 +884,19 @@ describe("spatial_skeleton_edit_tool", () => { SpatialSkeletonEditTool.prototype.activate.call(tool, activation as any); // Fire the split action (simulates pressing "s" while hovering node 77). - actions.get("spatial-skeleton-split")?.({}); + actions.get(SKELETON_ENTER_SPLIT_MODE)?.({}); - expect(selectSegment).toHaveBeenCalledWith(11n, true); - expect(selectSpatialSkeletonNode).toHaveBeenCalledWith( - hoveredNode.nodeId, + expect(layer.spatialSkeletonSplitMode.value).toBe(true); + // The selected-node highlight stays hidden until the user clicks the node to split. + expect(layer.spatialSkeletonSuppressSelectedNodeHighlight.value).toBe( true, - expect.objectContaining({ nodeId: hoveredNode.nodeId }), ); - expect(splitSkeletonsCommand.createCommand).toHaveBeenCalledWith(layer, { - nodeId: hoveredNode.nodeId, - segmentId: hoveredNode.segmentId, - }); - expect(splitExecute).toHaveBeenCalledTimes(1); + // Pressing "s" only arms split mode: the split itself runs on the in-mode pick, so nothing is + // selected and no command is created yet. + expect(selectSegment).not.toHaveBeenCalled(); + expect(selectSpatialSkeletonNode).not.toHaveBeenCalled(); + expect(splitSkeletonsCommand.createCommand).not.toHaveBeenCalled(); + expect(splitExecute).not.toHaveBeenCalled(); } finally { dispose(); } @@ -890,6 +913,8 @@ describe("spatial_skeleton_edit_tool", () => { updateUnconditionally: vi.fn(() => true), active: true, unsnappedPosition: new Float32Array([1, 2, 3]), + pickingIndicatorSuppressed: false, + changed: makeChangedSignal(), }; const layer = { displayState: { @@ -934,7 +959,7 @@ describe("spatial_skeleton_edit_tool", () => { try { SpatialSkeletonEditTool.prototype.activate.call(tool, activation as any); - actions.get("spatial-skeleton-add-node")?.({ + actions.get(SKELETON_ADD_NODE)?.({ stopPropagation: vi.fn(), detail: { preventDefault: vi.fn() }, }); diff --git a/src/ui/skeleton_optimistic_edit_queue_tab.spec.ts b/src/ui/skeleton_optimistic_edit_queue_tab.spec.ts index d233d5ca1..299b5b459 100644 --- a/src/ui/skeleton_optimistic_edit_queue_tab.spec.ts +++ b/src/ui/skeleton_optimistic_edit_queue_tab.spec.ts @@ -72,17 +72,31 @@ describe("SpatialSkeletonOptimisticEditQueueTab", () => { document.body.replaceChildren(); }); - it("keeps Queue tab registration disabled by the debug flag by default", () => { + it("registers no Queue tab when debug registration is disabled", () => { const { layer } = makeQueueTabLayer(() => []); const hidden = { value: false, changed: makeSignal().changed }; - expect(OPTIMISTIC_EDIT_QUEUE_DEBUG).toBe(false); expect( - maybeRegisterSpatialSkeletonOptimisticEditQueueTab(layer, hidden as any), + maybeRegisterSpatialSkeletonOptimisticEditQueueTab( + layer, + hidden as any, + false, + ), ).toBe(false); expect(layer.tabs.add).not.toHaveBeenCalled(); }); + it("defaults to the OPTIMISTIC_EDIT_QUEUE_DEBUG flag when not told otherwise", () => { + const { layer } = makeQueueTabLayer(() => []); + const hidden = { value: false, changed: makeSignal().changed }; + + // Asserted against the flag rather than a literal: this is a development toggle, so pinning its + // shipped value here only breaks the suite whenever someone flips it. + expect( + maybeRegisterSpatialSkeletonOptimisticEditQueueTab(layer, hidden as any), + ).toBe(OPTIMISTIC_EDIT_QUEUE_DEBUG); + }); + it("registers a dedicated Queue tab when debug registration is enabled", () => { const { layer } = makeQueueTabLayer(() => []); const hidden = { value: false, changed: makeSignal().changed }; From d8531cc19d171b858cfdd08197141bd70ad6aa8d Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Thu, 13 Aug 2026 14:36:38 +0200 Subject: [PATCH 10/11] Merge pull request from MetaCell/feat/tool-bind-precedence feat: add bind priority and restore rotate key --- src/help/input_event_bindings.ts | 2 +- src/ui/skeleton_tab.ts | 8 +- src/ui/tool.ts | 68 ++++++++++- src/ui/tool_binding_precedence.spec.ts | 157 +++++++++++++++++++++++++ src/viewer.ts | 33 +++++- 5 files changed, 255 insertions(+), 13 deletions(-) create mode 100644 src/ui/tool_binding_precedence.spec.ts diff --git a/src/help/input_event_bindings.ts b/src/help/input_event_bindings.ts index ff5ec6b1b..70bdf77dd 100644 --- a/src/help/input_event_bindings.ts +++ b/src/help/input_event_bindings.ts @@ -255,7 +255,7 @@ export class InputEventBindingHelpDialog extends SidePanel { layerBindings = []; layerToolBindingsMap.set(tool.context, layerBindings); } - layerBindings.push([`shift+key${key.toLowerCase()}`, tool.description]); + layerBindings.push([`key${key.toLowerCase()}`, tool.description]); } } const layerToolBindings = Array.from(layerToolBindingsMap.entries()); diff --git a/src/ui/skeleton_tab.ts b/src/ui/skeleton_tab.ts index c12e27ac6..59c11a2ba 100644 --- a/src/ui/skeleton_tab.ts +++ b/src/ui/skeleton_tab.ts @@ -92,7 +92,10 @@ import { type SpatialSkeletonSegmentRenderRow, type SpatialSkeletonSegmentRenderState, } from "#src/ui/skeleton_tab_render.js"; -import { makeToolButton } from "#src/ui/tool.js"; +import { + CONTEXTUAL_PANEL_BINDING_PRIORITY, + makeToolButton, +} from "#src/ui/tool.js"; import type { ArraySpliceOp } from "#src/util/array.js"; import { registerActionListener, @@ -360,9 +363,12 @@ export class SpatialSkeletonEditTab extends Tab { // Add the tab navigation map to the viewer's slice and perspective view // panels so shortcuts work when the user's focus is on a viewport, not just // the sidebar. Scoped to this Tab's lifetime via `this` as the context. + // Bound below `USER_TOOL_BINDING_PRIORITY` so that a tool the user has bound + // to one of these letters still wins while the tab is open. layer.manager.root.toolBinder.bindInputEventMap( getDefaultSkeletonTabBindings(), this, + CONTEXTUAL_PANEL_BINDING_PRIORITY, ); let allNodes: SpatiallyIndexedSkeletonNode[] = []; diff --git a/src/ui/tool.ts b/src/ui/tool.ts index 30ecfb7f6..011903de3 100644 --- a/src/ui/tool.ts +++ b/src/ui/tool.ts @@ -37,11 +37,11 @@ import { animationFrameDebounce } from "#src/util/animation_frame_debounce.js"; import type { Borrowed, Owned } from "#src/util/disposable.js"; import { RefCounted } from "#src/util/disposable.js"; import { getDropEffectFromModifiers } from "#src/util/drag_and_drop.js"; -import type { - ActionEvent, +import type { ActionEvent } from "#src/util/event_action_map.js"; +import { EventActionMap, + registerActionListener, } from "#src/util/event_action_map.js"; -import { registerActionListener } from "#src/util/event_action_map.js"; import { verifyObject, verifyObjectProperty, @@ -52,9 +52,25 @@ import { Signal } from "#src/util/signal.js"; const TOOL_KEY_PATTERN = /^[A-Z]$/; +// Priorities used when attaching an `EventActionMap` as a parent of the +// viewer's root binding maps. `HierarchicalMap.get` consults parents with +// priority greater than 0 before the map's own direct bindings, and the +// built-in default maps are attached at `NEGATIVE_INFINITY`, giving: +// +// +Inf bindings of the currently active tool +// 1000 bindings supplied via the Python `config_state` +// 100 letters the user has bound a tool to +// 10 bindings of a visible side panel, e.g. the skeleton tab +// 0 direct bindings on the root maps +// -Inf built-in global and data panel bindings +export const ACTIVE_TOOL_BINDING_PRIORITY = Number.POSITIVE_INFINITY; +export const USER_TOOL_BINDING_PRIORITY = 100; +export const CONTEXTUAL_PANEL_BINDING_PRIORITY = 10; + export type InputEventMapBinder = ( eventActionMap: EventActionMap, context: RefCounted, + priority?: number, ) => void; export class ToolActivation extends RefCounted { @@ -310,15 +326,57 @@ export class GlobalToolBinder extends RefCounted { localBinders = new Set(); localBindersChanged = new Signal(); + /** + * Maps the letter keys that currently have a tool bound to the corresponding + * `tool-` action. Attached as a parent of the viewer's root binding + * maps at `USER_TOOL_BINDING_PRIORITY` so that a tool the user has bound + * takes precedence over the built-in binding for the same letter. + */ + readonly boundKeyEventActionMap = new EventActionMap(); + + private readonly registeredBoundKeys = new Set(); + constructor( private inputEventMapBinder: InputEventMapBinder, public toolPaletteState: MultiToolPaletteState, ) { super(); + this.boundKeyEventActionMap.label = "Tool key bindings"; + this.registerDisposer( + this.changed.add(() => this.updateBoundKeyEventActionMap()), + ); } - bindInputEventMap(inputEventMap: EventActionMap, context: RefCounted) { - this.inputEventMapBinder(inputEventMap, context); + /** + * Brings `boundKeyEventActionMap` in sync with `bindings`. Driven by the + * `changed` signal, which every mutation path dispatches, including + * `LocalToolBinder.clear`, which deletes from `bindings` directly. + */ + private updateBoundKeyEventActionMap() { + const { bindings, boundKeyEventActionMap, registeredBoundKeys } = this; + // Only the bare letter and the legacy `shift`+letter form are registered. + // Including optional `alt`/`control` would allow a tool bound to `P` to + // shadow `control+keyp` → `open-command-palette`. + const eventIdentifier = (key: string) => + `shift?+key${key.toLowerCase()}` as const; + for (const key of registeredBoundKeys) { + if (bindings.has(key)) continue; + boundKeyEventActionMap.delete(eventIdentifier(key)); + registeredBoundKeys.delete(key); + } + for (const key of bindings.keys()) { + if (registeredBoundKeys.has(key)) continue; + boundKeyEventActionMap.set(eventIdentifier(key), `tool-${key}`); + registeredBoundKeys.add(key); + } + } + + bindInputEventMap( + inputEventMap: EventActionMap, + context: RefCounted, + priority?: number, + ) { + this.inputEventMapBinder(inputEventMap, context, priority); } get(key: string): Borrowed | undefined { diff --git a/src/ui/tool_binding_precedence.spec.ts b/src/ui/tool_binding_precedence.spec.ts new file mode 100644 index 000000000..9013cd19b --- /dev/null +++ b/src/ui/tool_binding_precedence.spec.ts @@ -0,0 +1,157 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { describe, expect, it } from "vitest"; +import { + getDefaultGlobalBindings, + getDefaultSkeletonTabBindings, + getDefaultSliceViewPanelBindings, +} from "#src/ui/default_input_event_bindings.js"; +import type { Tool } from "#src/ui/tool.js"; +import { + CONTEXTUAL_PANEL_BINDING_PRIORITY, + GlobalToolBinder, + USER_TOOL_BINDING_PRIORITY, +} from "#src/ui/tool.js"; +import { EventActionMap } from "#src/util/event_action_map.js"; + +/** + * Minimal stand-in for a bound tool, covering only the members + * `GlobalToolBinder.set` and `deleteBinding` touch. + */ +function makeTool(identifier: string): Tool { + return { + localBinder: { + bindings: new Map(), + jsonToKey: new Map(), + changed: { dispatch: () => {} }, + }, + changed: { add: () => () => {} }, + keyBinding: undefined, + savedJsonString: undefined, + toJSON: () => ({ type: identifier }), + dispose: () => {}, + } as unknown as Tool; +} + +function makeToolBinder() { + return new GlobalToolBinder( + () => {}, + {} as unknown as ConstructorParameters[1], + ); +} + +/** + * Mirrors how `Viewer` wires a root binding map: the built-in defaults at + * NEGATIVE_INFINITY and the user's tool bindings at USER_TOOL_BINDING_PRIORITY. + */ +function makeRootMap(toolBinder: GlobalToolBinder, defaults: EventActionMap) { + const rootMap = new EventActionMap(); + rootMap.addParent(defaults, Number.NEGATIVE_INFINITY); + rootMap.addParent( + toolBinder.boundKeyEventActionMap, + USER_TOOL_BINDING_PRIORITY, + ); + return rootMap; +} + +describe("GlobalToolBinder.boundKeyEventActionMap", () => { + it("contains only letters with a tool bound", () => { + const toolBinder = makeToolBinder(); + const { boundKeyEventActionMap } = toolBinder; + + expect(boundKeyEventActionMap.get("at:keyr")).toBeUndefined(); + + toolBinder.set("R", makeTool("a")); + expect(boundKeyEventActionMap.get("at:keyr")?.action).toBe("tool-R"); + // The legacy `shift`+letter form activates the tool as well. + expect(boundKeyEventActionMap.get("at:shift+keyr")?.action).toBe("tool-R"); + // Other letters are unaffected. + expect(boundKeyEventActionMap.get("at:keye")).toBeUndefined(); + + toolBinder.set("R", undefined); + expect(boundKeyEventActionMap.get("at:keyr")).toBeUndefined(); + }); + + it("does not claim modifier combinations used by system bindings", () => { + const toolBinder = makeToolBinder(); + toolBinder.set("P", makeTool("a")); + expect( + toolBinder.boundKeyEventActionMap.get("at:control+keyp"), + ).toBeUndefined(); + }); +}); + +describe("user tool binding precedence", () => { + it("overrides the data panel rotation bindings", () => { + const toolBinder = makeToolBinder(); + const rootMap = makeRootMap(toolBinder, getDefaultSliceViewPanelBindings()); + + expect(rootMap.get("at:keyr")?.action).toBe("rotate-relative-z-"); + expect(rootMap.get("at:keye")?.action).toBe("rotate-relative-z+"); + + toolBinder.set("R", makeTool("a")); + expect(rootMap.get("at:keyr")?.action).toBe("tool-R"); + // `e` keeps rotating: precedence applies per letter. + expect(rootMap.get("at:keye")?.action).toBe("rotate-relative-z+"); + + toolBinder.set("R", undefined); + expect(rootMap.get("at:keyr")?.action).toBe("rotate-relative-z-"); + }); + + it("overrides the global bindings", () => { + const toolBinder = makeToolBinder(); + const rootMap = makeRootMap(toolBinder, getDefaultGlobalBindings()); + + expect(rootMap.get("at:keyl")?.action).toBe("recolor"); + + toolBinder.set("L", makeTool("a")); + expect(rootMap.get("at:keyl")?.action).toBe("tool-L"); + }); + + it("leaves the command palette shortcut alone", () => { + const toolBinder = makeToolBinder(); + const rootMap = makeRootMap(toolBinder, getDefaultGlobalBindings()); + + toolBinder.set("P", makeTool("a")); + expect(rootMap.get("at:control+keyp")?.action).toBe("open-command-palette"); + }); + + it("overrides a visible side panel's bindings", () => { + const toolBinder = makeToolBinder(); + const rootMap = makeRootMap(toolBinder, getDefaultSliceViewPanelBindings()); + rootMap.addParent( + getDefaultSkeletonTabBindings(), + CONTEXTUAL_PANEL_BINDING_PRIORITY, + ); + + // The skeleton tab still outranks the data panel bindings. + expect(rootMap.get("at:keyr")?.action).toBe("skeleton-go-root"); + + toolBinder.set("R", makeTool("a")); + expect(rootMap.get("at:keyr")?.action).toBe("tool-R"); + }); + + it("yields to the active tool's own bindings", () => { + const toolBinder = makeToolBinder(); + const rootMap = makeRootMap(toolBinder, getDefaultSliceViewPanelBindings()); + const activeToolMap = EventActionMap.fromObject({ keyr: "tool-action" }); + rootMap.addParent(activeToolMap, Number.POSITIVE_INFINITY); + + toolBinder.set("R", makeTool("a")); + expect(rootMap.get("at:keyr")?.action).toBe("tool-action"); + }); +}); diff --git a/src/viewer.ts b/src/viewer.ts index 1dc73d2b3..0cd73f41d 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -108,7 +108,12 @@ import { SelectionDetailsPanel } from "#src/ui/selection_details.js"; import { SidePanelManager } from "#src/ui/side_panel.js"; import { StateEditorDialog } from "#src/ui/state_editor.js"; import { StatisticsDisplayState, StatisticsPanel } from "#src/ui/statistics.js"; -import { GlobalToolBinder, LocalToolBinder } from "#src/ui/tool.js"; +import { + ACTIVE_TOOL_BINDING_PRIORITY, + GlobalToolBinder, + LocalToolBinder, + USER_TOOL_BINDING_PRIORITY, +} from "#src/ui/tool.js"; import { MultiToolPaletteDropdownButton, MultiToolPaletteManager, @@ -616,6 +621,24 @@ export class Viewer extends RefCounted implements ViewerState { this.showLayerDialog = showLayerDialog; this.resetStateWhenEmpty = resetStateWhenEmpty; + // Letters the user has bound a tool to take precedence over the built-in + // binding for the same letter. All three root maps are needed: the panel + // maps for letters claimed by the data panel bindings (e.g. `keyr`) while a + // panel has focus, and the global map for letters claimed by the global + // bindings (e.g. `keyl`) while focus is elsewhere. + for (const rootEventActionMap of [ + this.inputEventBindings.global, + this.inputEventBindings.sliceView, + this.inputEventBindings.perspectiveView, + ]) { + this.registerDisposer( + rootEventActionMap.addParent( + this.globalToolBinder.boundKeyEventActionMap, + USER_TOOL_BINDING_PRIORITY, + ), + ); + } + this.layerSpecification = new TopLevelLayerListSpecification( this.display, this.dataSourceProvider, @@ -1235,17 +1258,15 @@ export class Viewer extends RefCounted implements ViewerState { private toolInputEventMapBinder = ( inputEventMap: EventActionMap, context: RefCounted, + priority: number = ACTIVE_TOOL_BINDING_PRIORITY, ) => { context.registerDisposer( - this.inputEventBindings.sliceView.addParent( - inputEventMap, - Number.POSITIVE_INFINITY, - ), + this.inputEventBindings.sliceView.addParent(inputEventMap, priority), ); context.registerDisposer( this.inputEventBindings.perspectiveView.addParent( inputEventMap, - Number.POSITIVE_INFINITY, + priority, ), ); }; From 05998710296a01a1abd5a753a9e33a1e0ad9b21e Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Thu, 13 Aug 2026 17:17:55 +0200 Subject: [PATCH 11/11] fix: correct merge conflict --- src/ui/skeleton_edit_tool_messages.ts | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/ui/skeleton_edit_tool_messages.ts b/src/ui/skeleton_edit_tool_messages.ts index 571ce0713..a88ecca54 100644 --- a/src/ui/skeleton_edit_tool_messages.ts +++ b/src/ui/skeleton_edit_tool_messages.ts @@ -109,28 +109,6 @@ export function getSpatialSkeletonToolPointStatusFields( } // --- Name / status / actions message system --- -// -// The tool's status bar is split into three parts: a name (rendered by the -// caller via a fixed header, see SPATIAL_SKELETON_EDIT_TOOL_NAME), a short -// `status` describing what's currently true, and a short `actions` list -// describing what's currently doable. Keeping these separate (rather than -// one long banner string) avoids mixing state with instructions, and lets -// the no-selection default state stop advertising actions that don't apply -// yet (e.g. shift+click, which requires an existing selection). -// -// User-facing copy says "from node" rather than "merge anchor" — the -<<<<<<< HEAD -export interface SpatialSkeletonToolStatusText { - status: string; - actions: string; -} - -export const SPATIAL_SKELETON_EDIT_TOOL_NAME = "Skeleton editing"; -export const SPATIAL_SKELETON_ROTATE_PAN_HINT = - "middle-click or ctrl+click to rotate/pan"; - -======= ->>>>>>> 209548a32 (Merge pull request #285 from MetaCell/feature/NGLASS-2012) export type SpatialSkeletonDefaultSelectionState = | "none" | "selected-visible"