From 9876b0256cd69b873329e23a5afe8461fe6bd2c1 Mon Sep 17 00:00:00 2001 From: afonso pinto Date: Thu, 19 Mar 2026 14:04:31 +0000 Subject: [PATCH] fix: Correct picking formulas --- src/perspective_view/panel.ts | 107 ++++++++-------- src/rendered_data_panel.ts | 108 +++-------------- src/rendered_data_panel_picking.spec.ts | 93 ++++++++++++++ src/rendered_data_panel_picking.ts | 154 ++++++++++++++++++++++++ src/sliceview/panel.ts | 46 ++++--- 5 files changed, 354 insertions(+), 154 deletions(-) create mode 100644 src/rendered_data_panel_picking.spec.ts create mode 100644 src/rendered_data_panel_picking.ts diff --git a/src/perspective_view/panel.ts b/src/perspective_view/panel.ts index 7b062b436..2dc6fab09 100644 --- a/src/perspective_view/panel.ts +++ b/src/perspective_view/panel.ts @@ -36,9 +36,11 @@ import type { RenderedDataViewerState, } from "#src/rendered_data_panel.js"; import { + getCenteredPickWindowCoordinate, getPickDiameter, getPickOffsetSequence, RenderedDataPanel, + resolveNearestPanelPickSample, } from "#src/rendered_data_panel.js"; import { DerivedProjectionParameters, @@ -717,55 +719,66 @@ export class PerspectivePanel extends RenderedDataPanel { mouseState.pickedRenderLayer = null; const pickDiameter = getPickDiameter(pickRadius); const pickOffsetSequence = getPickOffsetSequence(pickRadius); - const numOffsets = pickOffsetSequence.length; - for (let i = 0; i < numOffsets; ++i) { - const offset = pickOffsetSequence[i]; - const zValue = data[4 * offset]; - if (zValue === 0) continue; - const relativeX = offset % pickDiameter; - const relativeY = (offset - relativeX) / pickDiameter; - const glWindowZ = 1.0 - zValue; - tempVec3[0] = - (2.0 * (glWindowX + relativeX - pickRadius)) / - pickingData.viewportWidth - - 1.0; - tempVec3[1] = - (2.0 * (glWindowY + relativeY - pickRadius)) / - pickingData.viewportHeight - - 1.0; - tempVec3[2] = 2.0 * glWindowZ - 1.0; - vec3.transformMat4(tempVec3, tempVec3, pickingData.invTransform); - let { position: mousePosition, unsnappedPosition } = mouseState; - const { value: voxelCoordinates } = this.navigationState.position; - const rank = voxelCoordinates.length; - if (mousePosition.length !== rank) { - mousePosition = mouseState.position = new Float32Array(rank); - } - if (unsnappedPosition.length !== rank) { - unsnappedPosition = mouseState.unsnappedPosition = new Float32Array( - rank, - ); - } - mousePosition.set(voxelCoordinates); - mouseState.coordinateSpace = this.navigationState.coordinateSpace.value; - const displayDimensions = - this.navigationState.pose.displayDimensions.value; - const { displayDimensionIndices } = displayDimensions; - for ( - let i = 0, spatialRank = displayDimensionIndices.length; - i < spatialRank; - ++i - ) { - mousePosition[displayDimensionIndices[i]] = tempVec3[i]; - } - unsnappedPosition.set(mousePosition); - const pickValue = data[4 * pickDiameter * pickDiameter + 4 * offset]; - pickingData.pickIDs.setMouseState(mouseState, pickValue); - mouseState.displayDimensions = displayDimensions; - mouseState.setActive(true); + const resolvedPick = resolveNearestPanelPickSample( + data, + pickOffsetSequence, + pickRadius, + { + depthBaseOffset: 0, + pickBaseOffset: 4 * pickDiameter * pickDiameter, + }, + ); + if (resolvedPick === undefined || resolvedPick.depthValue === undefined) { + mouseState.setActive(false); return; } - mouseState.setActive(false); + const glWindowZ = 1.0 - resolvedPick.depthValue; + tempVec3[0] = + (2.0 * + getCenteredPickWindowCoordinate( + glWindowX, + resolvedPick.relativeX, + pickRadius, + )) / + pickingData.viewportWidth - + 1.0; + tempVec3[1] = + (2.0 * + getCenteredPickWindowCoordinate( + glWindowY, + resolvedPick.relativeY, + pickRadius, + )) / + pickingData.viewportHeight - + 1.0; + tempVec3[2] = 2.0 * glWindowZ - 1.0; + vec3.transformMat4(tempVec3, tempVec3, pickingData.invTransform); + let { position: mousePosition, unsnappedPosition } = mouseState; + const { value: voxelCoordinates } = this.navigationState.position; + const rank = voxelCoordinates.length; + if (mousePosition.length !== rank) { + mousePosition = mouseState.position = new Float32Array(rank); + } + if (unsnappedPosition.length !== rank) { + unsnappedPosition = mouseState.unsnappedPosition = new Float32Array( + rank, + ); + } + mousePosition.set(voxelCoordinates); + mouseState.coordinateSpace = this.navigationState.coordinateSpace.value; + const displayDimensions = this.navigationState.pose.displayDimensions.value; + const { displayDimensionIndices } = displayDimensions; + for ( + let i = 0, spatialRank = displayDimensionIndices.length; + i < spatialRank; + ++i + ) { + mousePosition[displayDimensionIndices[i]] = tempVec3[i]; + } + unsnappedPosition.set(mousePosition); + pickingData.pickIDs.setMouseState(mouseState, resolvedPick.pickValue); + mouseState.displayDimensions = displayDimensions; + mouseState.setActive(true); } translateDataPointByViewportPixels( diff --git a/src/rendered_data_panel.ts b/src/rendered_data_panel.ts index 6f39a9c0f..c7bc7b086 100644 --- a/src/rendered_data_panel.ts +++ b/src/rendered_data_panel.ts @@ -23,6 +23,10 @@ import type { DisplayContext } from "#src/display_context.js"; import { RenderedPanel } from "#src/display_context.js"; import type { NavigationState } from "#src/navigation_state.js"; import { PickIDManager } from "#src/object_picking.js"; +import { + clearOutOfBoundsPickData, + getPickDiameter, +} from "#src/rendered_data_panel_picking.js"; import { displayToLayerCoordinates, layerToDisplayCoordinates, @@ -77,88 +81,14 @@ export class PickRequest { const pickRequestInterval = 30; -export function getPickDiameter(pickRadius: number): number { - return 1 + pickRadius * 2; -} - -let _cachedPickRadius = -1; -let _cachedPickOffsetSequence: Uint32Array | undefined; - -/** - * Sequence of offsets into C order (pickDiamater, pickDiamater) array in order of increasing - * distance from center. - */ -export function getPickOffsetSequence(pickRadius: number) { - if (pickRadius === _cachedPickRadius) { - return _cachedPickOffsetSequence!; - } - _cachedPickRadius = pickRadius; - const pickDiameter = 1 + pickRadius * 2; - const maxDist2 = pickRadius ** 2; - const getDist2 = (x: number, y: number) => - (x - pickRadius) ** 2 + (y - pickRadius) ** 2; - - let offsets = new Uint32Array(pickDiameter * pickDiameter); - let count = 0; - for (let x = 0; x < pickDiameter; ++x) { - for (let y = 0; y < pickDiameter; ++y) { - if (getDist2(x, y) > maxDist2) continue; - offsets[count++] = y * pickDiameter + x; - } - } - offsets = offsets.subarray(0, count); - offsets.sort((a, b) => { - const x1 = a % pickDiameter; - const y1 = (a - x1) / pickDiameter; - const x2 = b % pickDiameter; - const y2 = (b - x2) / pickDiameter; - return getDist2(x1, y1) - getDist2(x2, y2); - }); - return (_cachedPickOffsetSequence = offsets); -} - -/** - * Sets array elements to 0 that would be outside the viewport. - * - * @param buffer Array view, which contains a C order (pickDiameter, pickDiameter) array. - * @param baseOffset Offset into `buffer` corresponding to (0, 0). - * @param stride Stride between consecutive elements of the array. - * @param glWindowX Center x position, must be integer. - * @param glWindowY Center y position, must be integer. - * @param viewportWidth Width of viewport in pixels. - * @param viewportHeight Width of viewport in pixels. - */ -export function clearOutOfBoundsPickData( - buffer: Float32Array, - baseOffset: number, - stride: number, - glWindowX: number, - glWindowY: number, - viewportWidth: number, - viewportHeight: number, - pickRadius: number, -) { - const pickDiameter = 1 + pickRadius * 2; - const startX = glWindowX - pickRadius; - const startY = glWindowY - pickRadius; - if ( - startX >= 0 && - startY >= 0 && - startX + pickDiameter <= viewportWidth && - startY + pickDiameter <= viewportHeight - ) { - return; - } - for (let relativeY = 0; relativeY < pickDiameter; ++relativeY) { - for (let relativeX = 0; relativeX < pickDiameter; ++relativeX) { - const x = startX + relativeX; - const y = startY + relativeY; - if (x < 0 || y < 0 || x >= viewportWidth || y >= viewportHeight) { - buffer[baseOffset + (y * pickDiameter + x) * stride] = 0; - } - } - } -} +export { + clearOutOfBoundsPickData, + getCenteredPickWindowCoordinate, + getPickDiameter, + getPickOffsetSequence, + resolveNearestPanelPickSample, + type ResolvedPanelPickSample, +} from "#src/rendered_data_panel_picking.js"; export abstract class RenderedDataPanel extends RenderedPanel { /** @@ -234,13 +164,15 @@ export abstract class RenderedDataPanel extends RenderedPanel { gl.bindBuffer(WebGL2RenderingContext.PIXEL_PACK_BUFFER, buffer); } const { renderViewport } = this; - const glWindowX = + const glWindowX = Math.floor( this.mouseX - - renderViewport.visibleLeftFraction * renderViewport.logicalWidth; - const glWindowY = + renderViewport.visibleLeftFraction * renderViewport.logicalWidth, + ); + const glWindowY = Math.floor( renderViewport.height - - (this.mouseY - - renderViewport.visibleTopFraction * renderViewport.logicalHeight); + (this.mouseY - + renderViewport.visibleTopFraction * renderViewport.logicalHeight), + ); this.issuePickRequest(glWindowX, glWindowY, pickRadius); pickRequest.sync = gl.fenceSync( WebGL2RenderingContext.SYNC_GPU_COMMANDS_COMPLETE, @@ -882,7 +814,7 @@ export abstract class RenderedDataPanel extends RenderedPanel { ((clientX - bounds.left) / bounds.width) * element.offsetWidth - element.clientLeft; const mouseY = - ((clientY - bounds.top) / bounds.height) * element.offsetHeight + + ((clientY - bounds.top) / bounds.height) * element.offsetHeight - element.clientTop; const { mouseState } = this.viewer; mouseState.pageX = clientX + window.scrollX; diff --git a/src/rendered_data_panel_picking.spec.ts b/src/rendered_data_panel_picking.spec.ts new file mode 100644 index 000000000..96f64a913 --- /dev/null +++ b/src/rendered_data_panel_picking.spec.ts @@ -0,0 +1,93 @@ +/** + * @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 { + clearOutOfBoundsPickData, + getPickDiameter, + getPickOffsetSequence, + resolveNearestPanelPickSample, +} from "#src/rendered_data_panel_picking.js"; + +describe("resolveNearestPanelPickSample", () => { + it("dereferences slice pick data using the sampled offset", () => { + const pickRadius = 5; + const pickOffsetSequence = getPickOffsetSequence(pickRadius); + const targetOffset = pickOffsetSequence[1]; + const data = new Float32Array(4 * getPickDiameter(pickRadius) ** 2); + data[4 * targetOffset] = 17; + + expect( + resolveNearestPanelPickSample(data, pickOffsetSequence, pickRadius), + ).toEqual({ + offset: targetOffset, + relativeX: targetOffset % getPickDiameter(pickRadius), + relativeY: + (targetOffset - (targetOffset % getPickDiameter(pickRadius))) / + getPickDiameter(pickRadius), + pickValue: 17, + depthValue: undefined, + }); + }); + + it("returns depth and pick payload from the same sampled pixel", () => { + const pickRadius = 2; + const pickDiameter = getPickDiameter(pickRadius); + const pickOffsetSequence = getPickOffsetSequence(pickRadius); + const targetOffset = pickOffsetSequence[3]; + const data = new Float32Array(2 * 4 * pickDiameter * pickDiameter); + data[4 * targetOffset] = 0.25; + data[4 * pickDiameter * pickDiameter + 4 * targetOffset] = 23; + + expect( + resolveNearestPanelPickSample(data, pickOffsetSequence, pickRadius, { + depthBaseOffset: 0, + pickBaseOffset: 4 * pickDiameter * pickDiameter, + }), + ).toEqual({ + offset: targetOffset, + relativeX: targetOffset % pickDiameter, + relativeY: (targetOffset - (targetOffset % pickDiameter)) / pickDiameter, + pickValue: 23, + depthValue: 0.25, + }); + }); +}); + +describe("clearOutOfBoundsPickData", () => { + it("zeros the relative pick-window indices that fall outside the viewport", () => { + const pickRadius = 1; + const pickDiameter = getPickDiameter(pickRadius); + const buffer = new Float32Array(4 * pickDiameter * pickDiameter).fill(1); + + clearOutOfBoundsPickData( + buffer, + 0, + 4, + 0, + 0, + 3, + 3, + pickRadius, + ); + + expect(buffer[0]).toBe(0); + expect(buffer[4]).toBe(0); + expect(buffer[4 * pickDiameter]).toBe(0); + expect(buffer[4 * (pickDiameter + 1)]).toBe(1); + }); +}); diff --git a/src/rendered_data_panel_picking.ts b/src/rendered_data_panel_picking.ts new file mode 100644 index 000000000..9427c42ee --- /dev/null +++ b/src/rendered_data_panel_picking.ts @@ -0,0 +1,154 @@ +/** + * @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. + */ + +export interface ResolvedPanelPickSample { + offset: number; + relativeX: number; + relativeY: number; + pickValue: number; + depthValue?: number; +} + +export function getPickDiameter(pickRadius: number): number { + return 1 + pickRadius * 2; +} + +let _cachedPickRadius = -1; +let _cachedPickOffsetSequence: Uint32Array | undefined; + +/** + * Sequence of offsets into C order (pickDiamater, pickDiamater) array in order of increasing + * distance from center. + */ +export function getPickOffsetSequence(pickRadius: number) { + if (pickRadius === _cachedPickRadius) { + return _cachedPickOffsetSequence!; + } + _cachedPickRadius = pickRadius; + const pickDiameter = getPickDiameter(pickRadius); + const maxDist2 = pickRadius ** 2; + const getDist2 = (x: number, y: number) => + (x - pickRadius) ** 2 + (y - pickRadius) ** 2; + + let offsets = new Uint32Array(pickDiameter * pickDiameter); + let count = 0; + for (let x = 0; x < pickDiameter; ++x) { + for (let y = 0; y < pickDiameter; ++y) { + if (getDist2(x, y) > maxDist2) continue; + offsets[count++] = y * pickDiameter + x; + } + } + offsets = offsets.subarray(0, count); + offsets.sort((a, b) => { + const x1 = a % pickDiameter; + const y1 = (a - x1) / pickDiameter; + const x2 = b % pickDiameter; + const y2 = (b - x2) / pickDiameter; + return getDist2(x1, y1) - getDist2(x2, y2); + }); + return (_cachedPickOffsetSequence = offsets); +} + +/** + * Sets array elements to 0 that would be outside the viewport. + * + * @param buffer Array view, which contains a C order (pickDiameter, pickDiameter) array. + * @param baseOffset Offset into `buffer` corresponding to (0, 0). + * @param stride Stride between consecutive elements of the array. + * @param glWindowX Center x pixel index. + * @param glWindowY Center y pixel index. + * @param viewportWidth Width of viewport in pixels. + * @param viewportHeight Width of viewport in pixels. + */ +export function clearOutOfBoundsPickData( + buffer: Float32Array, + baseOffset: number, + stride: number, + glWindowX: number, + glWindowY: number, + viewportWidth: number, + viewportHeight: number, + pickRadius: number, +) { + const pickDiameter = getPickDiameter(pickRadius); + const startX = glWindowX - pickRadius; + const startY = glWindowY - pickRadius; + if ( + startX >= 0 && + startY >= 0 && + startX + pickDiameter <= viewportWidth && + startY + pickDiameter <= viewportHeight + ) { + return; + } + for (let relativeY = 0; relativeY < pickDiameter; ++relativeY) { + for (let relativeX = 0; relativeX < pickDiameter; ++relativeX) { + const x = startX + relativeX; + const y = startY + relativeY; + if (x < 0 || y < 0 || x >= viewportWidth || y >= viewportHeight) { + buffer[ + baseOffset + (relativeY * pickDiameter + relativeX) * stride + ] = 0; + } + } + } +} + +export function getCenteredPickWindowCoordinate( + glWindowCoordinate: number, + relativeCoordinate: number, + pickRadius: number, +) { + return glWindowCoordinate + relativeCoordinate - pickRadius + 0.5; +} + +export function resolveNearestPanelPickSample( + data: ArrayLike, + pickOffsetSequence: ArrayLike, + pickRadius: number, + options: { + depthBaseOffset?: number; + pickBaseOffset?: number; + stride?: number; + } = {}, +): ResolvedPanelPickSample | undefined { + const { depthBaseOffset, pickBaseOffset = depthBaseOffset ?? 0, stride = 4 } = + options; + const pickDiameter = getPickDiameter(pickRadius); + for (let i = 0; i < pickOffsetSequence.length; ++i) { + const offset = pickOffsetSequence[i]; + const depthValue = + depthBaseOffset === undefined + ? undefined + : data[depthBaseOffset + stride * offset] ?? 0; + if (depthBaseOffset !== undefined && depthValue === 0) { + continue; + } + const pickValue = data[pickBaseOffset + stride * offset] ?? 0; + if (depthBaseOffset === undefined && pickValue === 0) { + continue; + } + const relativeX = offset % pickDiameter; + return { + offset, + relativeX, + relativeY: (offset - relativeX) / pickDiameter, + pickValue, + depthValue, + }; + } + return undefined; +} diff --git a/src/sliceview/panel.ts b/src/sliceview/panel.ts index fe616f185..1d8178a88 100644 --- a/src/sliceview/panel.ts +++ b/src/sliceview/panel.ts @@ -24,9 +24,11 @@ import type { RenderedDataViewerState, } from "#src/rendered_data_panel.js"; import { + getCenteredPickWindowCoordinate, getPickDiameter, getPickOffsetSequence, RenderedDataPanel, + resolveNearestPanelPickSample, } from "#src/rendered_data_panel.js"; import type { SliceView } from "#src/sliceview/frontend.js"; import { SliceViewRenderHelper } from "#src/sliceview/frontend.js"; @@ -469,22 +471,28 @@ export class SliceViewPanel extends RenderedDataPanel { ) { const { mouseState } = this.viewer; mouseState.pickedRenderLayer = null; - const pickDiameter = getPickDiameter(pickRadius); const pickOffsetSequence = getPickOffsetSequence(pickRadius); const { viewportWidth, viewportHeight } = pickingData; - const numOffsets = pickOffsetSequence.length; const { value: voxelCoordinates } = this.navigationState.position; const rank = voxelCoordinates.length; const displayDimensions = this.navigationState.pose.displayDimensions.value; const { displayRank, displayDimensionIndices } = displayDimensions; const setPosition = ( - xOffset: number, - yOffset: number, + relativeX: number, + relativeY: number, position: Float32Array, ) => { - const x = glWindowX + xOffset; - const y = glWindowY + yOffset; + const x = getCenteredPickWindowCoordinate( + glWindowX, + relativeX, + pickRadius, + ); + const y = getCenteredPickWindowCoordinate( + glWindowY, + relativeY, + pickRadius, + ); tempVec3[0] = (2.0 * x) / viewportWidth - 1.0; tempVec3[1] = (2.0 * y) / viewportHeight - 1.0; tempVec3[2] = 0; @@ -502,7 +510,7 @@ export class SliceViewPanel extends RenderedDataPanel { mouseState.coordinateSpace = this.navigationState.coordinateSpace.value; mouseState.displayDimensions = displayDimensions; - setPosition(0, 0, unsnappedPosition); + setPosition(pickRadius, pickRadius, unsnappedPosition); const setStateFromRelative = ( relativeX: number, @@ -513,21 +521,21 @@ export class SliceViewPanel extends RenderedDataPanel { if (mousePosition.length !== rank) { mousePosition = mouseState.position = new Float32Array(rank); } - setPosition( - relativeX - pickRadius, - relativeY - pickRadius, - mousePosition, - ); + setPosition(relativeX, relativeY, mousePosition); this.pickIDs.setMouseState(mouseState, pickId); mouseState.setActive(true); }; - for (let i = 0; i < numOffsets; ++i) { - const offset = pickOffsetSequence[i]; - const pickId = data[4 * i]; - if (pickId === 0) continue; - const relativeX = offset % pickDiameter; - const relativeY = (offset - relativeX) / pickDiameter; - setStateFromRelative(relativeX, relativeY, pickId); + const resolvedPick = resolveNearestPanelPickSample( + data, + pickOffsetSequence, + pickRadius, + ); + if (resolvedPick !== undefined) { + setStateFromRelative( + resolvedPick.relativeX, + resolvedPick.relativeY, + resolvedPick.pickValue, + ); return; } setStateFromRelative(pickRadius, pickRadius, 0);