Page MenuHomePhabricator

Segmentation View and Multilabel Segmentation must not be active together (toolmanager inconsistency)
Open, HighPublic

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 constantly 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.

Event Timeline

floca triaged this task as High priority.Fri, Jan 31, 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).Thu, Feb 13, 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
floca added a comment.Fri, Feb 14, 9:42 PM

@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.