From 7aa913b1b91218cd3cd12b54cfc8201b577b3e75 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Thu, 13 Aug 2026 18:40:04 +0200 Subject: [PATCH] fix: repair dropped lines from consolidation feat: Add optional layer-owned runtime cleanup fix: restore glsl_string in skeleton edge and node shaders The shader refactor was split by hand during the consolidation and dropped both builder.addFragmentCode(glsl_string) calls. glsl_string defines struct string_t, which shader UI select controls emit references to, so any skeleton shader using a select control would fail to compile at runtime. feat: hide skeletons action feat: prepend new status messages fix: restore reroot spec contract and comma-escaping behaviour chore: lint and format --- src/chunk_manager/base.ts | 3 +- src/chunk_manager/frontend.ts | 6 +- src/datasource/catmaid/api.spec.ts | 18 +++++- src/datasource/catmaid/frontend.ts | 3 + .../catmaid/spatial_skeleton_commands.ts | 10 ---- src/datasource/index.ts | 10 ++++ src/layer/index.ts | 13 +++- src/layer/layer_data_source.ts | 49 ++++++++++++++- src/layer/segmentation/index.ts | 59 ++++++++++++++++--- src/skeleton/actions.ts | 3 + src/skeleton/command_history.ts | 9 +++ src/skeleton/commands.ts | 15 +++-- src/skeleton/frontend.ts | 53 +++++++++++++++-- src/status.ts | 4 +- src/ui/default_input_event_bindings.ts | 2 + src/ui/skeleton_edit_tool_messages.spec.ts | 50 ++++++++++++++++ src/ui/skeleton_edit_tool_messages.ts | 12 ++++ src/ui/skeleton_edit_tools.ts | 23 +++++++- src/viewer.ts | 7 ++- 19 files changed, 305 insertions(+), 44 deletions(-) diff --git a/src/chunk_manager/base.ts b/src/chunk_manager/base.ts index 1f9f89e62e..38918e9fb6 100644 --- a/src/chunk_manager/base.ts +++ b/src/chunk_manager/base.ts @@ -100,8 +100,7 @@ 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_KEYS_RPC_ID = - "ChunkSource.invalidateKeys"; +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 27f064184a..c5b3b08cfe 100644 --- a/src/chunk_manager/frontend.ts +++ b/src/chunk_manager/frontend.ts @@ -326,7 +326,11 @@ export class ChunkQueueManager extends SharedObject { } function updateChunk(rpc: RPC, x: any) { - const source: ChunkSource = rpc.get(x.source); + const source = rpc.get(x.source) as ChunkSource | undefined; + if (source === undefined) { + // Source was removed while chunk update was in flight. + return; + } if (DEBUG_CHUNK_UPDATES) { console.log( `${Date.now()} Chunk.update received: ` + diff --git a/src/datasource/catmaid/api.spec.ts b/src/datasource/catmaid/api.spec.ts index 37cf6aee5f..f203b7c78d 100644 --- a/src/datasource/catmaid/api.spec.ts +++ b/src/datasource/catmaid/api.spec.ts @@ -926,7 +926,18 @@ describe("CatmaidClient skeleton editing methods", () => { { nodeId: 201, revisionToken: "2026-03-29T12:04:00Z" }, ], }), - ).resolves.toEqual({}); + ).resolves.toEqual({ + nodeSourceStateUpdates: [ + { + nodeId: 202, + sourceState: testSourceState("2026-03-29T12:08:00Z"), + }, + { + nodeId: 201, + sourceState: testSourceState("2026-03-29T12:08:00Z"), + }, + ], + }); expect(fetchMock).toHaveBeenCalledTimes(1); const requestBody = getFetchBody(fetchMock); @@ -1055,7 +1066,6 @@ describe("CatmaidClient skeleton editing methods", () => { const fetchMock = vi.fn().mockResolvedValue({ newroot: 202, skeleton_id: 17, - edition_time: "2026-03-29T12:08:00Z", }); (client as any).fetchProjectEndpoint = fetchMock; @@ -1075,7 +1085,9 @@ describe("CatmaidClient skeleton editing methods", () => { { nodeId: 201, revisionToken: "2026-03-29T12:04:00Z" }, ], }), - ).resolves.toEqual({}); + ).rejects.toThrow( + "CATMAID skeleton/reroot did not return the new root edition_time.", + ); expect(fetchMock).toHaveBeenCalledTimes(1); }); diff --git a/src/datasource/catmaid/frontend.ts b/src/datasource/catmaid/frontend.ts index dcfa9d0fc4..1e6244a4e2 100644 --- a/src/datasource/catmaid/frontend.ts +++ b/src/datasource/catmaid/frontend.ts @@ -482,6 +482,9 @@ export class CatmaidDataSourceProvider implements DataSourceProvider { id: "skeletons-chunked", default: true, subsource: { mesh: multiscaleSource }, + layerRuntimeStateDisposal: { + kind: "spatiallyIndexedSkeleton", + }, }, { id: "skeletons", diff --git a/src/datasource/catmaid/spatial_skeleton_commands.ts b/src/datasource/catmaid/spatial_skeleton_commands.ts index 8c536663c3..abe819b296 100644 --- a/src/datasource/catmaid/spatial_skeleton_commands.ts +++ b/src/datasource/catmaid/spatial_skeleton_commands.ts @@ -450,15 +450,6 @@ function requireCatmaidMergeCommandPayload(payload: object) { ); } -function validateCatmaidNodeDescription(description: string | undefined) { - if (description === undefined) return; - for (const line of description.split(/\r?\n/)) { - if (line.trim().includes(",")) { - throw new Error("Node descriptions containing commas are not supported."); - } - } -} - function cloneNodeSnapshot( node: SpatiallyIndexedSkeletonNode, ): SpatiallyIndexedSkeletonNode { @@ -4314,7 +4305,6 @@ class NodeDescriptionCommand implements SpatialSkeletonCommand { nextDescription: string | undefined, statusPrefix: string, ) { - validateCatmaidNodeDescription(nextDescription); const { node } = await getResolvedNodeForEdit( this.layer, this.stableNodeId, diff --git a/src/datasource/index.ts b/src/datasource/index.ts index c630322332..a8121301a2 100644 --- a/src/datasource/index.ts +++ b/src/datasource/index.ts @@ -153,6 +153,10 @@ export interface CompleteUrlOptions extends CompleteUrlOptionsBase { signal: AbortSignal; } +export interface LayerRuntimeStateDisposalRequest { + kind: string; +} + export interface DataSubsourceEntry { /** * Unique identifier (within the group) for this subsource. Stored in the JSON state @@ -182,6 +186,12 @@ export interface DataSubsourceEntry { * Specifies whether this associated data source is enabled by default. */ default: boolean; + + /** + * Optional layer-owned runtime cleanup requested when this active subsource's + * datasource is replaced or cleared. + */ + layerRuntimeStateDisposal?: LayerRuntimeStateDisposalRequest; } export interface ChannelMetadata { diff --git a/src/layer/index.ts b/src/layer/index.ts index be383f5e8a..e69488b55e 100644 --- a/src/layer/index.ts +++ b/src/layer/index.ts @@ -38,7 +38,10 @@ import type { } from "#src/datasource/index.js"; import { makeEmptyDataSourceSpecification } from "#src/datasource/index.js"; import type { DisplayContext, RenderedPanel } from "#src/display_context.js"; -import type { LoadedDataSubsource } from "#src/layer/layer_data_source.js"; +import type { + LayerDataSourceChangeRuntimeDisposalContext, + LoadedDataSubsource, +} from "#src/layer/layer_data_source.js"; import { LayerDataSource, layerDataSourceSpecificationFromJson, @@ -431,6 +434,14 @@ export class UserLayer extends RefCounted { subsources; } + // Derived classes may override to clear layer-owned runtime state for active + // datasources that explicitly request cleanup on source change. + disposeLayerRuntimeStateForDataSourceChange( + _context: LayerDataSourceChangeRuntimeDisposalContext, + ) { + return false; + } + updateDataSubsourceActivations() { function* getDataSubsources( this: UserLayer, diff --git a/src/layer/layer_data_source.ts b/src/layer/layer_data_source.ts index 63d07a7746..589627b047 100644 --- a/src/layer/layer_data_source.ts +++ b/src/layer/layer_data_source.ts @@ -31,6 +31,7 @@ import type { DataSourceWithRedirectInfo, DataSubsourceEntry, DataSubsourceSpecification, + LayerRuntimeStateDisposalRequest, } from "#src/datasource/index.js"; import { makeEmptyDataSourceSpecification } from "#src/datasource/index.js"; import type { UserLayer } from "#src/layer/index.js"; @@ -148,6 +149,7 @@ export class LoadedDataSubsource { enabled: boolean; activated: RefCounted | undefined = undefined; guardValues: any[] = []; + renderLayers = new Set(); messages = new MessageList(); isActiveChanged = new NullarySignal(); constructor( @@ -212,9 +214,13 @@ export class LoadedDataSubsource { addRenderLayer(renderLayer: Owned) { const activated = this.activated!; - activated.registerDisposer( - this.loadedDataSource.layer.addRenderLayer(renderLayer), - ); + const removeRenderLayer = + this.loadedDataSource.layer.addRenderLayer(renderLayer); + this.renderLayers.add(renderLayer); + activated.registerDisposer(() => { + this.renderLayers.delete(renderLayer); + removeRenderLayer(); + }); activated.registerDisposer(this.messages.addChild(renderLayer.messages)); } @@ -301,6 +307,16 @@ export class LoadedLayerDataSource extends RefCounted { } } +export type LayerDataSourceChangeReason = "replace" | "clear"; + +export interface LayerDataSourceChangeRuntimeDisposalContext { + request: LayerRuntimeStateDisposalRequest; + reason: LayerDataSourceChangeReason; + layerDataSource: LayerDataSource; + loadedDataSource: LoadedLayerDataSource; + loadedSubsource: LoadedDataSubsource; +} + export type LayerDataSourceLoadState = | { error: Error; @@ -368,10 +384,36 @@ export class LayerDataSource extends RefCounted { return this.loadState_; } + private disposeRuntimeStateForDataSourceChange( + reason: LayerDataSourceChangeReason, + ) { + const { loadState } = this; + if (loadState === undefined || loadState.error !== undefined) return false; + const handledRequestKinds = new Set(); + let changed = false; + for (const loadedSubsource of loadState.subsources) { + if (loadedSubsource.activated === undefined) continue; + const request = loadedSubsource.subsourceEntry.layerRuntimeStateDisposal; + if (request === undefined) continue; + if (handledRequestKinds.has(request.kind)) continue; + handledRequestKinds.add(request.kind); + changed = + this.layer.disposeLayerRuntimeStateForDataSourceChange({ + request, + reason, + layerDataSource: this, + loadedDataSource: loadState, + loadedSubsource, + }) || changed; + } + return changed; + } + set spec(spec: DataSourceSpecification) { const { layer } = this; this.messages.clearMessages(); if (spec.url.length === 0) { + this.disposeRuntimeStateForDataSourceChange("clear"); if (layer.dataSources.length !== 1) { const index = layer.dataSources.indexOf(this); if (index !== -1) { @@ -395,6 +437,7 @@ export class LayerDataSource extends RefCounted { disposableOnce(layer.markLoading()), ); if (this.refCounted_ !== undefined) { + this.disposeRuntimeStateForDataSourceChange("replace"); this.refCounted_.dispose(); this.loadState_ = undefined; } diff --git a/src/layer/segmentation/index.ts b/src/layer/segmentation/index.ts index 7c9a49a9d9..64a7d270e1 100644 --- a/src/layer/segmentation/index.ts +++ b/src/layer/segmentation/index.ts @@ -42,7 +42,10 @@ import { registerVolumeLayerType, UserLayer, } from "#src/layer/index.js"; -import type { LoadedDataSubsource } from "#src/layer/layer_data_source.js"; +import type { + LayerDataSourceChangeRuntimeDisposalContext, + LoadedDataSubsource, +} from "#src/layer/layer_data_source.js"; import { layerDataSourceSpecificationFromJson } from "#src/layer/layer_data_source.js"; import * as json_keys from "#src/layer/segmentation/json_keys.js"; import { registerLayerControls } from "#src/layer/segmentation/layer_controls.js"; @@ -104,6 +107,7 @@ import { SKELETON_GO_ROOT, SKELETON_GO_UNFINISHED, SKELETON_REDO, + SKELETON_TOGGLE_HIDDEN, SKELETON_UNDO, } from "#src/skeleton/actions.js"; import type { @@ -812,6 +816,9 @@ function copyOptionalSpatialSkeletonPosition( return new Float32Array(Array.from(value, Number)); } +const SPATIALLY_INDEXED_SKELETON_RUNTIME_DISPOSAL_KIND = + "spatiallyIndexedSkeleton"; + const Base = UserLayerWithAnnotationsMixin(UserLayer); export class SegmentationUserLayer extends Base { sliceViewRenderScaleHistogram = new RenderScaleHistogram(); @@ -1095,6 +1102,8 @@ export class SegmentationUserLayer extends Base { x === undefined ? undefined : parseUint64(x), ); + private savedHiddenObjectAlpha: number | undefined; + constructor(managedLayer: Borrowed) { super(managedLayer); this.codeVisible.changed.add(this.specificationChanged.dispatch); @@ -1597,6 +1606,31 @@ export class SegmentationUserLayer extends Base { this.spatialSkeletonState.markNodeDataChanged(options); } + disposeLayerRuntimeStateForDataSourceChange( + context: LayerDataSourceChangeRuntimeDisposalContext, + ) { + if ( + context.request.kind !== SPATIALLY_INDEXED_SKELETON_RUNTIME_DISPOSAL_KIND + ) { + return super.disposeLayerRuntimeStateForDataSourceChange(context); + } + let changed = false; + const spatialSkeletonLayers = new Set(); + for (const renderLayer of context.loadedSubsource.renderLayers) { + if ( + renderLayer instanceof PerspectiveViewSpatiallyIndexedSkeletonLayer || + renderLayer instanceof SliceViewPanelSpatiallyIndexedSkeletonLayer + ) { + spatialSkeletonLayers.add(renderLayer.base); + } + } + for (const spatialSkeletonLayer of spatialSkeletonLayers) { + changed = spatialSkeletonLayer.disposeRuntimeState() || changed; + } + changed = this.spatialSkeletonState.clearRuntimeState() || changed; + return changed; + } + activateDataSubsources(subsources: Iterable) { const updatedSegmentPropertyMaps: SegmentPropertyMap[] = []; const isGroupRoot = @@ -2100,6 +2134,17 @@ export class SegmentationUserLayer extends Base { } break; } + case SKELETON_TOGGLE_HIDDEN: { + const { hiddenObjectAlpha } = this.displayState; + if (this.savedHiddenObjectAlpha !== undefined) { + hiddenObjectAlpha.value = this.savedHiddenObjectAlpha; + this.savedHiddenObjectAlpha = undefined; + } else { + this.savedHiddenObjectAlpha = hiddenObjectAlpha.value; + hiddenObjectAlpha.value = 0; + } + break; + } case SKELETON_GO_ROOT: case SKELETON_GO_BRANCH_START: case SKELETON_GO_BRANCH_END: @@ -2676,13 +2721,11 @@ export class SegmentationUserLayer extends Base { ? svg_circle : nodeType === undefined ? svg_circle - : nodeType === undefined - ? svg_circle - : SPATIAL_SKELETON_NODE_TYPE_ICONS[nodeType], - title: nodeTypeIconTitle, - clickable: false, - }), - ); + : SPATIAL_SKELETON_NODE_TYPE_ICONS[nodeType], + title: nodeTypeIconTitle, + clickable: false, + }); + icon.appendChild(nodeTypeIcon); summaryRow.appendChild(icon); const skeletonDisplayTransform = diff --git a/src/skeleton/actions.ts b/src/skeleton/actions.ts index df2cf06a67..94ad685874 100644 --- a/src/skeleton/actions.ts +++ b/src/skeleton/actions.ts @@ -44,3 +44,6 @@ export const SKELETON_ENTER_CREATE = "skeleton-enter-create"; export const SKELETON_PIN_NODE = "skeleton-pin-node"; export const SKELETON_ENTER_DELETE_MODE = "skeleton-enter-delete-mode"; export const SKELETON_CLEAR_SELECTION = "skeleton-clear-node-selection"; + +// --- Display toggles --- +export const SKELETON_TOGGLE_HIDDEN = "skeleton-toggle-hidden"; diff --git a/src/skeleton/command_history.ts b/src/skeleton/command_history.ts index d17c7d1076..ceb61d7bd7 100644 --- a/src/skeleton/command_history.ts +++ b/src/skeleton/command_history.ts @@ -77,6 +77,10 @@ export class SpatialSkeletonCommandMappings { private nodeIdMappings = new Map(); private segmentIdMappings = new Map(); + get empty() { + return this.nodeIdMappings.size === 0 && this.segmentIdMappings.size === 0; + } + clear() { this.nodeIdMappings.clear(); this.segmentIdMappings.clear(); @@ -241,10 +245,15 @@ export class SpatialSkeletonCommandHistory extends RefCounted { } clear() { + const changed = + this.undoEntries.length !== 0 || + this.redoEntries.length !== 0 || + !this.mappings.empty; this.undoEntries = []; this.redoEntries = []; this.mappings.clear(); this.updateState(); + return changed; } setSource(source: unknown) { diff --git a/src/skeleton/commands.ts b/src/skeleton/commands.ts index d7c10abe89..084de70e3c 100644 --- a/src/skeleton/commands.ts +++ b/src/skeleton/commands.ts @@ -352,8 +352,11 @@ export function executeSpatialSkeletonMerge( export async function undoSpatialSkeletonCommand( layer: SpatialSkeletonLayerContext, ) { - const changed = await layer.spatialSkeletonState.commandHistory.undo(); - if (!changed) { + const { commandHistory } = layer.spatialSkeletonState; + if (commandHistory.isBusy.value) { + StatusMessage.showTemporaryMessage( + "Wait for the current skeleton edit to finish.", + ); return false; } const optimisticEditState = isSpatialSkeletonOptimisticEditState( @@ -364,7 +367,6 @@ export async function undoSpatialSkeletonCommand( if (optimisticEditState?.canUndoOptimisticEdit() === true) { return optimisticEditState.undoLatestOptimisticEdit(); } - const commandHistory = layer.spatialSkeletonState.commandHistory; if (!commandHistory.canUndo.value) { return false; } @@ -380,8 +382,11 @@ export async function undoSpatialSkeletonCommand( export async function redoSpatialSkeletonCommand( layer: SpatialSkeletonLayerContext, ) { - const changed = await layer.spatialSkeletonState.commandHistory.redo(); - if (!changed) { + const { commandHistory } = layer.spatialSkeletonState; + if (commandHistory.isBusy.value) { + StatusMessage.showTemporaryMessage( + "Wait for the current skeleton edit to finish.", + ); return false; } const optimisticEditState = isSpatialSkeletonOptimisticEditState( diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index 2766d16ec1..1ac26c5fbd 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -216,6 +216,7 @@ const DEFAULT_FRAGMENT_MAIN = `void main() { emitDefault(); } `; +const SELECTED_NODE_OUTLINE_FALLBACK_COLOR = vec3.fromValues(1.0, 0.95, 0.35); // Converts a linear 0..1 RGB triple to a CSS `rgb(...)` string for DOM markers. function vec3ToCssColor(color: vec3): string { @@ -770,6 +771,7 @@ vec4 getSegmentAppearance(highp uint segmentValue) { : this.defineEdgeRaycastCylinder(builder, skeletonParams); const path = this.dynamicColorPath(skeletonParams) ? "dynamic" : "legacy"; builder.addFragmentCode(edgeColorPathsGlsl(path, geometry.shading)); + builder.addFragmentCode(glsl_string); this.finalizeShaderBuilder( builder, shaderBuilderState, @@ -792,6 +794,7 @@ vec4 getSegmentAppearance(highp uint segmentValue) { builder.addFragmentCode( nodeColorPathsGlsl(path, geometry.legacyPremultiply), ); + builder.addFragmentCode(glsl_string); this.finalizeShaderBuilder( builder, shaderBuilderState, @@ -1954,6 +1957,46 @@ export class SpatiallyIndexedSkeletonSource extends SliceViewChunkSource< } } +export interface SpatiallyIndexedSkeletonSourceRuntimeDisposalOptions { + invalidateCache?: boolean; +} + +export function disposeSpatiallyIndexedSkeletonSourceRuntimeState( + sources: Iterable, + options: SpatiallyIndexedSkeletonSourceRuntimeDisposalOptions = {}, +) { + const uniqueSources = new Set(sources); + const invalidateCache = options.invalidateCache ?? true; + const chunkQueueManagersWithDeletedChunks = new Set< + ChunkManager["chunkQueueManager"] + >(); + let changed = false; + for (const source of uniqueSources) { + if (source.chunks.size !== 0) { + for (const chunkKey of source.chunks.keys()) { + source.deleteChunk(chunkKey); + } + chunkQueueManagersWithDeletedChunks.add( + source.chunkManager.chunkQueueManager, + ); + changed = true; + } + if ( + invalidateCache && + source.wasDisposed !== true && + source.rpc !== null && + source.rpcId !== null + ) { + source.invalidateCache(); + changed = true; + } + } + for (const chunkQueueManager of chunkQueueManagersWithDeletedChunks) { + chunkQueueManager.visibleChunksChanged.dispatch(); + } + return changed; +} + // Options are provided by the SliceView framework for scale selection, // but spatial skeleton sources expose all grid levels unconditionally. // TODO (SKM): validate if this is an ok deviation from the SliceView @@ -2253,10 +2296,10 @@ export class SpatiallyIndexedSkeletonLayer private cachedOverlayRenderRetainedVersion = -1; private maxRetainedOverlaySegments: number; private readonly selectedNodeOutlineColor = vec3.clone( - ACTIVE_NODE_BORDER_FALLBACK_COLOR, + SELECTED_NODE_OUTLINE_FALLBACK_COLOR, ); private readonly highlightedNodeOutlineColor = vec3.clone( - ACTIVE_NODE_BORDER_FALLBACK_COLOR, + SELECTED_NODE_OUTLINE_FALLBACK_COLOR, ); // The selected and hovered outline colors are derived together from a single // source segment color, so they share one cache generation. @@ -2412,7 +2455,7 @@ export class SpatiallyIndexedSkeletonLayer } else { vec3.copy( this.selectedNodeOutlineColor, - ACTIVE_NODE_BORDER_FALLBACK_COLOR, + SELECTED_NODE_OUTLINE_FALLBACK_COLOR, ); } @@ -2433,7 +2476,7 @@ export class SpatiallyIndexedSkeletonLayer } else { vec3.copy( this.highlightedNodeOutlineColor, - ACTIVE_NODE_BORDER_FALLBACK_COLOR, + SELECTED_NODE_OUTLINE_FALLBACK_COLOR, ); } } @@ -2656,7 +2699,7 @@ export class SpatiallyIndexedSkeletonLayer ) { super(); this.registerDisposer(() => { - this.disposeOverlayChunk(); + this.disposeRuntimeState(); }); let sources3d: SpatiallyIndexedSkeletonSourceEntry[]; let sources2d = options.sources2d ?? []; diff --git a/src/status.ts b/src/status.ts index 5713c3737c..4c60190c41 100644 --- a/src/status.ts +++ b/src/status.ts @@ -146,9 +146,9 @@ export class StatusMessage { if (this.modalElementWrapper !== undefined) { modalStatusContainer!.removeChild(this.modalElementWrapper); this.modalElementWrapper = undefined; - getStatusContainer().appendChild(this.element); + getStatusContainer().prepend(this.element); } else if (this.element.parentElement === null) { - getStatusContainer().appendChild(this.element); + getStatusContainer().prepend(this.element); } } } diff --git a/src/ui/default_input_event_bindings.ts b/src/ui/default_input_event_bindings.ts index ff6512a969..356d2e3469 100644 --- a/src/ui/default_input_event_bindings.ts +++ b/src/ui/default_input_event_bindings.ts @@ -31,6 +31,7 @@ import { SKELETON_PIN_NODE, SKELETON_REDO, SKELETON_REROOT, + SKELETON_TOGGLE_HIDDEN, SKELETON_TOGGLE_TRUE_END, SKELETON_UNDO, } from "#src/skeleton/actions.js"; @@ -64,6 +65,7 @@ export function getDefaultGlobalBindings() { map.set("keyn", "add-layer"); map.set("keyh", "help"); + map.set("keyg", SKELETON_TOGGLE_HIDDEN); map.set("space", "toggle-layout"); map.set("shift+space", "toggle-layout-alternative"); diff --git a/src/ui/skeleton_edit_tool_messages.spec.ts b/src/ui/skeleton_edit_tool_messages.spec.ts index 5372d1c87b..b0c4161b1b 100644 --- a/src/ui/skeleton_edit_tool_messages.spec.ts +++ b/src/ui/skeleton_edit_tool_messages.spec.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; +import { + formatSpatialSkeletonToolPoint, + getSpatialSkeletonCreateIdleStatusText, + getSpatialSkeletonCreatingStatusText, getSpatialSkeletonDefaultStatusText, getSpatialSkeletonDeleteIdleStatusText, getSpatialSkeletonDeletingStatusText, @@ -9,6 +13,8 @@ import { describe, expect, it } from "vitest"; getSpatialSkeletonSplitIdleStatusText, getSpatialSkeletonSplittingStatusText, getSpatialSkeletonToolPointSummaryRow, + getSpatialSkeletonToolPointStatusFields, +} from "#src/ui/skeleton_edit_tool_messages.js"; import { ADD_NODE_ACTION, DELETE_ACTION, @@ -64,6 +70,8 @@ describe("spatial_skeleton_tool_messages", () => { describe("getSpatialSkeletonDefaultStatusText", () => { it("no selection", () => { + expect(getSpatialSkeletonDefaultStatusText("none", false)).toEqual({ + status: "No selection", actions: [ SELECT_ACTION, MOVE_ACTION, @@ -79,6 +87,8 @@ describe("spatial_skeleton_tool_messages", () => { it("selected, visible skeleton", () => { expect( getSpatialSkeletonDefaultStatusText("selected-visible", false), + ).toEqual({ + status: "Node selected", actions: [ SELECT_ACTION, MOVE_ACTION, @@ -95,6 +105,8 @@ describe("spatial_skeleton_tool_messages", () => { it("selected, visible skeleton, shift held", () => { expect( getSpatialSkeletonDefaultStatusText("selected-visible", true), + ).toEqual({ + status: "Ready to place new node", actions: [ SELECT_ACTION, MOVE_ACTION, @@ -111,6 +123,8 @@ describe("spatial_skeleton_tool_messages", () => { it("selected, non-visible skeleton", () => { expect( getSpatialSkeletonDefaultStatusText("selected-hidden", false), + ).toEqual({ + status: "Node selected from non-visible skeleton", actions: [ SHOW_SKELETON_ACTION, MERGE_ACTION, @@ -125,6 +139,8 @@ describe("spatial_skeleton_tool_messages", () => { it("selected, non-visible skeleton, shift held — unaffected by shift", () => { expect( getSpatialSkeletonDefaultStatusText("selected-hidden", true), + ).toEqual({ + status: "Node selected from non-visible skeleton", actions: [ SHOW_SKELETON_ACTION, MERGE_ACTION, @@ -138,12 +154,16 @@ describe("spatial_skeleton_tool_messages", () => { }); it("returns a static moving-node status", () => { + expect(getSpatialSkeletonMovingStatusText()).toEqual({ + status: "Moving node", 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: [ SELECT_ACTION, EXIT_MERGE_ACTION, @@ -153,6 +173,8 @@ describe("spatial_skeleton_tool_messages", () => { }); it("no from node, key not held", () => { + expect(getSpatialSkeletonMergeStatusText("no-from-node", false)).toEqual({ + status: "Merge · click a node to merge from", actions: [SELECT_ACTION, SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); @@ -160,6 +182,8 @@ describe("spatial_skeleton_tool_messages", () => { 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: [ SELECT_ACTION, EXIT_MERGE_ACTION, @@ -171,6 +195,8 @@ describe("spatial_skeleton_tool_messages", () => { 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: [SELECT_ACTION, SPATIAL_SKELETON_ROTATE_PAN_ACTION], }); }); @@ -178,6 +204,8 @@ describe("spatial_skeleton_tool_messages", () => { 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: [ SHOW_SKELETON_ACTION, EXIT_MERGE_ACTION, @@ -189,18 +217,24 @@ describe("spatial_skeleton_tool_messages", () => { 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: [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_ACTION], }); }); describe("getSpatialSkeletonSplitIdleStatusText", () => { it("key held", () => { + expect(getSpatialSkeletonSplitIdleStatusText(true)).toEqual({ + status: "Split · click a node to form the root of a new skeleton", actions: [ SELECT_ACTION, EXIT_SPLIT_ACTION, @@ -210,18 +244,24 @@ describe("spatial_skeleton_tool_messages", () => { }); it("key not held", () => { + expect(getSpatialSkeletonSplitIdleStatusText(false)).toEqual({ + status: "Split · click a node to form the root of a new skeleton", 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_ACTION], }); }); describe("getSpatialSkeletonDeleteIdleStatusText", () => { it("key held", () => { + expect(getSpatialSkeletonDeleteIdleStatusText(true)).toEqual({ + status: "Delete · no selected nodes", actions: [ DELETE_CLICK_ACTION, EXIT_DELETE_ACTION, @@ -231,18 +271,24 @@ describe("spatial_skeleton_tool_messages", () => { }); it("key not held", () => { + expect(getSpatialSkeletonDeleteIdleStatusText(false)).toEqual({ + status: "Delete · no selected nodes", 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_ACTION], }); }); describe("getSpatialSkeletonCreateIdleStatusText", () => { it("key held", () => { + expect(getSpatialSkeletonCreateIdleStatusText(true)).toEqual({ + status: "Create · ready to place", actions: [ PLACE_ACTION, EXIT_CREATE_ACTION, @@ -252,12 +298,16 @@ describe("spatial_skeleton_tool_messages", () => { }); it("key not held", () => { + expect(getSpatialSkeletonCreateIdleStatusText(false)).toEqual({ + status: "Create · ready to place", 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_ACTION], }); }); diff --git a/src/ui/skeleton_edit_tool_messages.ts b/src/ui/skeleton_edit_tool_messages.ts index a88ecca542..b9ba06be22 100644 --- a/src/ui/skeleton_edit_tool_messages.ts +++ b/src/ui/skeleton_edit_tool_messages.ts @@ -109,6 +109,18 @@ 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 +// internal name (mergeAnchorNodeId, etc.) is unaffected. + export type SpatialSkeletonDefaultSelectionState = | "none" | "selected-visible" diff --git a/src/ui/skeleton_edit_tools.ts b/src/ui/skeleton_edit_tools.ts index 3c46920a8d..d550203e2c 100644 --- a/src/ui/skeleton_edit_tools.ts +++ b/src/ui/skeleton_edit_tools.ts @@ -62,16 +62,23 @@ import { StatusMessage } from "#src/status.js"; import { getDefaultSkeletonEditAuxBindings, getDefaultSkeletonEditNodeBindings, - + getDefaultSkeletonEditToolBindings, +} from "#src/ui/default_input_event_bindings.js"; +import { + getSpatialSkeletonCreateIdleStatusText, + getSpatialSkeletonCreatingStatusText, + getSpatialSkeletonDefaultStatusText, getSpatialSkeletonDeleteIdleStatusText, getSpatialSkeletonDeletingStatusText, getSpatialSkeletonMergeStatusText, getSpatialSkeletonMovingStatusText, + getSpatialSkeletonSplitIdleStatusText, +} from "#src/ui/skeleton_edit_tool_messages.js"; +import type { SpatialSkeletonToolStatusText } from "#src/ui/skeleton_edit_tool_shortcuts.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, @@ -145,11 +152,17 @@ function renderSpatialSkeletonToolStatus( body: HTMLElement, text: SpatialSkeletonToolStatusText, ) { - + removeChildren(body); + body.classList.add("neuroglancer-skeleton-tool-status"); + 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"; for (const shortcut of text.actions) { actionsElement.appendChild(renderSpatialSkeletonShortcut(shortcut)); } @@ -1396,12 +1409,16 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { ); if (disabledReason !== undefined) { StatusMessage.showTemporaryMessage(disabledReason); + renderSpatialSkeletonToolStatus(body, { + status: disabledReason, 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: [] }); queueMicrotask(() => activation.cancel()); return; diff --git a/src/viewer.ts b/src/viewer.ts index 0cd73f41da..9b1165096c 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -82,6 +82,7 @@ import { SKELETON_GO_ROOT, SKELETON_GO_UNFINISHED, SKELETON_REDO, + SKELETON_TOGGLE_HIDDEN, SKELETON_UNDO, } from "#src/skeleton/actions.js"; import { StatusMessage } from "#src/status.js"; @@ -1108,7 +1109,11 @@ export class Viewer extends RefCounted implements ViewerState { * Called once by the constructor to register the action listeners. */ private registerActionListeners() { - for (const action of ["recolor", "clear-segments"]) { + for (const action of [ + "recolor", + "clear-segments", + SKELETON_TOGGLE_HIDDEN, + ]) { this.bindAction(action, () => { this.layerManager.invokeAction(action); });