Support transforming the skeleton layer with affine and TSP transforms

[philippotto]

This PR takes care of transforming skeleton nodes (in the shader and when interacting with the positions) when necessary. During saving, the original node positions are stored (i.e., without any applied transformations).
In contrast to data layers (color and segmentation), the skeleton layer doesn't have an own transforms property. Instead it is treated like a layer that has no transforms at all. If a dataset with transformed layers exist, these layers will be transformed according to their transformations while the skeleton layer has the identity transforms. When one of the layers that have transforms is toggled to be rendered natively, the inverse transform is applied to the skeleton layer. As a result, the nodes will be transformed the same way the other layers are transformed.

When using the front-end export of skeletons to NML with transforms, the currently rendered node positions are written while the active transformation is encoded in the parameters section like the following. The array notation is loosely based on the TrakEm format (that uses something like <t2_patch oid=“33” width=“949.0” height=“901.0” transform=“matrix(1.0,0.0,-0.0,1.0,53.0,72.0)... />).

Affine:

<parameters>
    <experiment name="C555_DIAMOND_2f" description="" organization="sample_organization" wkUrl="http://localhost:9000" />
    ...
    <!-- The node positions in this file were transformed using the following affine transform -->
    <transform type="affine" positionsAreTransformed="true" matrix="[2.3271632194519043,12.151433944702148,0.6425480246543884,-5994.43896484375,-12.078832626342773,2.238189697265625,1.314159631729126,5853.94580078125,-1.229456901550293,0.8777050375938416,-12.358572006225586,8174.52685546875,0,0,0,1]" />
    ...
</parameters>

TPS:

  <parameters>
    ...
    <!-- The node positions in this file were transformed using a thin plate spline that was derived from the following correspondences: -->
    <transform type="thin_plate_spline" positionsAreTransformed="true">
      <correspondence source="[570.3021454931336,404.5548993008739,502.2248079151061]" target="[568.1197424615385,528.0132258461539,1622.1119725846156]" />
      ...
  </parameters>
....

If we decide in the future, that we always want to store the untransformed positions in the NML, we can do so and then change positionsAreTransformed="true" to false.

URL of deployed dev instance (used for testing):

• https://transformskeletons.webknossos.xyz/

Steps to test:

• create a new annotation for the C555_DIAMOND_2f-TPS dataset
• note that the skeleton layer in the left sidebar will have an icon representing whether it is rendered with or without transforms
• display the C555_DIAMOND_2f layer natively --> the skeleton layer will now be rendered with transforms
• use the download trees feature in the trees tab
  • there is one normal download (without transforms) and
  • a download with transforms
• the skeleton tool should be disabled now
• test the merger mode
• also test skeleton annotation in an nd dataset (without transforms)

TODOs:

• Disable skeleton annotation when the layer is transformed. otherwise, slightly weird behavior can occur, because the clicked position can slightly differ from the stored position
  • this is because the node positions are stored as integers. this leads to inaccuracies when going from the clicked position to an untransformed position (that is truncated) and then back again to the transformed coordinate space.
  • ~[ ] alternatively, we could change the storage format of node positions to float vectors <-- let's do this only when necessary
• when exporting a NML, do we want to encode which transformation system was applied during the import?
  • which format? how to encode TPS?
  • is it fair to not transform the coordinates when using the back-end download? yes, by default, you get the export as it's stored in the backend. if you need the transformed nodes, you can use the tree download which asks whether you want this.
• how should we treat the back-end export? we could leave it as is and argue that omitted transformation metadata in the NML encode that the node positions were exported without any transforms. then, it would be semantically correct.
• [ ] should the node property "position" in the DB be renamed to "transformedPosition" (as the front-end did it?)

Issues:

• contributes to Coordinate Systems for Annotation Layers #7270

---

(Please delete unneeded items, merge only when none are left open)

• Updated changelog
• Updated migration guide if applicable
• Updated documentation if applicable
• Adapted wk-libs python client if relevant API parts change
• Removed dev-only changes like prints and application.conf edits
• Considered common edge cases
• Needs datastore update after deployment

[philippotto]

@fm3 I adapted the front-end so that a node position is transformed at the latest possible time (e.g., in the shader or shortly before presenting the current position in the UI). Within the redux store, the untransformed position is stored and that is also sent to the server. To make the semantics clear, I renamed position to untransformedPosition everywhere, except when doing the communication with the back-end. That way, everything works without having to change the back-end. However, I'm wondering whether it makes sense to also adapt the back-end (since there are now some places where the property renaming between position <> untransformedPosition has to be done). What do you think about this? I assume it would be complicated because of the update actions that are already stored in the DB, right?

[fm3]

I assume it would be complicated because of the update actions that are already stored in the DB, right?

That’s right. We would either have to migrate all existing update actions or make both fields optional and use getOrElse logic, with an explicit error if both fields are None. While that would be possible, I don’t know if it would be nicer than just doing the renaming as you said

[philippotto]

That’s right. We would either have to migrate all existing update actions or make both fields optional and use getOrElse logic, with an explicit error if both fields are None. While that would be possible, I don’t know if it would be nicer than just doing the renaming as you said

Do you have a preference? I think, I'm against the getOrElse logic. This leaves us with two options: 1) keep it as is, 2) rename all existing update actions. I think, 2 would be a bit cleaner (but can also be weird, since untransformedPosition can be a bit confusing when there are no transforms in the first place). I'd be fine with 1, too, if you are?

[daniel-wer]

@philippotto Reviewing, the following came to my mind: Do you think it would be helpful to leverage typescript to prevent mixing untransformed and transformed positions? The way to do it would be nominal/branded/opaque types, see this example.

[daniel-wer]

Very cool that this is supported now! I'll report back after testing 🧪

[daniel-wer]

Works very well 🎉 Mainly, I think the NML export/import could be improved as well as two other small issues. My notes from testing:

• The icons for the skeleton layer in the layers tab are not vertically aligned
  
• The tps correspondences are serialized as <correspondence source="[570.3021454931336,404.5548993008739,502.2248079151061]" target="[568.1197424615385,528.0132258461539,1622.1119725846156]" />. Is the [570.3021454931336,404.5548993008739,502.2248079151061] part XML-idiomatic and can it be parsed without resorting to eval or custom string splitting? Otherwise I'd change it to something more idiomatic.
• As mentioned in one of the comments: Exporting a transformed skeleton and then importing it into the same annotation when the skeleton layer is no longer transformed leads to the nodes ending up in the wrong location. When fixing this, pay attention to the fact that NMLs currently can also be imported when the skeleton is transformed. We decided against this for now. See comment.
• I'm not sure whether this broke in this PR: The merger mode modal states "8 Replace the color of the current active tree and its mapped segments with a new one.", but pressing 8 doesn't change anything. see below

[philippotto]

@philippotto Reviewing, the following came to my mind: Do you think it would be helpful to leverage typescript to prevent mixing untransformed and transformed positions? The way to do it would be nominal/branded/opaque types, see this example.

In principle, I think, it's a pretty cool idea :) However, I'm a bit afraid that it will require quite some effort for two reasons. First, it only adds safety when the consumer of the node position is strict and requires a special (Not)TransformedPosition. This only works well when the consumer is a function that puts this requirement into the function's interface. For example, code like this:

    const getPos = (node: Readonly<MutableNode>) => getNodePosition(node, state);

    for (const edge of tree.edges.all()) {
      const sourceNode = tree.nodes.get(edge.source);
      const targetNode = tree.nodes.get(edge.target);
      lengthNmAcc += V3.scaledDist(getPos(sourceNode), getPos(targetNode), datasetScale);
      lengthVxAcc += V3.length(V3.sub(getPos(sourceNode), getPos(targetNode)));
    }

cannot easily require a (Not)TransformedPosition without defining a new function that accepts an array of (Not)TransformedPosition to do the computation. This means, that we would need to potentially extract quite a lot of code so that we can add the type constraints.

Second, mutation of positions via V3 (or similar) would probably lose the added semantics (we would need to find a workaround for that).

For these reasons, I'd rather not do this refactoring right now. I hope you agree!

[philippotto]

The tps correspondences are serialized as . Is the [570.3021454931336,404.5548993008739,502.2248079151061] part XML-idiomatic and can it be parsed without resorting to eval or custom string splitting? Otherwise I'd change it to something more idiomatic.

It can be parsed with JSON.parse which is why I find it acceptable. Still, I don't find it really great, but the alternative with nested XML tags seemed too convoluted to me and TrakEm also does it similarily..

[daniel-wer]

For these reasons, I'd rather not do this refactoring right now. I hope you agree!

Definitely agree, thanks for thinking this through :)

It can be parsed with JSON.parse which is why I find it acceptable. Still, I don't find it really great, but the alternative with nested XML tags seemed too convoluted to me and TrakEm also does it similarily..

Ok, convinced 👍

[philippotto]

@daniel-wer Thanks again for your review and testing 🙏 Now that our discussion has settled for now, the PR is ready from my point of view. There's certainly potential for improvement in the future, but as mentioned in another comment, I'd defer this for now.

I'm not sure whether this broke in this PR: The merger mode modal states "8 Replace the color of the current active tree and its mapped segments with a new one.", but pressing 8 doesn't change anything.

Very interesting! I did some digging and it turns out that this feature was removed a while back in 8124079. However, the modal wasn't updated then. I simply removed the hint from the modal now, because nobody complained about this feature being gone.

[daniel-wer]

Code LGTM and my previous points from testing were addressed :)

• During a renewed testing I noticed that the skeleton can still be modified even when the skeleton layer is transformed either by using keyboard shortcuts (Ctrl + Arrow Keys to move nodes, for example) or by using the context menu (Rightclick - Create Node Here). I'm also wondering whether "Import Agglomerate Skeleton" or "Import Synapses" from the context menu would do the correct thing for transformed skeleton layers or whether the entries should be disabled (maybe they are already, I didn't have an agglomerate file available for a transformed dataset).

[philippotto]

During a renewed testing I noticed that the skeleton can still be modified even when the skeleton layer is transformed either by using keyboard shortcuts (Ctrl + Arrow Keys to move nodes, for example) or by using the context menu (Rightclick - Create Node Here).

Good find! I fixed it now :)

I'm also wondering whether "Import Agglomerate Skeleton" or "Import Synapses" from the context menu would do the correct thing for transformed skeleton layers or whether the entries should be disabled (maybe they are already, I didn't have an agglomerate file available for a transformed dataset).

To properly support this scenario, we would need the ability that a skeleton layer uses the same transforms as a volume layer. We should tackle this once the need occurs :)

[daniel-wer]

LGTM 🎉