From 4b4c0d20114b04248245417bfc9f1c43ffd9e33b Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Thu, 24 Sep 2026 20:43:05 +0200 Subject: [PATCH 1/2] feat: remove picking indicator The picking indicator was extracted and the overlay approach was changed so the node highlight here uses the new approach. One important note, we suppressed the picking indicator before this. Could add back if picking indicator merged. --- src/display_context.ts | 56 ------ src/layer/index.ts | 21 --- src/panel_overlay.css | 10 - src/panel_overlay.spec.ts | 179 ++++++++++++++++++ src/panel_overlay.ts | 285 ++++++++++------------------- src/perspective_view/panel.ts | 65 +------ src/picking_indicator_overlay.css | 32 ---- src/picking_indicator_overlay.ts | 69 ------- src/rendered_data_panel.ts | 99 ++++------ src/skeleton/frontend.ts | 167 +++++++++-------- src/sliceview/panel.ts | 38 +--- src/ui/skeleton_edit_tools.spec.ts | 10 - src/ui/skeleton_edit_tools.ts | 19 +- src/viewer.ts | 7 - 14 files changed, 409 insertions(+), 648 deletions(-) create mode 100644 src/panel_overlay.spec.ts delete mode 100644 src/picking_indicator_overlay.css delete mode 100644 src/picking_indicator_overlay.ts diff --git a/src/display_context.ts b/src/display_context.ts index 2219ac0870..b15d64a68f 100644 --- a/src/display_context.ts +++ b/src/display_context.ts @@ -17,10 +17,6 @@ import { debounce } from "lodash-es"; import type { FrameNumberCounter } from "#src/chunk_manager/frontend.js"; -import type { - PanelOverlaySource, - PanelOverlayTarget, -} from "#src/panel_overlay.js"; import { TrackableValue } from "#src/trackable_value.js"; import { animationFrameDebounce } from "#src/util/animation_frame_debounce.js"; import type { Borrowed } from "#src/util/disposable.js"; @@ -306,16 +302,6 @@ export abstract class RenderedPanel extends RefCounted { abstract draw(): void; - // Repositions this panel's DOM overlays. Default no-op; overridden by panels - // that support overlays. - updateOverlays(): void {} - - scheduleOverlayUpdate(): void { - if (this.visible) { - this.context.scheduleOverlayUpdate(); - } - } - disposed() { this.context.unmonitorPanel(this.element, this.monitorState); this.context.removePanel(this); @@ -654,45 +640,6 @@ export class DisplayContext extends RefCounted implements FrameNumberCounter { animationFrameDebounce(() => this.draw()), ); - // Overlay sources shown on data panels, each with its optional panel-type - // target. Panels observe `panelOverlaysChanged` to add/remove their bindings. - readonly panelOverlays = new Map(); - readonly panelOverlaysChanged = new NullarySignal(); - - /** - * Registers an overlay source shown on the data panels matching `target` (every - * data panel by default). Returns a disposer that removes it. - */ - registerPanelOverlay( - source: PanelOverlaySource, - target: PanelOverlayTarget = {}, - ): () => void { - this.panelOverlays.set(source, target); - this.panelOverlaysChanged.dispatch(); - return () => { - if (this.panelOverlays.delete(source)) { - this.panelOverlaysChanged.dispatch(); - } - }; - } - - // Repositions DOM overlays across all panels, coalesced per animation frame - // and independent of `scheduleRedraw`. - readonly scheduleOverlayUpdate = this.registerCancellable( - animationFrameDebounce(() => this.updateOverlays()), - ); - - private updateOverlays() { - this.ensureBoundsUpdated(); - for (const panel of this.panels) { - if (!panel.shouldDraw) continue; - panel.ensureBoundsUpdated(); - const { renderViewport } = panel; - if (renderViewport.width === 0 || renderViewport.height === 0) continue; - panel.updateOverlays(); - } - } - ensureBoundsUpdated() { const { resizeGeneration } = this; if (this.boundsGeneration === resizeGeneration) return; @@ -737,9 +684,6 @@ export class DisplayContext extends RefCounted implements FrameNumberCounter { this.updateFinished.dispatch(); this.framerateMonitor.endLastTimeQuery(gl, ext); this.framerateMonitor.grabAnyFinishedQueryResults(gl); - // Each panel's draw() already updated its overlays, so drop any pending - // overlay-only update. - this.scheduleOverlayUpdate.cancel(); } getDepthArray(): Float32Array { diff --git a/src/layer/index.ts b/src/layer/index.ts index e69488b55e..12690d9f4c 100644 --- a/src/layer/index.ts +++ b/src/layer/index.ts @@ -56,8 +56,6 @@ import { PlaybackManager, Position, } from "#src/navigation_state.js"; -import type { PanelOverlaySource } from "#src/panel_overlay.js"; -import { isPanelOverlaySource } from "#src/panel_overlay.js"; import type { RenderLayerTransform } from "#src/render_coordinate_transform.js"; import { RENDERED_VIEW_ADD_LAYER_RPC_ID, @@ -1167,10 +1165,6 @@ 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; @@ -1695,21 +1689,6 @@ export function makeRenderedPanelVisibleLayerTracker< info.registerDisposer( layer.redrawNeeded.add(() => panel.scheduleRedraw()), ); - // Layers that contribute DOM panel overlays (e.g. skeleton - // selected/hovered node highlights) are bound to this panel; the binding - // (container + update wiring) is scoped to this per-(layer,panel) info. - const overlayPanel = panel as Partial<{ - bindOverlaySource( - source: PanelOverlaySource, - owner: RefCounted, - ): void; - }>; - if ( - isPanelOverlaySource(layer) && - typeof overlayPanel.bindOverlaySource === "function" - ) { - overlayPanel.bindOverlaySource(layer, info); - } const { backend } = layer; if (backend) { backend.rpc!.invoke(RENDERED_VIEW_ADD_LAYER_RPC_ID, { diff --git a/src/panel_overlay.css b/src/panel_overlay.css index e83787d330..22b5d60726 100644 --- a/src/panel_overlay.css +++ b/src/panel_overlay.css @@ -14,8 +14,6 @@ * limitations under the License. */ -/* Per-panel overlay container: covers the panel, never intercepts pointer - events, and clips overlays to the panel bounds. */ .neuroglancer-panel-overlay-container { position: absolute; inset: 0; @@ -23,11 +21,3 @@ z-index: 10; overflow: hidden; } - -/* Per-source sub-container within a panel; z-index is set from the source's - overlayPriority. */ -.neuroglancer-panel-overlay-source { - position: absolute; - inset: 0; - pointer-events: none; -} diff --git a/src/panel_overlay.spec.ts b/src/panel_overlay.spec.ts new file mode 100644 index 0000000000..859ab5dd90 --- /dev/null +++ b/src/panel_overlay.spec.ts @@ -0,0 +1,179 @@ +/** + * @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 { + applyRenderViewportToProjectionMatrix, + RenderViewport, +} from "#src/display_context.js"; +import type { PanelOverlayHost } from "#src/panel_overlay.js"; +import { PanelOverlayManager, projectToViewport } from "#src/panel_overlay.js"; +import type { ProjectionParameters } from "#src/projection_parameters.js"; +import { RefCounted } from "#src/util/disposable.js"; +import { mat4 } from "#src/util/geom.js"; + +function makeParameters(options: { + projectionMat: mat4; + focalDistance: number; + logicalWidth: number; + logicalHeight: number; + visibleRegion?: { + leftFraction: number; + topFraction: number; + widthFraction: number; + heightFraction: number; + }; +}): ProjectionParameters { + const { + focalDistance, + logicalWidth, + logicalHeight, + visibleRegion = { + leftFraction: 0, + topFraction: 0, + widthFraction: 1, + heightFraction: 1, + }, + } = options; + const renderViewport = Object.assign(new RenderViewport(), { + logicalWidth, + logicalHeight, + visibleLeftFraction: visibleRegion.leftFraction, + visibleTopFraction: visibleRegion.topFraction, + visibleWidthFraction: visibleRegion.widthFraction, + visibleHeightFraction: visibleRegion.heightFraction, + }); + const projectionMat = mat4.clone(options.projectionMat); + applyRenderViewportToProjectionMatrix(renderViewport, projectionMat); + const viewMatrix = mat4.fromTranslation(mat4.create(), [ + 0, + 0, + -focalDistance, + ]); + return { + ...renderViewport, + projectionMat, + viewProjectionMat: mat4.multiply(mat4.create(), projectionMat, viewMatrix), + displayDimensionRenderInfo: { + displayDimensionIndices: Int32Array.of(0, 1, 2), + }, + } as unknown as ProjectionParameters; +} + +function makeDotOverlay(host: PanelOverlayHost) { + const dot = document.createElement("div"); + host.container.appendChild(dot); + return Object.assign(new RefCounted(), { + update() {}, + disposed() { + dot.remove(); + }, + }); +} + +describe("projectToViewport", () => { + it("puts the focal point at the panel center, on the focal plane", () => { + for (const projectionMat of [ + mat4.perspective(mat4.create(), Math.PI / 2, 2, 0.5, 1.5), + mat4.ortho(mat4.create(), -2, 2, -1, 1, 0.5, 1.5), + ]) { + const parameters = makeParameters({ + projectionMat, + focalDistance: 1, + logicalWidth: 200, + logicalHeight: 100, + }); + const point = projectToViewport(parameters, [0, 0, 0])!; + expect(point.viewportLeft).toBeCloseTo(100); + expect(point.viewportTop).toBeCloseTo(50); + expect(point.focalPlaneDepthFraction).toBeCloseTo(0); + } + }); + + it("keeps the focal point at the panel center when part of the panel is clipped", () => { + const parameters = makeParameters({ + projectionMat: mat4.perspective(mat4.create(), Math.PI / 2, 2, 0.5, 1.5), + focalDistance: 1, + logicalWidth: 200, + logicalHeight: 100, + visibleRegion: { + leftFraction: 0.5, + topFraction: 0.25, + widthFraction: 0.5, + heightFraction: 0.75, + }, + }); + const point = projectToViewport(parameters, [0, 0, 0])!; + expect(point.viewportLeft).toBeCloseTo(100); + expect(point.viewportTop).toBeCloseTo(50); + }); + + it("gives no position for a point beyond the far plane", () => { + for (const projectionMat of [ + mat4.perspective(mat4.create(), Math.PI / 2, 2, 0.5, 1.5), + mat4.ortho(mat4.create(), -2, 2, -1, 1, 0.5, 1.5), + ]) { + const parameters = makeParameters({ + projectionMat, + focalDistance: 1, + logicalWidth: 200, + logicalHeight: 100, + }); + const twiceTheFocalDistance = [0, 0, -1]; + expect(projectToViewport(parameters, twiceTheFocalDistance)).toBe( + undefined, + ); + } + }); + + it("measures depth linearly from the focal plane under both projections", () => { + const perspective = makeParameters({ + projectionMat: mat4.perspective(mat4.create(), Math.PI / 2, 2, 0.5, 1.5), + focalDistance: 1, + logicalWidth: 200, + logicalHeight: 100, + }); + const orthographic = makeParameters({ + projectionMat: mat4.ortho(mat4.create(), -2, 2, -1, 1, 0.5, 1.5), + focalDistance: 1, + logicalWidth: 200, + logicalHeight: 100, + }); + const halfWayToFar = [0, 0, -0.25]; + expect( + projectToViewport(perspective, halfWayToFar)!.focalPlaneDepthFraction, + ).toBeCloseTo(0.5); + expect( + projectToViewport(orthographic, halfWayToFar)!.focalPlaneDepthFraction, + ).toBeCloseTo(0.5); + }); +}); + +describe("PanelOverlayManager", () => { + it("removes an overlay from the panel while other overlays stay", () => { + const panel = document.createElement("div"); + const manager = new PanelOverlayManager( + panel, + () => undefined, + () => true, + ); + const removeFirst = manager.add(makeDotOverlay); + manager.add(makeDotOverlay); + removeFirst(); + manager.update(); + expect(manager.container.children.length).toBe(1); + }); +}); diff --git a/src/panel_overlay.ts b/src/panel_overlay.ts index 15caf5922e..fbad3c380e 100644 --- a/src/panel_overlay.ts +++ b/src/panel_overlay.ts @@ -14,223 +14,124 @@ * limitations under the License. */ -/** - * DOM overlays positioned by projecting world-space positions to screen (e.g. - * the picking indicator and skeleton node highlights), updated on a coalesced, - * redraw-free pass independent of the WebGL render loop. - * - * The contract contains no neuroglancer-internal types, so render layers, - * built-ins, and external code implement it identically. Positions passed to - * `PanelOverlayContext.project` are in the global coordinate space. - */ - import "#src/panel_overlay.css"; -import type { WatchableValueInterface } from "#src/trackable_value.js"; +import type { CoordinateSpace } from "#src/coordinate_transform.js"; +import type { ProjectionParameters } from "#src/projection_parameters.js"; +import { animationFrameDebounce } from "#src/util/animation_frame_debounce.js"; +import type { Disposable } from "#src/util/disposable.js"; import { RefCounted } from "#src/util/disposable.js"; -import type { NullarySignal } from "#src/util/signal.js"; - -export interface PanelOverlayContext { - /** - * Projects a global-coordinate position to this panel's logical CSS pixels, or - * returns `undefined` if it is off-screen / behind the camera / culled by the - * cross-section slab. `scale` (default 1) conveys depth (perspective view); - * `opacity` (default 1) is the cross-section fade in slice views (1 on the - * slice plane, falling to 0 at the slab edge). - */ - project( - position: Float32Array, - ): { x: number; y: number; scale?: number; opacity?: number } | undefined; - - /** - * The source's container for this panel; the source reconciles its children. - * Created and removed by the panel. - */ - readonly container: HTMLElement; +import { getViewFrustumDepthRange, vec4 } from "#src/util/geom.js"; - /** - * CSS pixels per render-viewport device pixel, for sizing overlays specified in - * device pixels. - */ - readonly cssPerDevicePixel: number; - - /** - * The panel's type tags (e.g. `"perspective"`, `"cross-section"`), so a source - * can adapt its rendering to the panel it is drawing in. - */ - readonly panelTypes: readonly string[]; +export interface ViewportPoint { + readonly viewportLeft: number; + readonly viewportTop: number; + readonly focalPlaneDepthFraction: number; } -export interface PanelOverlaySource { - /** Higher draws on top of lower. Default 0. (Picking indicator uses 100.) */ - readonly overlayPriority?: number; - - /** Dispatch to reposition the overlay on the next frame without a GL redraw. */ - readonly overlayUpdateNeeded: NullarySignal; - - /** - * Optional runtime show/hide. When present and `false`, the panel hides this - * source's container and skips its update; changes trigger a coalesced pass. - */ - readonly overlayVisible?: WatchableValueInterface; - - /** Cheap, DOM-only update for one panel. Must not touch the GL canvas. */ - updatePanelOverlays(ctx: PanelOverlayContext): void; -} - -export function isPanelOverlaySource(x: unknown): x is PanelOverlaySource { - return ( - typeof (x as Partial | null | undefined) - ?.updatePanelOverlays === "function" - ); +const tempClip = vec4.create(); + +export function projectToViewport( + parameters: ProjectionParameters, + position: ArrayLike, +): ViewportPoint | undefined { + const { + projectionMat, + viewProjectionMat, + logicalWidth, + logicalHeight, + visibleLeftFraction, + visibleTopFraction, + visibleWidthFraction, + visibleHeightFraction, + displayDimensionRenderInfo: { displayDimensionIndices }, + } = parameters; + const clip = tempClip; + for (let i = 0; i < 3; ++i) { + const index = displayDimensionIndices[i]; + clip[i] = index >= 0 ? position[index] : 0; + } + clip[3] = 1; + vec4.transformMat4(clip, clip, viewProjectionMat); + const w = clip[3]; + if (w <= 0) return undefined; + const normalizedDeviceZ = clip[2] / w; + if (normalizedDeviceZ < -1 || normalizedDeviceZ > 1) return undefined; + const orthographic = projectionMat[15] === 1; + return { + viewportLeft: + (visibleLeftFraction + + ((clip[0] / w) * 0.5 + 0.5) * visibleWidthFraction) * + logicalWidth, + viewportTop: + (visibleTopFraction + + (0.5 - (clip[1] / w) * 0.5) * visibleHeightFraction) * + logicalHeight, + focalPlaneDepthFraction: orthographic + ? normalizedDeviceZ + : (w - 1) / (getViewFrustumDepthRange(projectionMat) / 2), + }; } -/** - * Restricts a globally-registered source to a subset of panels. When - * `panelTypes` is omitted the source is shown on every data panel; otherwise it - * is shown on a panel iff one of its {@link PanelOverlayHost.panelTypes} tags is - * listed. (An empty `panelTypes` therefore matches no panel.) - */ -export interface PanelOverlayTarget { - readonly panelTypes?: readonly string[]; -} +export type ProjectOverlayPosition = ( + position: Float32Array, + coordinateSpace: CoordinateSpace, +) => ViewportPoint | undefined; -function panelMatchesTarget( - target: PanelOverlayTarget, - panelTypes: readonly string[], -): boolean { - const { panelTypes: wanted } = target; - return ( - wanted === undefined || wanted.some((type) => panelTypes.includes(type)) - ); +export interface PanelOverlayHost { + readonly container: HTMLElement; + readonly project: ProjectOverlayPosition; + /** Updates the overlays at the next animation frame without a redraw. */ + scheduleUpdate(): void; } -/** The panel capabilities required by {@link PanelOverlayManager}. */ -export interface PanelOverlayHost { - readonly element: HTMLElement; - readonly visible: boolean; - readonly cssPerDevicePixel: number; - readonly panelTypes: readonly string[]; - project( - position: Float32Array, - ): { x: number; y: number; scale?: number; opacity?: number } | undefined; +export interface PanelOverlay extends Disposable { + update(): void; } -/** - * Owns a panel's overlay DOM and drives its updates. Holds a per-panel - * container with one child per bound {@link PanelOverlaySource} (z-index from - * `overlayPriority`), binds the viewer-level sources registered on the - * DisplayContext, and repositions every source on `update()`. - */ -export class PanelOverlayManager extends RefCounted { - private readonly container = document.createElement("div"); - private readonly bindings = new Map(); - private readonly globalOwners = new Map(); +export class PanelOverlayManager + extends RefCounted + implements PanelOverlayHost +{ + readonly container = document.createElement("div"); + private readonly overlays: PanelOverlay[] = []; + readonly scheduleUpdate = this.registerCancellable( + animationFrameDebounce(() => { + if (this.isDrawable()) this.update(); + else this.hide(); + }), + ); constructor( - private readonly host: PanelOverlayHost, - // Viewer-level sources (with their optional panel-type target) applied to this - // panel when the target matches, and the signal fired when the map changes. - private readonly globalSources: ReadonlyMap< - PanelOverlaySource, - PanelOverlayTarget - >, - globalSourcesChanged: NullarySignal, - // Requests a coalesced, redraw-free overlay pass. - private readonly requestUpdate: () => void, + panelElement: HTMLElement, + readonly project: ProjectOverlayPosition, + private readonly isDrawable: () => boolean, ) { super(); this.container.className = "neuroglancer-panel-overlay-container"; - host.element.appendChild(this.container); + panelElement.appendChild(this.container); this.registerDisposer(() => this.container.remove()); - this.registerDisposer( - globalSourcesChanged.add(() => this.syncGlobalSources()), - ); - this.registerDisposer(() => { - for (const owner of this.globalOwners.values()) owner.dispose(); - this.globalOwners.clear(); - }); - this.syncGlobalSources(); - } - - /** - * Binds `source`. `owner` scopes the binding's lifetime; the source's - * sub-container is removed when `owner` is disposed. - */ - bindSource(source: PanelOverlaySource, owner: RefCounted) { - const subContainer = document.createElement("div"); - subContainer.className = "neuroglancer-panel-overlay-source"; - subContainer.style.zIndex = `${source.overlayPriority ?? 0}`; - this.container.appendChild(subContainer); - this.bindings.set(source, subContainer); - owner.registerDisposer(() => { - subContainer.remove(); - this.bindings.delete(source); - this.requestUpdate(); - }); - owner.registerDisposer(source.overlayUpdateNeeded.add(this.requestUpdate)); - const { overlayVisible } = source; - if (overlayVisible !== undefined) { - owner.registerDisposer(overlayVisible.changed.add(this.requestUpdate)); - } - this.requestUpdate(); } - private syncGlobalSources() { - const { globalSources, globalOwners, host } = this; - for (const [source, owner] of globalOwners) { - const target = globalSources.get(source); - if ( - target === undefined || - !panelMatchesTarget(target, host.panelTypes) - ) { - owner.dispose(); - globalOwners.delete(source); - } - } - for (const [source, target] of globalSources) { - if ( - !globalOwners.has(source) && - panelMatchesTarget(target, host.panelTypes) - ) { - const owner = new RefCounted(); - globalOwners.set(source, owner); - this.bindSource(source, owner); - } - } + /** Overlays added later draw on top. Returns a function that removes the overlay. */ + add(createOverlay: (host: PanelOverlayHost) => PanelOverlay) { + const overlay = this.registerDisposer(createOverlay(this)); + this.overlays.push(overlay); + return () => { + this.unregisterDisposer(overlay); + this.overlays.splice(this.overlays.indexOf(overlay), 1); + overlay.dispose(); + }; } - /** Repositions every bound source. DOM only; does not touch the GL canvas. */ update() { - const { host } = this; - if (!host.visible) return; - const { cssPerDevicePixel, panelTypes } = host; - const project = (p: Float32Array) => host.project(p); - for (const [source, container] of this.bindings) { - if (source.overlayVisible?.value === false) { - if (container.style.display !== "none") - container.style.display = "none"; - continue; - } - if (container.style.display === "none") container.style.display = ""; - source.updatePanelOverlays({ - project, - container, - cssPerDevicePixel, - panelTypes, - }); - } + this.scheduleUpdate.cancel(); + this.container.hidden = false; + for (const overlay of this.overlays) overlay.update(); } - /** - * Hides every bound source's container without touching the GL canvas. Used - * when the panel failed to draw (so it rendered nothing this frame) to avoid - * leaving stale overlays over cleared canvas content. {@link update} - * restores visibility on the next successful frame. - */ - clear() { - for (const container of this.bindings.values()) { - if (container.style.display !== "none") container.style.display = "none"; - } + hide() { + this.scheduleUpdate.cancel(); + this.container.hidden = true; } } diff --git a/src/perspective_view/panel.ts b/src/perspective_view/panel.ts index 6b3802dcc0..2ece55e770 100644 --- a/src/perspective_view/panel.ts +++ b/src/perspective_view/panel.ts @@ -23,6 +23,7 @@ import type { DisplayContext } from "#src/display_context.js"; import { applyRenderViewportToProjectionMatrix } from "#src/display_context.js"; import type { VisibleRenderLayerTracker } from "#src/layer/index.js"; import { makeRenderedPanelVisibleLayerTracker } from "#src/layer/index.js"; +import { projectToViewport } from "#src/panel_overlay.js"; import { PERSPECTIVE_VIEW_RPC_ID } from "#src/perspective_view/base.js"; import type { PerspectiveViewReadyRenderContext, @@ -200,11 +201,6 @@ const tempVec3 = vec3.create(); const tempVec4 = vec4.create(); const tempMat4 = mat4.create(); -// Clamp range for the depth-based picking-indicator scale (relative to the base -// diameter at the focal plane). Keeps the ring from becoming extreme. -const PICKING_INDICATOR_MIN_DEPTH_SCALE = 0.6; -const PICKING_INDICATOR_MAX_DEPTH_SCALE = 1.7; - // Copy the OIT values to the main color buffer function defineTransparencyCopyShader(builder: ShaderBuilder) { builder.addOutputBuffer("vec4", "v4f_fragColor", null); @@ -1528,63 +1524,8 @@ export class PerspectivePanel extends RenderedDataPanel { ); } - readonly overlayPanelTypes = ["perspective"]; - - protected projectGlobalPosition(position: Float32Array) { - const { - viewProjectionMat, - logicalWidth, - logicalHeight, - displayDimensionRenderInfo: { displayDimensionIndices }, - } = this.projectionParameters.value; - // `position` is in global voxel space; extract display-space components. - const px = - displayDimensionIndices[0] >= 0 - ? position[displayDimensionIndices[0]] - : 0; - const py = - displayDimensionIndices[1] >= 0 - ? position[displayDimensionIndices[1]] - : 0; - const pz = - displayDimensionIndices[2] >= 0 - ? position[displayDimensionIndices[2]] - : 0; - const displayPos = tempVec3; - displayPos[0] = px; - displayPos[1] = py; - displayPos[2] = pz; - vec3.transformMat4(displayPos, displayPos, viewProjectionMat); - if (displayPos[2] < -1 || displayPos[2] > 1) return undefined; - - // Scale the indicator with depth to convey 3D position: the clip-space w is - // proportional to view-space depth for a perspective projection, so the - // ratio of the navigation center's w to the picked point's w is 1 at the - // focal plane, >1 nearer (larger ring), <1 farther (smaller ring). In - // orthographic mode m[3]=m[7]=m[11]=0, so both w values equal m[15] and the - // scale is 1 (constant size), needing no special-casing. - const m = viewProjectionMat; - const clipW = (x: number, y: number, z: number) => - m[3] * x + m[7] * y + m[11] * z + m[15]; - const pickedW = clipW(px, py, pz); - const center = this.navigationState.position.value; - const centerW = clipW( - displayDimensionIndices[0] >= 0 ? center[displayDimensionIndices[0]] : 0, - displayDimensionIndices[1] >= 0 ? center[displayDimensionIndices[1]] : 0, - displayDimensionIndices[2] >= 0 ? center[displayDimensionIndices[2]] : 0, - ); - let scale = 1; - if (pickedW > 1e-6 && centerW > 1e-6) { - scale = Math.min( - PICKING_INDICATOR_MAX_DEPTH_SCALE, - Math.max(PICKING_INDICATOR_MIN_DEPTH_SCALE, centerW / pickedW), - ); - } - return { - x: (displayPos[0] * 0.5 + 0.5) * logicalWidth, - y: (1 - (displayPos[1] * 0.5 + 0.5)) * logicalHeight, - scale, - }; + protected projectPosition(position: Float32Array) { + return projectToViewport(this.projectionParameters.value, position); } zoomByMouse(factor: number) { diff --git a/src/picking_indicator_overlay.css b/src/picking_indicator_overlay.css deleted file mode 100644 index 979f0a20f5..0000000000 --- a/src/picking_indicator_overlay.css +++ /dev/null @@ -1,32 +0,0 @@ -/** - * @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. - */ - -/* Ring drawn at the cursor's picked position. White, bordered by black on both - sides for contrast on any background. Size, opacity and position are set per - frame. */ -.neuroglancer-picking-indicator { - position: absolute; - left: 0; - top: 0; - box-sizing: border-box; - border-radius: 50%; - border: 2px solid rgba(255, 255, 255, 0.92); - box-shadow: - 0 0 0 1px rgba(0, 0, 0, 0.92), - inset 0 0 0 1px rgba(0, 0, 0, 0.92); - pointer-events: none; - will-change: transform; -} diff --git a/src/picking_indicator_overlay.ts b/src/picking_indicator_overlay.ts deleted file mode 100644 index 0ad070142a..0000000000 --- a/src/picking_indicator_overlay.ts +++ /dev/null @@ -1,69 +0,0 @@ -/** - * @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 "#src/picking_indicator_overlay.css"; - -import type { MouseSelectionState } from "#src/layer/index.js"; -import type { - PanelOverlayContext, - PanelOverlaySource, -} from "#src/panel_overlay.js"; -import type { NullarySignal } from "#src/util/signal.js"; - -// Base ring diameter in CSS pixels; scaled by the projection's depth `scale`. -const PICKING_INDICATOR_DIAMETER = 14; - -function createRing(): HTMLElement { - const element = document.createElement("div"); - element.className = "neuroglancer-picking-indicator"; - return element; -} - -/** A ring drawn at the cursor's picked position, driven by the mouse state. */ -export class PickingIndicatorOverlay implements PanelOverlaySource { - readonly overlayPriority = 100; - - constructor(private readonly mouseState: MouseSelectionState) {} - - get overlayUpdateNeeded(): NullarySignal { - return this.mouseState.changed; - } - - updatePanelOverlays(ctx: PanelOverlayContext): void { - const { container } = ctx; - const { mouseState } = this; - 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"; - return; - } - if (element === null) { - element = createRing(); - container.appendChild(element); - } - const size = PICKING_INDICATOR_DIAMETER * (pos.scale ?? 1); - const { style } = element; - style.display = ""; - style.width = `${size}px`; - style.height = `${size}px`; - style.opacity = `${pos.opacity ?? 1}`; - style.transform = `translate(${pos.x - size / 2}px, ${pos.y - size / 2}px)`; - } -} diff --git a/src/rendered_data_panel.ts b/src/rendered_data_panel.ts index e531acb309..78812b631d 100644 --- a/src/rendered_data_panel.ts +++ b/src/rendered_data_panel.ts @@ -19,12 +19,17 @@ import "#src/noselect.css"; import type { Annotation } from "#src/annotation/index.js"; import { getAnnotationTypeRenderHandler } from "#src/annotation/type_handler.js"; +import { coordinateSpacesEqual } from "#src/coordinate_transform.js"; import type { DisplayContext } from "#src/display_context.js"; import { RenderedPanel } from "#src/display_context.js"; import { hasSpatialSkeletonNodeSelection } from "#src/layer/segmentation/selection.js"; import type { NavigationState } from "#src/navigation_state.js"; import { PickIDManager } from "#src/object_picking.js"; -import type { PanelOverlaySource } from "#src/panel_overlay.js"; +import type { + PanelOverlay, + PanelOverlayHost, + ViewportPoint, +} from "#src/panel_overlay.js"; import { PanelOverlayManager } from "#src/panel_overlay.js"; import { displayToLayerCoordinates, @@ -38,7 +43,7 @@ import type { SpatialSkeletonSourceState } from "#src/skeleton/api.js"; import { StatusMessage } from "#src/status.js"; import type { TrackableValue } from "#src/trackable_value.js"; import { AutomaticallyFocusedElement } from "#src/util/automatic_focus.js"; -import type { Borrowed, RefCounted } from "#src/util/disposable.js"; +import type { Borrowed } from "#src/util/disposable.js"; import type { ActionEvent, EventActionMap, @@ -351,6 +356,33 @@ export abstract class RenderedDataPanel extends RenderedPanel { ); } + protected abstract projectPosition( + position: Float32Array, + ): ViewportPoint | undefined; + + private readonly overlays = this.registerDisposer( + new PanelOverlayManager( + this.element, + (position, coordinateSpace) => + coordinateSpacesEqual( + coordinateSpace, + this.navigationState.coordinateSpace.value, + ) + ? this.projectPosition(position) + : undefined, + () => { + this.ensureBoundsUpdated(); + const { width, height } = this.renderViewport; + return this.shouldDraw && width !== 0 && height !== 0; + }, + ), + ); + + /** Returns a function that removes the overlay. */ + addOverlay(createOverlay: (host: PanelOverlayHost) => PanelOverlay) { + return this.overlays.add(createOverlay); + } + draw() { const { width, height } = this.renderViewport; this.checkForPickRequestCompletion(true); @@ -364,13 +396,10 @@ export abstract class RenderedDataPanel extends RenderedPanel { newPickingData.pickIDs.clear(); if (!this.drawWithPicking(newPickingData)) { newPickingData.frameNumber = -1; - // The panel rendered nothing this frame; drop its overlays so stale - // markers don't linger over the cleared canvas region. - this.clearOverlays(); + this.overlays.hide(); return; } - // Reposition overlays for the new view. - this.updateOverlays(); + this.overlays.update(); // For the new frame, allow new pick requests regardless of interval since last request. this.nextPickRequestTime = 0; if (this.mouseX >= 0) { @@ -380,39 +409,6 @@ export abstract class RenderedDataPanel extends RenderedPanel { abstract drawWithPicking(pickingData: FramePickingData): boolean; - /** - * Projects a global-coordinate position to this panel's logical CSS pixels, or - * returns `undefined` if it is off-screen / behind the camera / culled by the - * cross-section slab. `scale` (default 1) conveys depth (perspective); - * `opacity` (default 1) is the cross-section fade in slice views. Implemented - * per panel using its own projection. - */ - protected abstract projectGlobalPosition( - position: Float32Array, - ): { x: number; y: number; scale?: number; opacity?: number } | undefined; - - /** - * Type tags used to target overlays (see {@link PanelOverlayTarget}), e.g. - * `["perspective"]` or `["cross-section"]`. - */ - abstract readonly overlayPanelTypes: readonly string[]; - - private overlays: PanelOverlayManager; - - // Called by the visible-layer tracker to bind a layer's overlay source; `owner` - // is the per-(layer,panel) attachment. - bindOverlaySource(source: PanelOverlaySource, owner: RefCounted) { - this.overlays.bindSource(source, owner); - } - - override updateOverlays() { - this.overlays.update(); - } - - clearOverlays() { - this.overlays.clear(); - } - private nextPickRequestTime = 0; private pendingPickRequestTimerId = -1; @@ -500,29 +496,6 @@ export abstract class RenderedDataPanel extends RenderedPanel { super(context, element, viewer.visibility); this.inputEventMap = viewer.inputEventMap; - const self = this; - this.overlays = this.registerDisposer( - new PanelOverlayManager( - { - element, - get visible() { - return self.visible; - }, - get cssPerDevicePixel() { - const { width, logicalWidth } = self.renderViewport; - return width > 0 ? logicalWidth / width : 1; - }, - get panelTypes() { - return self.overlayPanelTypes; - }, - project: (p) => self.projectGlobalPosition(p), - }, - context.panelOverlays, - context.panelOverlaysChanged, - () => this.scheduleOverlayUpdate(), - ), - ); - element.classList.add("neuroglancer-rendered-data-panel"); element.classList.add("neuroglancer-panel"); element.classList.add("neuroglancer-noselect"); diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index 7f90e72e2d..606822fae4 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -32,10 +32,7 @@ import type { PickState, VisibleLayerInfo, } from "#src/layer/index.js"; -import type { - PanelOverlayContext, - PanelOverlaySource, -} from "#src/panel_overlay.js"; +import type { PanelOverlay, PanelOverlayHost } from "#src/panel_overlay.js"; import type { PerspectivePanel } from "#src/perspective_view/panel.js"; import type { PerspectiveViewReadyRenderContext, @@ -49,6 +46,7 @@ import type { } from "#src/render_coordinate_transform.js"; import { getChunkTransformParameters } from "#src/render_coordinate_transform.js"; import type { RenderScaleHistogram } from "#src/render_scale_statistics.js"; +import type { RenderedDataPanel } from "#src/rendered_data_panel.js"; import type { RenderLayer, ThreeDimensionalRenderLayerAttachmentState, @@ -1283,7 +1281,7 @@ function getSkeletonNodeDiameter( // A selected/hovered node highlight to draw as a DOM ring overlay. `diameter` // and `borderWidth` are in render-viewport device px (matching the node's -// on-screen size); the panel converts them to CSS px via `cssPerDevicePixel`. +// on-screen size); the overlay converts them to CSS px. interface HighlightMarker { position: Float32Array; // global coordinate space kind: "selected" | "hovered"; @@ -1293,38 +1291,67 @@ interface HighlightMarker { borderWidth: number; } -// Reconciles the ring child elements of an overlay source's per-panel container -// to `markers`, projecting each via the panel context. Reuses/pools children. -function updateSkeletonHighlightOverlay( - markers: HighlightMarker[], - ctx: PanelOverlayContext, -) { - const { container, cssPerDevicePixel } = ctx; - let count = 0; - for (const marker of markers) { - const pos = ctx.project(marker.position); - if (pos === undefined) continue; - let element = container.children[count] as HTMLElement | undefined; - if (element === undefined) { - element = document.createElement("div"); - element.className = "neuroglancer-skeleton-node-highlight"; - container.appendChild(element); +class SkeletonNodeHighlightOverlay extends RefCounted implements PanelOverlay { + private readonly rings: HTMLElement[] = []; + + constructor( + private readonly host: PanelOverlayHost, + private readonly panel: RenderedDataPanel, + private readonly layer: SpatiallyIndexedSkeletonLayer, + private readonly renderOptions: ViewSpecificSkeletonRenderingOptions, + private readonly view: "2d" | "3d", + ) { + super(); + this.registerDisposer( + layer.highlightMarkersChanged.add(host.scheduleUpdate), + ); + this.registerDisposer(() => { + for (const ring of this.rings) ring.remove(); + }); + } + + update() { + const { host, panel, renderOptions, rings } = this; + const targetIsSliceView = this.view === "2d"; + const { diameter, borderWidth } = getSkeletonNodeHighlightRing( + renderOptions.mode.value, + renderOptions.lineWidth.value, + targetIsSliceView, + ); + const { width, logicalWidth } = panel.renderViewport; + const cssPerDevicePixel = width > 0 ? logicalWidth / width : 1; + // Marker positions are in the global coordinate space. + const coordinateSpace = panel.navigationState.coordinateSpace.value; + let count = 0; + for (const marker of this.layer.computeHighlightMarkers( + diameter, + borderWidth, + )) { + const point = host.project(marker.position, coordinateSpace); + if (point === undefined) continue; + let ring = rings[count]; + if (ring === undefined) { + ring = document.createElement("div"); + ring.className = "neuroglancer-skeleton-node-highlight"; + host.container.appendChild(ring); + rings.push(ring); + } + ++count; + const size = marker.diameter * cssPerDevicePixel; + ring.hidden = false; + const { style } = ring; + style.width = `${size}px`; + style.height = `${size}px`; + style.borderWidth = `${Math.max(1, marker.borderWidth * cssPerDevicePixel)}px`; + style.borderColor = marker.color; + style.setProperty("--ng-node-highlight-outline", marker.outlineColor); + // Matches the cross-section fade of the node itself. + style.opacity = targetIsSliceView + ? `${1 - Math.abs(point.focalPlaneDepthFraction)}` + : "1"; + style.transform = `translate(${point.viewportLeft - size / 2}px, ${point.viewportTop - size / 2}px)`; } - ++count; - const size = marker.diameter * cssPerDevicePixel; - const { style } = element; - style.display = ""; - style.width = `${size}px`; - style.height = `${size}px`; - style.borderWidth = `${Math.max(1, marker.borderWidth * cssPerDevicePixel)}px`; - style.borderColor = marker.color; - style.setProperty("--ng-node-highlight-outline", marker.outlineColor); - style.opacity = `${pos.opacity ?? 1}`; - style.transform = `translate(${pos.x - size / 2}px, ${pos.y - size / 2}px)`; - } - const { children } = container; - for (let i = count; i < children.length; ++i) { - (children[i] as HTMLElement).style.display = "none"; + for (let i = count; i < rings.length; ++i) rings[i].hidden = true; } } @@ -3942,10 +3969,7 @@ function attachSpatiallyIndexedSkeletonLayer( ); } -export class PerspectiveViewSpatiallyIndexedSkeletonLayer - extends PerspectiveViewRenderLayer - implements PanelOverlaySource -{ +export class PerspectiveViewSpatiallyIndexedSkeletonLayer extends PerspectiveViewRenderLayer { private renderHelper: RenderHelper; private browseRenderHelper: RenderHelper; private renderOptions: ViewSpecificSkeletonRenderingOptions; @@ -3982,23 +4006,6 @@ export class PerspectiveViewSpatiallyIndexedSkeletonLayer this.registerDisposer(histogram3d.visibility.add(this.visibility)); } - readonly overlayPriority = 0; - get overlayUpdateNeeded() { - return this.base.highlightMarkersChanged; - } - updatePanelOverlays(ctx: PanelOverlayContext) { - const { renderOptions } = this; - const ring = getSkeletonNodeHighlightRing( - renderOptions.mode.value, - renderOptions.lineWidth.value, - /*targetIsSliceView=*/ false, - ); - updateSkeletonHighlightOverlay( - this.base.computeHighlightMarkers(ring.diameter, ring.borderWidth), - ctx, - ); - } - attach( attachment: VisibleLayerInfo< PerspectivePanel, @@ -4007,6 +4014,18 @@ export class PerspectiveViewSpatiallyIndexedSkeletonLayer ) { super.attach(attachment); attachSpatiallyIndexedSkeletonLayer(this.base, this, attachment, "3d"); + attachment.registerDisposer( + attachment.view.addOverlay( + (host) => + new SkeletonNodeHighlightOverlay( + host, + attachment.view, + this.base, + this.renderOptions, + "3d", + ), + ), + ); } get gl() { @@ -4135,10 +4154,7 @@ export class PerspectiveViewSpatiallyIndexedSkeletonLayer } } -export class SliceViewPanelSpatiallyIndexedSkeletonLayer - extends SliceViewPanelRenderLayer - implements PanelOverlaySource -{ +export class SliceViewPanelSpatiallyIndexedSkeletonLayer extends SliceViewPanelRenderLayer { private renderHelper: RenderHelper; private browseRenderHelper: RenderHelper; private renderOptions: ViewSpecificSkeletonRenderingOptions; @@ -4178,23 +4194,6 @@ export class SliceViewPanelSpatiallyIndexedSkeletonLayer return this.base.gl; } - readonly overlayPriority = 0; - get overlayUpdateNeeded() { - return this.base.highlightMarkersChanged; - } - updatePanelOverlays(ctx: PanelOverlayContext) { - const { renderOptions } = this; - const ring = getSkeletonNodeHighlightRing( - renderOptions.mode.value, - renderOptions.lineWidth.value, - /*targetIsSliceView=*/ true, - ); - updateSkeletonHighlightOverlay( - this.base.computeHighlightMarkers(ring.diameter, ring.borderWidth), - ctx, - ); - } - getValueAt(_position: Float32Array) { return undefined; } @@ -4225,6 +4224,18 @@ export class SliceViewPanelSpatiallyIndexedSkeletonLayer ) { super.attach(attachment); attachSpatiallyIndexedSkeletonLayer(this.base, this, attachment, "2d"); + attachment.registerDisposer( + attachment.view.addOverlay( + (host) => + new SkeletonNodeHighlightOverlay( + host, + attachment.view, + this.base, + this.renderOptions, + "2d", + ), + ), + ); } draw( diff --git a/src/sliceview/panel.ts b/src/sliceview/panel.ts index 7782a73b75..334133edab 100644 --- a/src/sliceview/panel.ts +++ b/src/sliceview/panel.ts @@ -19,6 +19,7 @@ import type { DisplayContext } from "#src/display_context.js"; import type { VisibleRenderLayerTracker } from "#src/layer/index.js"; import { makeRenderedPanelVisibleLayerTracker } from "#src/layer/index.js"; import { PickIDManager } from "#src/object_picking.js"; +import { projectToViewport } from "#src/panel_overlay.js"; import type { FramePickingData, RenderedDataViewerState, @@ -532,38 +533,11 @@ export class SliceViewPanel extends RenderedDataPanel { setStateFromRelative(pickRadius, pickRadius, 0); } - readonly overlayPanelTypes = ["cross-section"]; - - protected projectGlobalPosition(position: Float32Array) { - const { - viewProjectionMat, - logicalWidth, - logicalHeight, - displayDimensionRenderInfo: { displayDimensionIndices }, - } = this.sliceView.projectionParameters.value; - const displayPos = tempVec3; - displayPos[0] = - displayDimensionIndices[0] >= 0 - ? position[displayDimensionIndices[0]] - : 0; - displayPos[1] = - displayDimensionIndices[1] >= 0 - ? position[displayDimensionIndices[1]] - : 0; - displayPos[2] = - displayDimensionIndices[2] >= 0 - ? position[displayDimensionIndices[2]] - : 0; - vec3.transformMat4(displayPos, displayPos, viewProjectionMat); - const ndcZ = displayPos[2]; - if (ndcZ < -1 || ndcZ > 1) return undefined; - return { - x: (displayPos[0] * 0.5 + 0.5) * logicalWidth, - y: (1 - (displayPos[1] * 0.5 + 0.5)) * logicalHeight, - // Cross-section fade: match the node's on-screen alpha (1 on the slice - // plane, → 0 at the slab edge). See getCircleAlphaMultiplier in circles.ts. - opacity: 1 - Math.abs(ndcZ), - }; + protected projectPosition(position: Float32Array) { + return projectToViewport( + this.sliceView.projectionParameters.value, + position, + ); } /** diff --git a/src/ui/skeleton_edit_tools.spec.ts b/src/ui/skeleton_edit_tools.spec.ts index ba1d48e355..a064198e4c 100644 --- a/src/ui/skeleton_edit_tools.spec.ts +++ b/src/ui/skeleton_edit_tools.spec.ts @@ -900,10 +900,6 @@ 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: { @@ -995,10 +991,6 @@ 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(); @@ -1493,8 +1485,6 @@ describe("spatial_skeleton_edit_tool", () => { updateUnconditionally: vi.fn(() => true), active: true, unsnappedPosition: new Float32Array([1, 2, 3]), - pickingIndicatorSuppressed: false, - changed: makeChangedSignal(), }; const layer = { displayState: { diff --git a/src/ui/skeleton_edit_tools.ts b/src/ui/skeleton_edit_tools.ts index c8b13459c6..c2775e2b32 100644 --- a/src/ui/skeleton_edit_tools.ts +++ b/src/ui/skeleton_edit_tools.ts @@ -952,7 +952,7 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { return; } dragStarted = true; - this.setNodeMoveActive(true); + this.dragInProgress = true; skeletonLayer!.markSegmentEdited(nodeInfo!.segmentId); panel.element.dataset.skeletonPressMode = "move"; this.setStatus(getSpatialSkeletonMovingStatusText()); @@ -988,7 +988,7 @@ export class SpatialSkeletonEditTool extends SpatialSkeletonToolBase { if (finished) return; finished = true; if (this.dragInProgress) { - this.setNodeMoveActive(false); + this.dragInProgress = false; delete panel.element.dataset.skeletonPressMode; this.clearStatus(); } @@ -1494,26 +1494,13 @@ 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.setNodeMoveActive(false); + this.dragInProgress = false; this.pending = false; this.createPlacedThisHold = false; this.mergeKeyHeld = false; diff --git a/src/viewer.ts b/src/viewer.ts index ff732dd212..40ade2339b 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -71,7 +71,6 @@ import { WatchableDisplayDimensionRenderInfo, } from "#src/navigation_state.js"; import { overlaysOpen } from "#src/overlay.js"; -import { PickingIndicatorOverlay } from "#src/picking_indicator_overlay.js"; import { ScreenshotHandler } from "#src/python_integration/screenshots.js"; import { allRenderLayerRoles, RenderLayerRole } from "#src/renderlayer.js"; import { @@ -602,12 +601,6 @@ export class Viewer extends RefCounted implements ViewerState { options: Partial = {}, ) { super(); - // Show the picking indicator on every data panel. - this.registerDisposer( - display.registerPanelOverlay( - new PickingIndicatorOverlay(this.mouseState), - ), - ); this.screenshotHandler = this.registerDisposer(new ScreenshotHandler(this)); this.screenshotManager = this.registerDisposer(new ScreenshotManager(this)); const { From d060e8da4ba4499bcc307692698022bfe9ed416c Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Thu, 24 Sep 2026 21:09:48 +0200 Subject: [PATCH 2/2] refactor: reduce comment bloat --- src/skeleton/frontend.css | 6 +--- src/skeleton/frontend.ts | 70 +++++++-------------------------------- 2 files changed, 13 insertions(+), 63 deletions(-) diff --git a/src/skeleton/frontend.css b/src/skeleton/frontend.css index 03b57b14d6..228f4d605d 100644 --- a/src/skeleton/frontend.css +++ b/src/skeleton/frontend.css @@ -14,11 +14,7 @@ * limitations under the License. */ -/* Ring outlining a selected/hovered skeleton node. A pure border around the - node with a single 1px contrast halo on the outside, so it never covers the - node point. Size, border width/color and the halo color are set per marker; - the halo color adapts to the ring color's luminance (white for dark rings, - black for light ones). */ +/* Size, border width and colors are set inline per node. */ .neuroglancer-skeleton-node-highlight { position: absolute; left: 0; diff --git a/src/skeleton/frontend.ts b/src/skeleton/frontend.ts index 606822fae4..924d694bc7 100644 --- a/src/skeleton/frontend.ts +++ b/src/skeleton/frontend.ts @@ -216,7 +216,6 @@ const DEFAULT_FRAGMENT_MAIN = `void main() { `; 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 { return `rgb(${Math.round(color[0] * 255)}, ${Math.round( color[1] * 255, @@ -226,9 +225,7 @@ const SELECTED_NODE_OUTLINE_MIN_WIDTH_2D = "3.5"; const SELECTED_NODE_OUTLINE_MAX_WIDTH_2D = "8.0"; const SELECTED_NODE_OUTLINE_MIN_WIDTH_3D = "3.0"; const SELECTED_NODE_OUTLINE_MAX_WIDTH_3D = "7.0"; -// Fraction of the node diameter used as the highlight outline width before -// clamping to the min/max above. Nodes are small (~5-6px), so this mostly hits -// the min for typical nodes and scales up the ring for larger nodes. +// Outline width as a fraction of node diameter, before the min/max clamp. const SELECTED_NODE_OUTLINE_DIAMETER_FRACTION = "0.5"; // Saturation adjustment factor and threshold for the highlighted (hovered) node border: each @@ -1279,13 +1276,11 @@ function getSkeletonNodeDiameter( return lineWidth; } -// A selected/hovered node highlight to draw as a DOM ring overlay. `diameter` -// and `borderWidth` are in render-viewport device px (matching the node's -// on-screen size); the overlay converts them to CSS px. +// `diameter` and `borderWidth` are in render-viewport device pixels. interface HighlightMarker { position: Float32Array; // global coordinate space kind: "selected" | "hovered"; - color: string; // CSS ring color, derived from the node's segment color + color: string; // CSS ring color outlineColor: string; // CSS halo color, contrasting with `color` diameter: number; borderWidth: number; @@ -1320,7 +1315,6 @@ class SkeletonNodeHighlightOverlay extends RefCounted implements PanelOverlay { ); const { width, logicalWidth } = panel.renderViewport; const cssPerDevicePixel = width > 0 ? logicalWidth / width : 1; - // Marker positions are in the global coordinate space. const coordinateSpace = panel.navigationState.coordinateSpace.value; let count = 0; for (const marker of this.layer.computeHighlightMarkers( @@ -1355,9 +1349,7 @@ class SkeletonNodeHighlightOverlay extends RefCounted implements PanelOverlay { } } -// On-screen size (render-viewport device px) of a node's selection ring, -// matching the old in-shader outline: a band of `borderWidth` sitting just -// outside the node, so the outer `diameter` = nodeDiameter + 2 * outline. +// The ring sits just outside the node, so it never covers the node point. function getSkeletonNodeHighlightRing( renderMode: SkeletonRenderMode, lineWidth: number, @@ -2289,8 +2281,6 @@ export class SpatiallyIndexedSkeletonLayer private resolveGlobalPosition: | ((modelPosition: ArrayLike) => Float32Array | undefined) | undefined; - // Fires when the set of highlighted nodes (selected/hovered) changes, so panels - // can reposition their DOM node-highlight markers without a full canvas redraw. readonly highlightMarkersChanged = new NullarySignal(); private inspectionState: SpatiallyIndexedSkeletonInspectionState | undefined; private overlayChunk: SkeletonOverlayChunk | undefined; @@ -2329,8 +2319,6 @@ export class SpatiallyIndexedSkeletonLayer private readonly highlightedNodeOutlineColor = vec3.clone( 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. private nodeOutlineColorGeneration = 0; private cachedNodeOutlineColorGeneration = -1; @@ -2438,9 +2426,7 @@ export class SpatiallyIndexedSkeletonLayer return segmentIds; } - // Segment fill color a node's outline should contrast against, or undefined - // when no segment can be resolved. Falls back to the currently selected - // segment when the node carries no segment id. + // Falls back to the selected segment's color when the node has no segment id. private getNodeSegmentColor( nodeInfo: SelectedSkeletonNodeInfo, ): Float32Array | undefined { @@ -2454,13 +2440,7 @@ export class SpatiallyIndexedSkeletonLayer return getBaseObjectColor(this.displayState, segmentId); } - // Updates `selectedNodeOutlineColor` and `highlightedNodeOutlineColor` in - // place. Each outline is chosen, independently of the other, for high contrast - // against its own node's segment color: the selected node uses the muted - // palette, and the hovered node uses its own segment color pushed away from - // (or, if already very saturated, towards) grey. Because the two are computed - // independently, a given segment color always yields the same selected color - // and the same hovered color. + // Each outline contrasts with its own node's segment color. private updateNodeOutlineColorPair() { const currentGeneration = this.nodeOutlineColorGeneration; if (this.cachedNodeOutlineColorGeneration === currentGeneration) { @@ -2869,12 +2849,10 @@ export class SpatiallyIndexedSkeletonLayer skeletonShaderParameters: this.browsePassSkeletonShaderParameters, }; const requestRedraw = () => this.redrawNeeded.dispatch(); - // Node highlights are DOM overlays, so a selected/hovered change repositions - // the markers without a canvas redraw. + // Node highlights are DOM overlays, so they update without a redraw. if (this.selectedNodeInfo?.changed) { this.registerDisposer( this.selectedNodeInfo.changed.add(() => { - // Recompute the marker's contrast color for the new node. invalidateNodeOutlineColors(); this.highlightMarkersChanged.dispatch(); }), @@ -2882,8 +2860,6 @@ export class SpatiallyIndexedSkeletonLayer } if (this.suppressSelectedNodeHighlight?.changed) { this.registerDisposer( - // The selected-node ring is a DOM overlay, so toggling suppression must - // refresh the markers (not just request a canvas redraw). this.suppressSelectedNodeHighlight.changed.add(() => { this.highlightMarkersChanged.dispatch(); }), @@ -2901,7 +2877,6 @@ export class SpatiallyIndexedSkeletonLayer if (pendingNodePositionVersion?.changed) { this.registerDisposer( pendingNodePositionVersion.changed.add(() => { - // A node's position moved: redraw geometry and reposition markers. requestRedraw(); this.highlightMarkersChanged.dispatch(); }), @@ -2918,9 +2893,6 @@ export class SpatiallyIndexedSkeletonLayer }), ); } - // A marker is emitted only when its node's skeleton would be drawn (see - // computeHighlightMarkers), so its visibility depends on the object alphas - // and the visible-segment set. Refresh the overlay when any of those change. const refreshHighlightVisibility = () => { this.highlightMarkersChanged.dispatch(); }; @@ -3019,27 +2991,15 @@ export class SpatiallyIndexedSkeletonLayer }; } - /** - * Builds highlight markers for the selected/hovered nodes. `diameter` and - * `borderWidth` are the node's on-screen ring size (device px) for the calling - * view, so the marker matches the node's size — the old in-shader outline sat - * just outside the node with the same thickness. Positions are resolved from - * the stored info (model space) or the node cache, then transformed to global - * space; entries whose position is unavailable are omitted. - */ + /** Omits a node whose position is unavailable. */ computeHighlightMarkers( diameter: number, borderWidth: number, ): HighlightMarker[] { const { resolveGlobalPosition } = this; if (resolveGlobalPosition === undefined) return []; - // Refresh the per-node contrast colors (selected uses the muted palette, - // hovered uses its saturated segment color) so markers match the previous - // in-shader outline colors. this.updateNodeOutlineColorPair(); - // Mirror the shader's per-segment visibility so a ring is never drawn over a - // skeleton that isn't rendered: a segment draws at `objectAlpha` when it is - // visible/selected and at `hiddenObjectAlpha` otherwise. + // Mirrors the shader, so a ring never marks a skeleton that is not drawn. const visibleSegments = getVisibleSegments( this.displayState.segmentationGroupState.value, ); @@ -3060,19 +3020,15 @@ export class SpatiallyIndexedSkeletonLayer : hiddenObjectAlpha; if (effectiveAlpha <= 0) return; } else if (objectAlpha <= 0 && hiddenObjectAlpha <= 0) { - // Unknown segment: fall back to the whole-layer invisibility test. return; } - // Prefer the live cached position (which applies any pending move) so the - // marker stays in sync when the node moves; fall back to the position - // captured at selection time if the node isn't currently cached. + // The cached position includes any pending move. const modelPosition = this.getCachedNodeSnapshot(nodeId)?.position ?? info?.position; if (modelPosition === undefined) return; const global = resolveGlobalPosition(modelPosition); if (global === undefined) return; - // Halo contrasts with the ring color: white around a dark ring, black - // around a light one (WCAG black/white crossover luminance ~0.179). + // Relative luminance at which black and white contrast equally. const outlineColor = getRelativeLuminance(color) < 0.179 ? "rgba(255, 255, 255, 0.85)" @@ -3090,9 +3046,7 @@ export class SpatiallyIndexedSkeletonLayer ? undefined : this.selectedNodeInfo?.value?.nodeId; const hoveredNodeId = this.hoveredNodeInfo?.value?.nodeId; - // When the same node is both selected and hovered, show only the hovered - // marker (as the old shader did — hovered won over selected), avoiding an - // overlapping ring. + // Hovered wins over selected, so one node never shows two rings. if (selectedNodeId !== undefined && selectedNodeId !== hoveredNodeId) { add( this.selectedNodeInfo?.value,