Page MenuHomePhabricator

Segmentation View and Multilabel Segmentation must not be active together (toolmanager inconsistency)
Closed, ResolvedPublic

Description

Due to the fact that both segmentation views use the same toolmanager instance, we run into inconstencies invalid states if both views are opened in a workbench session.
We have a kind of race condition, because both views set the reference image and work image of the toolmanager due to various events (from node change to rendering reinit). And the views also use the toolmanager data as a kind of internal state because the continiuosly use it like there own member variable. This leads sooner then later to invalid states, strange view behavior or crashes.

Seperating it or making it lokal is not trival and basically leads to a complete overhaul of the segmentation (which would not the worest).

The most sensible but pragmatic mitigation strategy would be to prevent that both plugins are not part of the same package. Either be documentation (weak) or by adding a check on the CMake level preventing that both plugins can be activated at the same time.

Revisions and Commits

rMITK MITK
Restricted Differential Revision
Restricted Differential Revision

Event Timeline

floca triaged this task as High priority.Jan 31 2020, 8:13 PM
floca created this task.
kalali renamed this task from Segmentation View and Multilabel Segmentation must not be active together (toolmanager inconsitency) to Segmentation View and Multilabel Segmentation must not be active together (toolmanager inconsistency).Feb 13 2020, 11:09 AM
floca added a subscriber: kislinsk.

We discussed it:

  • CMake based mutual exclusion is not an option
  • one mitigation strategy could be to introduce a kind of context for the toolmanagerprovider. @kislinsk will look into it, if it is a possible solution.
  • Or we could start to make the segmentation right :P

@kislinsk I thought about it again after our conversation and checked the code base. I really tend to not introduce context to the toolmanager provider (see above) as it is tinkering for a already flawed design.
I have looked into the code, there are only areas where the provider is used.

  1. in the seg views
  2. in two context menu action (mask2label and surface2label
  3. in several SegmentationUI widgets used by the views
  4. in an interactor added for MultiLabel

Making it local in 1 and 3 is work but no problem. Widgets of 3 where originially designed to be local and have a hack to also use the Provider.

  1. I think is better/sufficient located in the MultiLabelSegmentation Utility View.
  2. Is the only thing I am not quite sure how to remove the provider but a) I have not enough exp with interactions and b) it has a smell/bad felling to missuse the provider here to reach the labelImage. There must be cleaner ways.

So ultimatly I think we should realy think of taking the effort (after checking the open questions) instead of adding context. The refactoring will also not be lost if we Seg 4.0 may come in the future because it can be seen as preperational work. My 2 cents. If you have other opinions, let me know.

I think we should still go for the context version because it still is a rather easy and backward-compatible fix for something that we actually don't want to recycle for Seg 4.0 as a microservice-based approach for tools doesn't really need anything fancy from the current solution and is not a big challenge to implement without preperational work. We also do not know how often the current mechanism is used by our external users but it certainly is. I'd prefer to avoid the hassle of a deprecation process. Last but not least it's a priority/time issue regarding all the work that is necessary already for the upcoming release.

kislinsk added a revision: Restricted Differential Revision.May 28 2020, 5:22 PM