Hello Amir,

As you have suggested, here is the PR: https://github.com/MITK/MITK/pull/248
.

I've made some changes in those files I sent you previously, also removed
some misleading comments I placed during the tests I did.

I created some minimal test applications, if you think this should be added
I'd appreciate guidance on how to add it to the project.

Cheers,

Carlo.


On Tue, Jun 30, 2020 at 5:39 AM Kalali, Amir <a.kal...@dkfz-heidelberg.de>
wrote:

> Hello Carlo,
>
>
>
> Thank you for pointing that out. I took a look at it and I think you’re
> right, there seems to be a mixture of the individual rendering manager
> (which might or might not be the same for every render window widget) and
> the global, singleton rendering manager.
>
>
>
> The QmitkRenderWindowWidget was introduced while refactoring the rendering
> widgets to allow for a more flexible multi widget editor. Such a flexible
> multi widget editor should be able to display different image in parallel
> (with different geometry / view direction etc.) and thus it makes sense to
> also allow different data storages and rendering managers.
>
>
>
> I’m happy to see your changes contributed, just fork
> https://github.com/MITK/MITK and create a pull request. I looked at your
> changes and I would like to comment on it, which is easy having a PR.
>
>
>
> Best,
>
>
>
> Amir
>
>
>
> *From:* Carlo [mailto:cv.forn...@gmail.com]
> *Sent:* Thursday, 4 June, 2020 23:16
> *To:* mitk-users@lists.sourceforge.net
> *Subject:* [mitk-users] QmitkRenderWindowWidget design flaw: Setting
> datastorage globally.
>
>
>
> Hello,
>
> It's been a month since I started to develop an application using MITK as
> a toolkit. I need to render two independent 3D windows, to achieve this I
> choose the QmitkRenderWindowWidget, but inside QmitkRenderWindowWidget.cpp
> we have the method:
>
> (1) void QmitkRenderWindowWidget::InitializeGUI();
>
>
>
> This is a private method called by the constructor and inside it we have
> this line of code:
>
>
>
> (2) mitk::RenderingManager::GetInstance()->SetDataStorage(m_DataStorage);
>
>
>
> This code sets m_DataStorage's content globally, and at the moment we declare 
> QmitkRenderWindowWidget, all other rendering windows are modified.
>
>
>
> The behavior seems to be a design flaw, because QmitkRenderWindowWidget has a 
> method:
>
>
>
> (3) void QmitkRenderWindowWidget::SetDataStorage(mitk::DataStorage* 
> dataStorage);
>
>
>
> I can't tell for sure, but this method seems to set the datastorage only for 
> the current QmitkRenderWindowWidget. So there is no need for (2).
>
>
>
> Here is the next thing that took my attention:
>
>
>
> QmitkStdMultiWidget inherits QmitkAbstractMultiWidget, the last one has a 
> method:
>
>
>
> (4) void QmitkStdMultiWidget::SetDataStorage(mitk::DataStorage* dataStorage);
>
>
>
> QmitkStdMultiWidget has four rendering widgets, when calling method (4), 
> there are four callings for method (3), one for each rendering widget,
>
> but their constructors had already called four times the method (2), and this 
> last method has messed up every other rendering window in the current 
> application.
>
>
>
> For these reasons, I believe method (2) should be removed as it is there 
> probably by mistake. Also, the algorithms for widgets initialization doesn't
>
> appear to consider method (2) existence.
>
>
>
> I'd welcome any thoughts on this.
>
>
_______________________________________________
mitk-users mailing list
mitk-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mitk-users

Reply via email to