Page MenuHomePhabricator

Inconsistent view initialization in rendering manager
Open, HighPublic

Description

Looking at the mitk::RenderingManager there are several functions that are basically doing the same. They only differ in their given parameters and therefore need to process the given data slightly differently.
However, there is sometimes a different logic although they should do the same and I wonder why that is, e.g.:

https://phabricator.mitk.org/source/mitk/browse/develop/Modules/Core/src/Controllers/mitkRenderingManager.cpp$447 uses initializeGlobalTimeSNC whereas https://phabricator.mitk.org/source/mitk/browse/develop/Modules/Core/src/Controllers/mitkRenderingManager.cpp$374 does not use this variable.

On the other hand,
https://phabricator.mitk.org/source/mitk/browse/develop/Modules/Core/src/Controllers/mitkRenderingManager.cpp$331 extends the bounding box to have "an extent bigger than zero in any direction" whereas
the function in https://phabricator.mitk.org/source/mitk/browse/develop/Modules/Core/src/Controllers/mitkRenderingManager.cpp$426 does no such thing.

Also, only in https://phabricator.mitk.org/source/mitk/browse/develop/Modules/Core/src/Controllers/mitkRenderingManager.cpp$385 mitk::RenderingManagerViewsInitializedEvent() is called, although a a rendering request / view initialization happened in other functions as well.

There is also an old comment in https://phabricator.mitk.org/source/mitk/browse/develop/Modules/Core/src/Controllers/mitkRenderingManager.cpp$297 that says "Remove old function, so only this one is working." so maybe that's a hint to remove the later function?

I found this while working on D517 to separate the rendering from the view initialization.

If we can clarify this we might be able to remove many lines of code and simplify the code that is concerned with view initialization, which would bring us a step forward in finding a good solution for related tasks, such as T28490, T27613 and T26496

Event Timeline

kalali triaged this task as High priority.Jul 9 2021, 4:14 PM
kalali created this task.

The later function bool RenderingManager::InitializeView(vtkRenderWindow *renderWindow, const TimeGeometry *geometry, bool initializeGlobalTimeSNC), using the initializeGlobalTimeSNC is rarely used at all, only in

  • QmitkToFUtilView::UseToFVisibilitySettings
  • QmitkDataNodeOpenInAction::OnActionTriggered
  • ReinitAction::Run
  • LabelSetJumpToAction::Run

I tested this function using initializeGlobalTimeSNC = false, so that m_TimeNavigationController->SetInputWorldTimeGeometry(geometry); is not called. Using a 4D image the time slicer is not correctly initialized and cannot be used / changed. I wonder in which scenario one would disable setting the correct input world time geometry!? Obviously this function was not correctly used (also by myself) and the default value for this parameter was simply accepted, although it is set to false.

Simply removing this function does not work since it's the only function that allows to pass a specific single render window. And it will be used more often when using the mxn-multi widget.
However, it is not complicated to refactor these functions to remove this parameter and to simplify the functions to call each-other to allow passing a single render window.