Improve segment proofreading in 3D viewport

[philippotto]

• allows to activate segment via mesh in 3D viewport
• highlights supervoxels on hover
• allows merging/splitting in 3d viewport by interacting with the agglomerates / supervoxels

URL of deployed dev instance (used for testing):

• https://___.webknossos.xyz

Steps to test:

• create new annotation
• select a hdf5 mapping
• select proofreading tool and load some meshes
• try to split and merge them in the 3D viewport

TODOs:

• backend:
  • as an alternative to positions, also accept unmappedSegmentIds for split/merge related operations
  • implement API that maps from segmentId to a segmentPosition
• frontend
  • integrate the above ino the context menu in the 3D viewport
  • don't show the proofreading cross in the data viewports if the user activated a supervoxel in the 3D viewport
  • bugs
    • loading the same mesh twice breaks the hovering effect for unknown reasons
    • after proofread operation, selected super voxel is not highlighted anymore (but stored as active)
    • when creating a new annotation, a new active segment id is chosen. when switching to proofreading, one can try to merge with that segment id. however, it doesn't exist anywhere.
    • [ ] selecting one partner in the 3D viewport and the other in a data viewport (or vice versa) doesn't work the user is told so now. hopefully can be improved with Load Oversegmentation Meshes during Proofreading #6569

Issues:

• contributes to Improve Proofreading #7619

---

(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 When you have some time, it would be great if you could make the splitAgglomerate and mergeAgglomerate compatible with passing the unmapped segment ids. The same basically for the /agglomerateGraphMinCut and /getNeighborsForAgglomerateNode route 🙏

[fm3]

@philippotto I’ve done that now, please give it a try! The new params are called segmentId1, segmentId2, segmentId

I assume that the frontend now always has the segment ids, right? Applying the update actions is backwards compatible, but do I need to build the same for the mincut/neighbors routes?

[philippotto]

@philippotto I’ve done that now, please give it a try! The new params are called segmentId1, segmentId2, segmentId

Awesome, thanks, I'll give it a try.

I assume that the frontend now always has the segment ids, right?

We don't have the "magic mapping", yet, but for the typical split/merge proofreading operations we already have the (unmapped) segment ids, yes.

Applying the update actions is backwards compatible, but do I need to build the same for the mincut/neighbors routes?

For the mincut/neighbors route in non-3d-viewport, we don't have the unmapped id yet, but we can do another request to get these (like we do it for split/merge). These round trips will then be eliminated with the magic mapping soon.

[fm3]

For the mincut/neighbors route in non-3d-viewport, we don't have the unmapped id yet, but we can do another request to get these (like we do it for split/merge). These round trips will then be eliminated with the magic mapping soon.

Right, I wasn’t sure about that. If it’s complicated to do these roundtrips, I can also make the routes accept both variants

[philippotto]

@MichaelBuessemeyer Thank you for your thorough testing and feedback 👍 I incorporated most of your feedback, fixed the hiccups you noticed and did some performance optimizations. Please have another look and feel free to test again :)

[MichaelBuessemeyer]

Thanks for incorporating the feedback 🙏

Unfortunately, I'm not sure how to do this well. As an example, the store has a property for the active cell id (should be renamed to active segment id but this is another issue ^^). If there is a mapping, this refers to the mapped id, which could also be called agglomerate id. However, if there is no mapping, "agglomerateId" is highly misleading..

Hmm, similar to the other thread above, I'm not sure how to do this well. I wouldn't want to use unmappedSegmentMesh, because (a) it's a confusing term for the frequent case where no mappings exist in the first place and (b) when a mapping exists, the back-end maps the individual mesh chunks to the requested id and then returns these chunks. So, they are mapped in a sense. I'm not very happy about it, either, but I'm having trouble to see how to it can be improved easily..

I can see your valid points :/. It make more sense to keep it the way it is 👍

During testing I encountered the following unexpected behaviours:

• When I select the Split from all neighboring segments action, I get a A precomputed mesh could not be loaded for this segment. More information was printed to the browser's console. warning toast. All segments and buckets are reloaded as expected but this warning toast should not occur imo.

• Moreover, during testing I came up with another potential useful feature: When I hover a segment in the data viewports the (mapped) segment id is displayed in the footer bar. But if I hover a supervoxel its mapped segment is not displayed in the footer bar. Would it be possible to add this / create a new issue for this?
  In particular I needed this use case to check whether two meshes belong to the same segment or not. This was pretty difficult as their color was nearly identical (while one of the was hovered). To check this, I needed to find the segment in the data viewport and hover them there. Imo supporting 3d viewports here as well would help out a lot in this case :)

Thanks all I could find 🎉

Btw: Thanks for getting rid of unnecessary rerendering of components once the flycam changed 🚀

[philippotto]

When I select the Split from all neighboring segments action, I get a A precomputed mesh could not be loaded for this segment. More information was printed to the browser's console. warning toast. All segments and buckets are reloaded as expected but this warning toast should not occur imo.

Can you reproduce this reliably? If so, could you share the steps for reproduction? :)

[MichaelBuessemeyer]

Can you reproduce this reliably? If so, could you share the steps for reproduction? :)

Oh sorry, for me this behaviour occurred every time I used "Split from all neighboring segments" thus I thought that no explicit steps are needed.

I used the following steps:

• load https://bettermeshcontextmenu.webknossos.xyz/annotations/66310e57010000ad00313c3d#2850,4318,1770,0,1
• load segment "Segment 102775"
• in the 3d viewport somewhere at the nucleus of the segment / cell use the "Split from all neighboring segments" option from the context menu. Then the warning should pop up

[philippotto]

Thanks for the repro steps 🙏 Indeed, it happens quite frequently on the dev instance for me, too.

@fm3 The error is coming from the back-end (the front-end might still be at fault for sending weird inputs) and I'm not sure whether I interpet it correctly.

The frontend uses the /meshes/chunks route and sends a payload à la:

{
  "meshFile": "meshfile_4-4-2",
  "segmentId": 7571522
}

Error:

[
  {
    "error": "Failed to load chunk list for segment {0} from mesh file “{1}”"
  },
  {
    "chain": "[Server Time 2024-05-02 08:04]  <~ zero chunks"
  }
]

The string interpolation doesn't seem to work, but I guess the backend simply cannot find chunks for id 7571522. Probably because that supervoxel doesn't exist in the editable mapping, anymore?

[philippotto]

When I select the Split from all neighboring segments action, I get a A precomputed mesh could not be loaded for this segment. More information was printed to the browser's console. warning toast. All segments and buckets are reloaded as expected but this warning toast should not occur imo.

As discussed here, this issue is unrelated to this PR and even occurs in view-only mode. I created this issue for the problem.

@MichaelBuessemeyer It would be great if you could have a final look at my latest commits. I'd love to merge the PR today unless someone vetoes :)

[MichaelBuessemeyer]

Thanks for investigating the bug I found 🙏 as well as making it possible to deselect a super voxel via ctrl + click.

During checking your changes I found a little bug in them:

You redefined the tracing-mesh_listing_failed message in messages.tsx to a function but did not adjust the case where this message is used. Please see my indicating in line comments.
I was able to confirm the bug in the dev instance -> the displayed toast is empty:

That's all 🎉.

After this little fix, this PR is ready to go 🏃

[philippotto]

During checking your changes I found a little bug in them:

Argh, you are right. TypeScript should have caught this, too, but didn't 😢 Maybe because a function can be a react component and the toast is able to render this. Still... I fixed it now!