Page MenuHomePhabricator

Improve reinit behavior
Closed, ResolvedPublic

Description

Reinit and global reinit currently also reset the views/crosshair and the time step. In most use cases this is rather annoying, for example when creating a segmentation and the current time step and cross hair position is reset. However, one may want to reset everything to see each and every data node in the scene. I propose to do two things:

  1. Do not reset anything unnecessary when doing a reinit
    • Keep cross hair position if it fits, adapt it to the new scene bounds to stay virtually at the same position, and only reset it if both soft approaches do not work
    • Basically same for time step
    • Do not reset the 3d camera angle/orientation
  2. Introduce a true "Reset scene/view" operation which basically equals the current reinit operations.

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
Restricted Differential Revision

Event Timeline

kalali edited subscribers, added: kleina; removed: thomass.EditedJun 25 2021, 4:18 PM

Just for testing purposes: I created an additional Enum value to mitk::RenderingManager::RequestType, called NO_UPDATE. I'm using this new value inside QmitkSegmentationView::CreateNewSegmentation, for the call mitk::RenderingManager::GetInstance()->InitializeViews(referenceImage->GetTimeGeometry(), mitk::RenderingManager::NO_UPDATE, true);.

The result is that the render view is not being reset when a new segmentation node is created (using the "New..."-Button). What happens inside RenderingManager::InitializeViews:

if (((type == REQUEST_UPDATE_ALL) || ((type == REQUEST_UPDATE_2DWINDOWS) && (id == 1)) ||
     ((type == REQUEST_UPDATE_3DWINDOWS) && (id == 2))))
{
  this->InternalViewInitialization(baseRenderer, timeGeometry, boundingBoxInitialized, id);
}

lead to InternalViewInitialization NOT being called (since the new type mitk::RenderingManager::NO_UPDATE does not fulfill the if-condition).

So it seems as if there is already the possibility to separate the geometry resetting and the render view / crosshair resetting. Need to investigate to check different cases and find possible side-effects.

For a more thorough investigation of possible side-effects I will list the occurrences of similar code and see if the mentioned solution could be applied there.
I already started doing that some time ago in T26496: [mxn multi widget] Re-initializations reset all render windows and will continue there:

In T26496, @kalali wrote:

I propose to first gather different actions which lead to an update request.

kalali added a revision: Restricted Differential Revision.Jul 5 2021, 6:05 PM

My conclusion / suggestion so far:

  1. RenderingManager::RequestUpdate*-functions add render windows to a list of render windows to be re-rendered, where re-rendering means updating the graphics / pixel (OpenGL)
  2. RendereringManager::InitializeView*-functions (changed to RenderWindowManager::InitializeView* in D517) update the Time- / Slice-Navigation controller and the camera controller to a specific view ("field of view").

RequestUpdate*-functions should only be called if the graphics / pixel have been changed.
InitializeView*-functions should probably not be called automatically in most cases, as they reset the user generated "field of view".

kalali raised the priority of this task from Wishlist to Normal.Jul 7 2021, 10:29 AM
kalali edited projects, added MITK (v2021.10); removed MITK.
kalali moved this task from Backlog to Cycle on the MITK (v2021.10) board.
kalali removed a revision: Restricted Differential Revision.Mar 8 2022, 3:23 PM

My changes related to this task were mainly to clear some classes and remove uncertainty (see T28617: Inconsistent view initialization in rendering manager).
As far as I see it some changes have been made in the subtask T28490: Segmentation plugin: "new" segmentation should not run reinit! and the respective D523: This was mainly concerned with NOT resetting the view / camera using a newly introduced parameter "resetCamera".

But I think @kislinsk wants to put more "intelligence" into it and separate the re-initialization in some way. Maybe we can discuss this / write down use-cases where we still have unexpected behavior.
I also added subtasks that fit to this task and show the problem of our current reinit implementation.

kalali added a revision: Restricted Differential Revision.Jun 23 2022, 6:54 PM
kalali claimed this task.

Adding a data node reset-geometry-action was a first step and enough to tackle this issue. If there are more requirements for some "intelligence", new tasks need to be opened.