Page MenuHomePhabricator
Create Task
Maniphest T27524

Should RenderingManager be used without singleton?
Closed, ResolvedPublic
Actions

Description

Currently we have a lot of calls to RenderingManager::GetInstance() to receive the rendering manager singleton and work with it. This is typically done to perform some actions on all render windows (which are handled by the global rendering manager singleton).

However, it is possible to register render windows with an individual rendering manager, leading to something like
mitk::BaseRenderer::GetInstance(renderWindow)->GetRenderingManager()
or
mitk::BaseRenderer::GetInstance(mitk::BaseRenderer::GetRenderWindowByName("stdmulti.widget0"))

The second option is not a good option in my opinion since it explicitly requires a specific, magic string. The first option only works if you already have a render window at hand.

I think we never faced the problem of needing multiple rendering managers. Also, this was done to only use the rendering manager singleton: rMITK5e0e87c36865: Handle RenderingManager like the singleton it is.

Reason for this topic to come up was the latest PR on GitHub: https://github.com/MITK/MITK/pull/248
On the user list the user struggled with the difference between the two functions accessing the "global" or "local" rendering manager.

Related Objects

Event Timeline

kalali triaged this task as Normal priority.Jul 3 2020, 10:37 AM
kalali created this task.
kalali added a subscriber: • thomass.Jul 3 2020, 1:00 PM
kalali updated the task description. (Show Details)Aug 14 2020, 5:58 PM
kislinsk added a subscriber: kislinsk.Aug 14 2020, 9:03 PM

I cannot see any declaration of GetRenderingManager() in BaseRenderer (even before rMITK5e0e87c3)? In the second example I even cannot see anything related to RenderingManager? :-)

Anyway, to answer the title question: no. We never had or missed the possibility to actually use multiple rendering managers since it was never fully implemented to work this way. This eventually lead to all the code removal in rMITK5e0e87c3.

kalali added a comment.Aug 17 2020, 11:38 AM
In T27524#208371, @kislinsk wrote:

I cannot see any declaration of GetRenderingManager() in BaseRenderer

That's because you removed it. So I'm currently realizing that the topic was silently (unintentionally?) closed by your commit rMITK5e0e87c3 (which was done before this task came up). That leads me to the question if the task "Remove all possibilities to use multiple rendering managers" is done. If it is this topic can be closed.

In T27524#208371, @kislinsk wrote:

In the second example I even cannot see anything related to RenderingManager? :-)

There was a missing call to GetRenderingManager() as in the example above.

floca moved this task from Backlog to MITK Meeting on the Request for Discussion board.Sep 1 2020, 5:48 PM
kislinsk closed this task as Resolved.Oct 9 2020, 11:35 AM
kislinsk claimed this task.
MITK · MIC · IMSY · DKFZ