Skip to content

feat(annotation) Add default annotation properties #694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

feat(annotation) Add default annotation properties #694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
47f8d34
feat(annotation) Add default annontation properties
chrisj Jan 11, 2025
356c42c
added optional format override property to AnnotationNumericPropertySpec
chrisj Jan 16, 2025
03eaf93
no longer assume three dimensions when calculating default annotation…
chrisj Jan 16, 2025
324342f
use globalCoordinateSpace.units in defaultProperties rather than assu…
chrisj Jan 17, 2025
43c3e4a
fix default annotation properties to use transformed coordinates
chrisj Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions src/annotation/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ import {
import { parseDataTypeValue } from "#src/util/lerp.js";
import { getRandomHexString } from "#src/util/random.js";
import { NullarySignal, Signal } from "#src/util/signal.js";
import { formatLength } from "#src/util/spatial_units.js";
import { Uint64 } from "#src/util/uint64.js";
import { ALLOWED_UNITS } from "#src/widget/scale_bar.js";

export type AnnotationId = string;

Expand Down Expand Up @@ -106,6 +108,7 @@ export interface AnnotationNumericPropertySpec
min?: number;
max?: number;
step?: number;
format?: (x: number) => string;
}

export const propertyTypeDataType: Record<
Expand Down Expand Up @@ -496,6 +499,9 @@ export function formatNumericProperty(
property: AnnotationNumericPropertySpec,
value: number,
): string {
if (property.format) {
return property.format(value);
}
const formattedValue =
property.type === "float32" ? value.toPrecision(6) : value.toString();
const { enumValues, enumLabels } = property;
Expand Down Expand Up @@ -695,6 +701,15 @@ export interface AnnotationTypeHandler<T extends Annotation = Annotation> {
annotation: T,
callback: (vec: Float32Array, isVector: boolean) => void,
) => void;
defaultProperties: (
annotation: T,
layerPosition: Float32Array<ArrayBufferLike>[],
scales: Float64Array,
units: readonly string[],
) => {
properties: AnnotationNumericPropertySpec[];
values: number[];
};
}

function serializeFloatVector(
Expand Down Expand Up @@ -751,6 +766,32 @@ function deserializeTwoFloatVectors(
return offset;
}

function lineLength(
annotationLayerPositions: Float32Array<ArrayBufferLike>[],
scales: Float64Array,
units: readonly string[],
) {
if (annotationLayerPositions.length !== 2) {
return;
}
const [pointA, pointB] = annotationLayerPositions;
const scalesRank = scales.length;
const lineRank = pointA.length;
if (scalesRank < lineRank) {
return;
}
let lengthSquared = 0;
for (let dim = 0; dim < lineRank; dim++) {
const unitInfo = ALLOWED_UNITS.find((x) => x.unit === units[dim]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

units will always be normalized to "m" if it is some multiple of meters, so there is no need to check ALLOWED_UNITS.

Note that ALLOWED_UNITS is actually from some now-dead code that was originally used to show annotation properties just like you are adding with this PR.

However, you may as well support any unit for length --- just as long as all units for which there is a non-zero delta are the same.

Or perhaps it could be represented as X meters + Y seconds, if for example there are both meters and seconds units. You would compute the length separately for each unit.

The actual SI prefix can be selected using pickSiPrefix, as for the scale bar.

if (!unitInfo) {
return;
}
const voxelToNanometers = scales[dim] * unitInfo.lengthInNanometers;
lengthSquared += ((pointA[dim] - pointB[dim]) * voxelToNanometers) ** 2;
}
return Math.sqrt(lengthSquared);
}

export const annotationTypeHandlers: Record<
AnnotationType,
AnnotationTypeHandler
Expand Down Expand Up @@ -814,6 +855,28 @@ export const annotationTypeHandlers: Record<
callback(annotation.pointA, false);
callback(annotation.pointB, false);
},
defaultProperties(
annotation: Line,
annotationLayerPositions: Float32Array<ArrayBufferLike>[],
scales: Float64Array,
units: readonly string[],
) {
annotation;
const properties: AnnotationNumericPropertySpec[] = [];
const values: number[] = [];
const length = lineLength(annotationLayerPositions, scales, units);
if (length) {
properties.push({
type: "float32",
identifier: "Length",
default: 0,
description: "Length of the line annotation in nanometers",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably just say "length of the line annotation", with a type of string, and then the value can include the appropriate unit suffix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jeremy, getting back to this now, are you suggesting adding "string" as a type of AnnotationNumericPropertySpec or Adding a AnnotationStringPropertySpec. I thought there was no string currently for performance reasons.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I forgot that string properties aren't supported for annotations --- only for segment properties. They would be challenging to support within a shader, and some changes to the precomputed annotation encoding would be needed to support strings in that format, but there is no fundamental difficulty with supporting them solely for display in the "Selection details" panel. However, I realize that with the format member you added to AnnotationNumericPropertySpec we can anyway accomplish the same thing without adding string annotation support.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, would you suggest skipping the "X meters + Y seconds" idea and instead only calculate length if all the non-zeros are the same unit?

format: formatLength,
});
values.push(length);
}
return { properties, values };
},
},
[AnnotationType.POINT]: {
icon: "⚬",
Expand Down Expand Up @@ -858,6 +921,18 @@ export const annotationTypeHandlers: Record<
visitGeometry(annotation: Point, callback) {
callback(annotation.point, false);
},
defaultProperties(
annotation: Point,
layerPosition: Float32Array<ArrayBufferLike>[],
scales: Float64Array,
units: string[],
) {
annotation;
layerPosition;
scales;
units;
return { properties: [], values: [] };
},
},
[AnnotationType.AXIS_ALIGNED_BOUNDING_BOX]: {
icon: "❑",
Expand Down Expand Up @@ -926,6 +1001,18 @@ export const annotationTypeHandlers: Record<
callback(annotation.pointA, false);
callback(annotation.pointB, false);
},
defaultProperties(
annotation: AxisAlignedBoundingBox,
layerPosition: Float32Array<ArrayBufferLike>[],
scales: Float64Array,
units: string[],
) {
annotation;
layerPosition;
scales;
units;
return { properties: [], values: [] };
},
},
[AnnotationType.ELLIPSOID]: {
icon: "◎",
Expand Down Expand Up @@ -994,6 +1081,18 @@ export const annotationTypeHandlers: Record<
callback(annotation.center, false);
callback(annotation.radii, true);
},
defaultProperties(
annotation: Ellipsoid,
layerPosition: Float32Array<ArrayBufferLike>[],
scales: Float64Array,
units: string[],
) {
annotation;
layerPosition;
scales;
units;
return { properties: [], values: [] };
},
},
};

Expand Down
28 changes: 25 additions & 3 deletions src/ui/annotations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1783,6 +1783,8 @@ export function UserLayerWithAnnotationsMixin<
icon.textContent = handler.icon;
positionGrid.appendChild(icon);

const annotationLayerPositions: Float32Array<ArrayBufferLike>[] =
[];
if (layerRank !== 0) {
const { layerDimensionNames } = (
chunkTransform as ChunkTransformParameters
Expand All @@ -1800,6 +1802,7 @@ export function UserLayerWithAnnotationsMixin<
annotation,
chunkTransform as ChunkTransformParameters,
(layerPosition, isVector) => {
annotationLayerPositions.push(layerPosition);
const copyButton = makeCopyButton({
title: "Copy position",
onClick: () => {
Expand Down Expand Up @@ -1854,6 +1857,24 @@ export function UserLayerWithAnnotationsMixin<

const { relationships, properties } = annotationLayer.source;
const sourceReadonly = annotationLayer.source.readonly;
const globalCoordinateSpace =
this.manager.root.coordinateSpace.value;
const defaultProperties = annotationTypeHandlers[
annotation.type
].defaultProperties(
annotation,
annotationLayerPositions,
globalCoordinateSpace.scales,
globalCoordinateSpace.units,
);
const allProperties = [
...defaultProperties.properties,
...properties,
];
const allValues = [
...defaultProperties.values,
...annotation.properties,
];

// Add the ID to the annotation details.
const label = document.createElement("label");
Expand All @@ -1872,8 +1893,10 @@ export function UserLayerWithAnnotationsMixin<
label.appendChild(valueElement);
parent.appendChild(label);

for (let i = 0, count = properties.length; i < count; ++i) {
const property = properties[i];
for (let i = 0, count = allProperties.length; i < count; ++i) {
const property = allProperties[i];
const value = allValues[i];

const label = document.createElement("label");
label.classList.add("neuroglancer-annotation-property");
const idElement = document.createElement("span");
Expand All @@ -1886,7 +1909,6 @@ export function UserLayerWithAnnotationsMixin<
if (description !== undefined) {
label.title = description;
}
const value = annotation.properties[i];
const valueElement = document.createElement("span");
valueElement.classList.add(
"neuroglancer-annotation-property-value",
Expand Down
Loading