Skip to content
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

feat(annotation) Add default annontation properties #694

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

feat(annotation) Add default annontation properties #694

Show file tree
Hide file tree
Changes from 3 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
57 changes: 57 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,6 +54,7 @@ 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";

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

export const propertyTypeDataType: Record<
Expand Down Expand Up @@ -496,6 +498,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 +700,13 @@ export interface AnnotationTypeHandler<T extends Annotation = Annotation> {
annotation: T,
callback: (vec: Float32Array, isVector: boolean) => void,
) => void;
defaultProperties: (
annotation: T,
scales: Float64Array,
) => {
properties: AnnotationNumericPropertySpec[];
values: number[];
};
}

function serializeFloatVector(
Expand Down Expand Up @@ -814,6 +826,33 @@ export const annotationTypeHandlers: Record<
callback(annotation.pointA, false);
callback(annotation.pointB, false);
},
defaultProperties(annotation: Line, scales: Float64Array) {
const properties: AnnotationNumericPropertySpec[] = [];
const values: number[] = [];
const rank = scales.length;
if (
rank === annotation.pointA.length &&
rank === annotation.pointB.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.

format: formatLength,
});
let length = 0;
for (let dim = 0; dim < rank; dim++) {
length +=
(((annotation.pointA[dim] - annotation.pointB[dim]) / 1e-9) *
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shouldn't always be nanometers --- it should take into account the units set in the coordinate space.

If all dimensions have the same base unit, e.g. meter, then you could just choose an appropriate scale automatically as in the scale bar.

If there are mixed units, e.g. meters and seconds, then probably just don't display the length at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated it to use globalCoordinateSpace.units and using ALLOWED_UNITS from scale_bar.ts to disable the property. If any unit is a non-length unit, it won't add the property. Also cleaned up the code

scales[dim]) **
2;
}
length = Math.sqrt(length);
values.push(length);
}
return { properties, values };
},
},
[AnnotationType.POINT]: {
icon: "⚬",
Expand Down Expand Up @@ -858,6 +897,11 @@ export const annotationTypeHandlers: Record<
visitGeometry(annotation: Point, callback) {
callback(annotation.point, false);
},
defaultProperties(annotation: Point, scales: Float64Array) {
annotation;
scales;
return { properties: [], values: [] };
},
},
[AnnotationType.AXIS_ALIGNED_BOUNDING_BOX]: {
icon: "❑",
Expand Down Expand Up @@ -926,6 +970,14 @@ export const annotationTypeHandlers: Record<
callback(annotation.pointA, false);
callback(annotation.pointB, false);
},
defaultProperties(
annotation: AxisAlignedBoundingBox,
scales: Float64Array,
) {
annotation;
scales;
return { properties: [], values: [] };
},
},
[AnnotationType.ELLIPSOID]: {
icon: "◎",
Expand Down Expand Up @@ -994,6 +1046,11 @@ export const annotationTypeHandlers: Record<
callback(annotation.center, false);
callback(annotation.radii, true);
},
defaultProperties(annotation: Ellipsoid, scales: Float64Array) {
annotation;
scales;
return { properties: [], values: [] };
},
},
};

Expand Down
20 changes: 17 additions & 3 deletions src/ui/annotations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1854,6 +1854,19 @@ 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, globalCoordinateSpace.scales);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still not correct --- you need to transform the scales, taking into account the various affine transforms that apply, similar to what is done here:

neuroglancer/src/ui/annotations.ts

Line 1073 in 5898236

function getMousePositionInAnnotationCoordinates(

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 +1885,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 +1901,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