-
Notifications
You must be signed in to change notification settings - Fork 302
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.
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
base: master
Are you sure you want to change the base?
Conversation
add line length property
c826934
to
47f8d34
Compare
Overall I like this idea, thanks! Originally some default properties like length, volume, etc. were shown for annotations but that support was left out when I refactored some years ago to support more than 3 dimensions. The logic related to scales will need to change ---- the coordinate space of the annotation is not necessarily 3d and doesn't necessarily directly correspond to the global coordinate space, so some coordinate transforms will need to happen, and units taken into account also. |
src/ui/annotations.ts
Outdated
this.manager.root.coordinateSpace.value; | ||
const defaultProperties = annotationTypeHandlers[ | ||
annotation.type | ||
].defaultProperties(annotation, globalCoordinateSpace.scales); |
There was a problem hiding this comment.
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( |
src/annotation/index.ts
Outdated
let length = 0; | ||
for (let dim = 0; dim < rank; dim++) { | ||
length += | ||
(((annotation.pointA[dim] - annotation.pointB[dim]) / 1e-9) * |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
c45bba8
to
aad7550
Compare
…ming meters split line length code into a separate function
aad7550
to
324342f
Compare
The points stored in the annotation are in the "model" coordinate space. There is an affine transform that maps between that coordinate space and the concatenation of the layer-local and global coordinate spaces. The lengths could be computed either in the "model" coordinate space or in the global coordinate space,.or maybe both, but right now you are using the untransformed points in the model coordinate space with the global coordinate space scales, which is not correct. |
add line length property
This came up during a meeting that the scale bar has been used as a rough measure of the size of features in datasets. I was thinking we should have a better way to measure distance and the line annotation is already a tool that has most of the requirements.
I think it would also make sense to add additional properties for other types such as ellipsoid volume and semi axis lengths, bounding box volume and dimensions.
I am currently making assumptions to cast float32array to a vec3 and there might be a better method of using the globalCoordinateSpace.