Enable merger mode in skeleton and hybrid tracings

[MichaelBuessemeyer]

URL of deployed dev instance (used for testing):

• https://enablemergermodeinhybridtracings.webknossos.xyz

Steps to test:

• open a skeleton or hybrid tracing with a segmentation layer
• create some trees
• then open the settings menu and enable the merger mode under "Nodes & Trees"
• the segments that are marked with a node of a tress should now be colored in the same color
• further annotating should all color all annotated segments in the same color (for each tree)
• pressing 8 should ask the user whether to change the color of the segment. If selected yes a random color for the segments should be chosen. Selecting no should do nothing.
• pressing 9 should turn the opacity of the segmentation on and off.
• disabling the merger mode again should turn off all these features but the segments should keep their color

Issues:

• fixes Integrate Merger Mode #3351

---

• Updated changelog
• [ ] Updated migration guide if applicable
• Updated documentation if applicable
• [ ] Needs datastore update after deployment
• Ready for review

[philippotto]

Nice job 👍 Thanks for integrating and cleaning up the merger mode script. I left some comments and also have one other thought:

I know that the overall structure of the script wasn't created by you, but I think we can still improve it. At the moment, the implementation mixes function declarations and high-level calls, which makes the structure a bit hard to grasp in my opinion. My suggestion would be to make use of a structure like this:

// Note that mergerModeState is passed in here, which enables the definition to be
// outside of enableMergerMode
function mapSegmentColorToTree(mergerModeState, segId, treeId) {
 ...
}

...
function getAllNodesWithTreeId() {
  ...
}

async function createNodeOverwrite(...) { ... }


export function enableMergerMode() {
  const unregisterKeyHandlers = [];
  const mergerModeState = {
    mergerModeState: ...,
    segmentationOn: false,
    ...
  };
  unregisterKeyHandlers.push(
    api.utils.registerKeyHandler("9", () => {
      changeOpacity(mergerModeState);
    }),
  );
  // ...
}

export function disableMergerMode() {...}

That way, the enable function would be more concise.

Also, it would be really cool if you could add flow typing to the module :)

[MichaelBuessemeyer]

@philippotto What do you think about improving the UX?

• When already having some trees and then enabling the merger mode, the initial mapping takes quite some time. Maybe a spinner or so would show the user that there is going on some calculation/processing.
• Ask the user whether he/she wants to do this initial processing. (e.g. with a modal)
• ... add more

Btw. this PR is ready for another review 🙂

[philippotto]

Nice, thanks for the refactoring!

Ask the user whether he/she wants to do this initial processing. (e.g. with a modal)

For simplicity, I'd always assume that the user wants the initial processing (that was the previous behavior, too).

When already having some trees and then enabling the merger mode, the initial mapping takes quite some time. Maybe a spinner or so would show the user that there is going on some calculation/processing.

Yes, that's a good idea. I'd suggest to show a modal when checking the checkbox which says:

"You just enabled the merger mode. This mode allows to merge segmentation cells by creating trees and nodes. Each tree maps the marked segments (the ones where nodes were created in) to one new segment."

As long as the initial processing is not done, the "Ok" button of the modal should be disabled. Below the explanation text there could be something like "At the moment, the existing trees are used to merge segments. This dialog can be closed after the initial processing has been completed".

[MichaelBuessemeyer]

@philippotto I added a modal that opens up when the merger mode is enabled. It shortly explains the merger mode and is only closeable when the preprocessing of already existing trees finished.

I also parallelized the initial processing instead of using the recursive variant

[philippotto]

Sorry for letting this slip through the cracks and thanks for nagging me!

I noticed two things during testing:

• when enabling the merger mode checkbox and closing the settings, the checkbox is unchecked again after re-opening the settings. this should be persisted somehow
• the merger mode didn't work for hybrid tracings for me. the reason is probably that the mapping expects the standard "segmentation" layer. probably strongly related to Enable segmentation mappings in tracing modes #3863. maybe just disable merger-mode for hybrid for now?

[MichaelBuessemeyer]

when enabling the merger mode checkbox and closing the settings, the checkbox is unchecked again after re-opening the settings. this should be persisted somehow

I fixed this by creating a new variable in the temporary configuration of the store. The option refresh is caused because the settings view is recreated each time it is reopened. So persisting it via state is not possible so I choose the store to do this 🙂

the merger mode didn't work for hybrid tracings for me. the reason is probably that the mapping expects the standard "segmentation" layer. probably strongly related to #3863. maybe just disable merger-mode for hybrid for now?

This is weird, I tried it out and starting without any node and then enabling the merger mode in hybrid tracings works fine. But having nodes seems to break the preprocessing. I will investigate this.

[MichaelBuessemeyer]

These changes were created when I saved this file. I think this is just an optimization made by the eslint/prettier. In all cases, it should not change the functionality of the methods.

[MichaelBuessemeyer]

@philippotto I slightly changed the example as I misunderstood it by only giving it a quick look. In my opinion, the newer version makes it more clear, that the name of the segmentation layer is not always "segmentation".

[MichaelBuessemeyer]

Here was the original mistake: I used "segmentation" instead of segmentationLayerName which cause the merger mode not to work in hybrid tracing when having at least one node for the preprocessing

[MichaelBuessemeyer]

the merger mode didn't work for hybrid tracings for me. the reason is probably that the mapping expects the standard "segmentation" layer. probably strongly related to #3863. maybe just disable merger-mode for hybrid for now?

I fixed that now. It was just a small mistake caused by sloppiness. I left a comment at the specific line 🙂

[MichaelBuessemeyer]

@philippotto I still need your review on the newest changes / approval for this PR.

[MichaelBuessemeyer]

TODO @MichaelBuessemeyer

• disable merger mode option when mappings are unsupported and add a tooltip for this 🙂

[philippotto]

Also, one last minor point before I'll approve :P

• the "id mapping" checkbox in the segmentation tab should be automatically enabled when enabling the merger mode (unselecting the merger mode or id mapping checkbox doesn't have to do anything, imo)

[philippotto]

Awesome! Happy to see this deployed very soon :)

(Don't forget to address my last nitpick :))