Page MenuHomePhabricator

Segmentation plugin: "new" segmentation should not run reinit!
Closed, ResolvedPublic

Description

When creating a new segmentation ("New..." button) the position of the views is reset to whatever the default is. This can be really annoying given the following (and probably often occurring) workflow:

  • load an image
  • navigate to the position that you want to annotate (this position can be hard to find)
  • notice that you forgot to create a segmentation
  • press "New..."
  • view resets automatically
  • shit. gotta find object of interest again :-D

I know, this is a minor thing. But I ran into it multiple times and began feeling frustrated :-)

Best,
Fabian

image.png (21×441 px, 5 KB)

Ubuntu 18.04

Revisions and Commits

rMITK MITK
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision

Event Timeline

isensee updated the task description. (Show Details)
kalali triaged this task as High priority.May 19 2021, 11:51 AM
kalali edited projects, added MITK (v2021.10); removed MITK.
kalali added a subscriber: kalali.

I agree on that. I had some problems with that with the new mxn-multiwidget. The topic was also mentioned in T27613: Improve reinit behavior.

We should look into this and focus on the mentioned related task by @kislinsk to start with separating the two processes:

  1. matching the geometry of the images for interaction
  2. resetting the view of render windows to see the new segmentation

The QmitkNewSegmentationDialog, which is used for the "New-button" does not contain any logic for resetting the render windows.
It can be used to simply create a new segmentation, e.g. when using the QmitkCreateMultiLabelSegmentationAction from the data manager context menu - here this dialog is used but no re-initialization / view resetting is done.

The QmitkNewSegmentationDialog is - as mentioned in the task description - mostly used for creating new segmentation nodes from within the (multilabel) segmentation view.
Here the dialog is used inside the QmitkSegmentationView and the QmitkMultiLabelSegmentationView within the CreateNewSegmentation- and the OnNewLabel-function. Both functions end with RenderingManager::GetInstance()->InitializeViews and the argument RenderingManager::REQUEST_UPDATE_ALL, which leads to the undesired re-initialization.

A first idea was mentioned here:

Just for testing purposes: I created an additional Enum value to mitk::RenderingManager::RequestType, called NO_UPDATE. [...]

This allows to disable resetting the view / crosshair etc. after a new segmentation has been created.

After a new segmentation node was been created, it is added to the data manager. This event itself will call RequestUpdateAll multiple times. However, the view will not be reset with the mentioned new value NO_UPDATE - that means, requesting a render update does not change the view / crosshair / camera orientation etc. The view is only updated / reset when InitializeViews or InitializeViewsByBoundingObjects is called (which in turn calls InternalViewInitialization and updates the slice navigation controller).

I see no difference between calling InitializeViews with the new Enum value NO_UPDATE and not calling InitializeViews at all, solely using mitk::RenderingManager::GetInstance()->RequestUpdateAll() to correctly set the rendering geometry without changing the view after a new segmentation nodes has been created.
However, both variants do not allow segmenting on the new segmentation node as the warning message shows, stating that a reinit should be performed on the segmentation image. The warning message appears in the scenario where I changed the camera view before creating a new segmentation:

NO_UPDATE_02.png (1×2 px, 229 KB)

In another tasks / comment, I stated that it is actually possible to use the new Enum value NO_UPDATE to correctly set the rendering geometry without producing the warning message.
So I looked closer and found out the following:

  • testing it with Pic3D.nrrd works pretty stable
  • testing it with brain.nrrd does not work in all cases
    • creating a new segmentation using one of the above mentioned approaches (such that the view is not reinitialized) creates the warning message
    • creating a new segmentation after performing a reinit on the brain.nrrd data node using one of the above mentioned approaches works fine
    • creating a new segmentation after performing a reinit on the brain.nrrd data node AND changing the view to any arbitrary perspective (e.g. as in the picture shown above) works fine

With "works fine" I mean the following:

  • the segmentation node is created and added to the data storage
  • the view is not being reset
  • correctly segmenting on the image using the current view works correctly

I also tested everything with US4DCyl and I was able to create a new segmentation without performing a reinit on the data node before segmentation creation. The newly created segmentation fits perfectly and I was able to correctly draw a segmentation mask on the uninitialized image.

So my conclusion from this: Neither the NO_UPDATE argument nor the InitializeViews function is required at all. However, this only works if the data node / image itself is correctly loaded. It seems as there is a difference between the initialization that is performed when loading the image and the initialization that is performed when calling InitializeViews or using the reinit-action from the context menu.

Note: One problem still remains: If another node is reinitialized (a node that is not selected as reference node inside the segmentation plugin view) then the warning message appears again. So in the end it is not possible to create and use a segmentation on an image if this image was not the last image to be reinitialized.
We need to look deeper into the segmentation plugin to find out if it is possible to create and draw a segmentation without a correctly initialized reference image.
But for this task (description mentions just a single image / data node) the proposed solution should work.

So my conclusion from this: [...] It seems as there is a difference between the initialization that is performed when loading the image and the initialization that is performed when calling InitializeViews or using the reinit-action from the context menu.

I tested this by changing

// calculate bounding geometry of these nodes
bounds= dataStorage->ComputeBoundingGeometry3D(filteredNodes, "visible");

to

mitk::TimeGeometry::ConstPointer bounds;
  if (!filteredNodes->empty())
  {
    // calculate bounding geometry of these nodes
    auto image = dynamic_cast<mitk::Image*>(filteredNodes->front()->GetData());
    if (nullptr != image)
    {
      bounds= image->GetTimeGeometry();
    }
  }

which results in "reinit", "global reinit" and "initialization after loading" being the same for a single image. Of course this is not working for multiple loaded images since only the geometry of a single node is taken into account. I wanted to show that the ComputeBoundingGeometry3D does something different than simply returning the time geometry for a single loaded image (the GlobalReinitAction uses mitk::RenderWindowManager::InitializeViewsByBoundingObjects which in turn calls mitk::DataStorage::ComputeBoundingGeometry3D).

So I dug a little bit deeper using brain.nrrd as test data:

ReinitGlobalReinit
calling RenderWindowManager::InitializeViews(RenderingManager::GetInstance(), image->GetTimeGeometry());calling RenderWindowManager::InitializeViewsByBoundingObjects(RenderingManager::GetInstance(), dataStorage);
uses image->GetTimeGeometry()uses dataStorage->ComputeBoundingGeometry3D(filteredNodes, "visible");
geometry is of type ProportionalTimeGeometry::Pointergeometry is of type ArbitraryTimeGeometry::Pointer
geometry has the following 8 bounding box points: ([89.999999985533307, -49.900372828480933, -115.07371257268798]; [90.000002618471896, -112.14799369826491, 55.950360867432238]; [89.999999985533307, 154.95263821517955, -40.513375486902760]; [90.000002618471896, 92.705017345395575, 130.51069795321743]; [-92.000000014466565, -49.900370615288132, -115.07371865338078]; [-91.999997381527976, -112.14799148507211, 55.950354786739439]; [-92.000000014466565, 154.95264042837238, -40.513381567595559]; [-91.999997381527976, 92.705019558588376, 130.51069187252463])geometry has the following 8 bounding box points: ([-92.000000014466565, -112.14799369826491, -115.07371865338078]; [-92.000000014466565, -112.14799369826491, 130.51069795321743]; [-92.000000014466565, 154.95264042837235, -115.07371865338078]; [-92.000000014466565, 154.95264042837235, 130.51069795321743]; [90.000002618471882, -112.14799369826491, -115.07371865338078]; [90.000002618471882, -112.14799369826491, 130.51069795321743]; [90.000002618471882, 154.95264042837235, -115.07371865338078]; [90.000002618471882, 154.95264042837235, 130.51069795321743])
resulting in the following bounds: (-92.000000014466565, 90.000002618471896, -112.14799369826491, 154.95264042837238, -115.07371865338078, 130.51069795321743), which is the minimum / maximum extend for each axis / dimensionresulting in the following bounds: (-92.000000014466565, 90.000002618471882, -112.14799369826491, 154.95264042837235, -115.07371865338078, 130.51069795321743), which is the minimum / maximum extend for each axis / dimension
resulting in the navigation controller being at position Axial: 45, Sagittal: 45, Coronal: 54resulting in the navigation controller being at position Axial: 47, Sagittal: 45, Coronal: 111

Since the bounding box is the same but the geometry differs, I tried to find out where the geometry is used during initialization:
Inside RenderWindowManager::InternalViewInitialization there is a call to nc->SetInputWorldTimeGeometry(geometry);, which uses the given geometry to define the input geometry for the slice navigation controller.
This leads to a different behaviour for both actions / function calls.

We need to look deeper into the segmentation plugin to find out if it is possible to create and draw a segmentation without a correctly initialized reference image.

I started looking into this and eventually I stumbled upon this: T16132: Allow segmentation with several images
I debugged a little and there are some questions that came up that I would like to discuss:

  • the InternalViewInitialization, which is called by InitializeView does a couple of things:
    1. re-setting the view direction of the slice navigation controller to its default in case this has been changed - typically not the case)
    2. setting the input world geometry of the slice navigation controller to the new geometry - to define the bounds of the geometry, slice number, extends etc.
    3. re-setting the camera controller of each base renderer to make the view fit the new geometry
    4. setting the slice position to the center of an image

Point 1. to 3. are necessary for a correct update of the slice navigation controller. Point 4. is actually not required to update the geometry information for the slice navigation controller. If I remove this line, I end up with the cross-hair being at position "0". That is because SliceNavigationController::Update contains m_Slice->SetPos(0);. So removing also this line does not change the last crosshair position and therefore the render windows are not being reset in terms of "clicked position". The Zoom however will still be reset.

The problem with this is that this holds for all view initializations, so the crosshair is also not reset for a (global) reinit. So I suggest to introduce a variable that defines if the crosshair should be reset.

Point 3. actually triggers some functions that will result in a correct rendering of the image pixels, but it also triggers many functions related to the slice navigation position etc.
There are many functions that are triggered (e.g. updating the slice navigation controller, updating the stepper etc.) that all in turn trigger a view initialization and rendering request. The whole thing is completely entangled with itself and hard to understand. There are even multiple occurrences of re-computing the bounding geometry for different purposes which probably takes a lot of time. Disentangling the view initialization computation would probably speed up the rendering / view interaction but I guess we need to discuss this first.

We discussed this and two important points were made:

  • The difference between RenderWindowManager::InitializeViews(RenderingManager::GetInstance(), image->GetTimeGeometry()); and RenderWindowManager::InitializeViewsByBoundingObjects(RenderingManager::GetInstance(), dataStorage); is reasonable and important - even for a single data node (e.g. brain.nrrd, where the image itself is somehow rotated / tilted).
  • We definitely want to reset the geometry automatically such that the current view geometry of the renderer matches with the segmentation geometry. That way the user can immediately start drawing. We don't want the user to have to care for the correct geometry aligning manually.

As a result we don't want to remove the mitk::RenderingManager::GetInstance()->InitializeViews(referenceImage->GetTimeGeometry(), mitk::RenderingManager::REQUEST_UPDATE_ALL, true); function call from the QmitkSegmentationView::CreateNewSegmentation but rather try to reset the crosshair position, after it has been set to zero or center inside InitializeViews.
The problem is that we cannot simply store the slice / navigation controller slider position for each base renderer and set it to where it was before since the geometries before and after view initialization may differ so there is some point transformation involved.

I first looked into RenderingManager::InternalViewInitialization so see if we can somehow replace nc->GetSlice()->SetPos(nc->GetSlice()->GetSteps() / 2); with a computation of the correct slice position. However, inside this function we only have access to a single base renderer / slice position which does not allow us to do a point transformation in 3D.
So I utilized two functions that are already existing and allow to compute / transform a 3D Point from Index to world and back, using renderer specific geometries:
QmitkAbstractMultiWidget::GetSelectedPosition --> QmitkStdMultiWidget::GetSelectedPosition
and
QmitkAbstractMultiWidget::SetSelectedPosition --> QmitkStdMultiWidget::SetSelectedPosition

These functions retrieve the three plane geometries, find the intersecting 3D point in world coordinates and allow to convert a 3D point in world coordinates back into index coordinates for each base renderer by finding the closest slice.
However, in order to access these functions we need the current render window part which is only accessible within the QmitkSegmentationView and not within the RenderingManager (render window part is inside a plugin).
So I came up with the solution in the differential for you to look at @floca && @kislinsk. I'm open for discussions and for a different approach if we can somehow find a way to perform the whole point transformation inside the rendering manager.

However, this is a very simple, working and perfectly fine solution as far as I can see. But it is a segmentation view specific solution because it is implemented inside the segmentation plugin.
Also, this solution still does not preserve the zoom but that's the next step.

kalali added a revision: Restricted Differential Revision.Jul 15 2021, 10:38 PM

Also, this solution still does not preserve the zoom but that's the next step.

D523 now also provides a solution for NOT resetting the camera / fitting the view.

Has been solved for the segmentation plugin and the multilabel segmentation plugin.
A general reformatting of the resetting-functionality will happen in T27613: Improve reinit behavior.