Page MenuHomePhabricator

Remove QmitkStdMultiWidget dependencies from Workbench Views
Closed, ResolvedPublic

Description

Almost all MITK Views have a direct dependency to the QmitkStdMultiWidget(Editor) through the base class QmitkFunctionality.

The API used by the MITK Views should be extracted into interfaces independent of a specific widget (editor) implementation. This will allow for a better customisation of the application framework.

Please comment on the initial draft about how to refactor the current system:

http://www.mitk.org/wiki/Development/MultiWidgetAbstraction

All names (interfaces, methods, etc.) are up for discussion. I probably didn't consider a couple of things, so please read critically!

Event Timeline

Screenshot of NiftyView (CMIC application)

NiftyView.png (1×1 px, 519 KB)

(In reply to comment #0)

Hi Sascha,

A few comments, which may or may not be useful.
:-)

  1. Overall, looks about right, to my level of understanding.
  2. I have developed an editor, which contains a main viewer widget QmitkMIDASMultiViewWidget, containing a grid layout (up to 5x5 grid sections), each containing a QmitkMIDASSingleViewWidget which wraps a QmitkMIDASStdMultiWidget which is a subclass of QmitkStdMultiWidget. This should enable me to have multiple ortho-views, and things like linked cross hairs, even when images have different axes orientation and origins. This is useful for comparing registrations. So, my main concern is down to the main "effect" I see of the current implementation. The main annoyance to me, is that the application flips back to the QmitkStdMultiWidgetEditor instead of my editor. It's not yet clear to me if the proposed solution will completely fix this. The main issue of QmitkFunctionality directly referencing QmitkStdMultiWidget will disappear, and that will help immensely. However, both editors (QmitkStdMultiWidgetEditor, and my editor) will both be "interested" in the same data types. So how should this be handled? When a new image is loaded, I don't want the viewer to flip between editors, as they will both be valid editors for that kind of data. (see screenshot).
  3. I like the migration plan. However, should point 4 not contain the word "gradually". The danger for me is that only some of the views get migrated, some get forgotten, and we haven't really solved the problem.
  4. I like the remarks...... out of interest, for my own personal education, can someone point me towards some unit testing for plugins? Or do we just unit test Modules / libraries?
  5. I assume the part about decorators means that individual decoratioins can be turned on/off. i.e. what if I want corner annotations, but not coloured borders? If thats the case, will QmitkStdMultiWidget need modifying.

Thanks as always.

Matt

Hi,

thanks for the feedback.

  1. I have developed an editor, which contains a main viewer widget

QmitkMIDASMultiViewWidget, containing a grid layout (up to 5x5 grid sections),
each containing a QmitkMIDASSingleViewWidget which wraps a
QmitkMIDASStdMultiWidget which is a subclass of QmitkStdMultiWidget. This
should enable me to have multiple ortho-views, and things like linked cross
hairs, even when images have different axes orientation and origins. This is
useful for comparing registrations. So, my main concern is down to the main
"effect" I see of the current implementation. The main annoyance to me, is
that the application flips back to the QmitkStdMultiWidgetEditor instead of my
editor. It's not yet clear to me if the proposed solution will completely fix
this. The main issue of QmitkFunctionality directly referencing
QmitkStdMultiWidget will disappear, and that will help immensely. However, both
editors (QmitkStdMultiWidgetEditor, and my editor) will both be "interested" in
the same data types. So how should this be handled? When a new image is
loaded, I don't want the viewer to flip between editors, as they will both be
valid editors for that kind of data. (see screenshot).

Yes, that resolution of this kind of problem should be stated more clearly. It is definitely something to solve once and for all.

The idea is that you could either switch off the org.mitk.gui.qt.stdmultiwidgeteditor plug-in (so it will not register its editor) or that you use the BlueBerry workbench API to set the "default editor" for certain datatypes. Views which will get or add data should work with the data storage associated with the current editor (the "default editor" if none is opened yet). They should usually not activate other editors by themselves. I will state that more clearly on the Wiki.

  1. I like the migration plan. However, should point 4 not contain the word

"gradually". The danger for me is that only some of the views get migrated,
some get forgotten, and we haven't really solved the problem.

I would migrate the most used ones but since we have quite a lot of them (also not open-source) this must be done by the whole group...

  1. I like the remarks...... out of interest, for my own personal education, can

someone point me towards some unit testing for plugins? Or do we just unit
test Modules / libraries?

Unit tests for plug-ins are definitely a weak point in MITK. They exist, but usually the plug-ins need a GUI and cross-platform GUI-Testing is not fun...

In MITK/BlueBerry/Testing you can find plug-ins which test different parts of the BlueBerry API (which itself is provided by other plug-ins). If your plug-in to test exposes some API, you could look into that. If you want real GUI-Testing, there is no final solution yet.

  1. I assume the part about decorators means that individual decoratioins can be

turned on/off. i.e. what if I want corner annotations, but not coloured
borders? If thats the case, will QmitkStdMultiWidget need modifying.

Yes, maybe the QmitkStdMultiWidget needs to modified for this. I am not really sure yet if the "decorations" API in the interfaces will be really useful. It was a shot at a generalised and easily "extendible" interface.

Thanks,
Sascha

Added Daniel to the CC list.

Do you guys think that a

LevelWindowManager* IRenderWindowPart::GetLevelWindowManager()

method would be useful?

(In reply to comment #4)

Do you guys think that a

LevelWindowManager* IRenderWindowPart::GetLevelWindowManager()

method would be useful?

I am not sure that it would necessarily go on the IRenderWindowPart. Has it been decided whether there will be one global LevelWindowManager, or one per DataStorage? In either of these cases, the LevelWindow property is attached to each image node, and hence can be retrieved from the DataStorage, so it might be simpler to query your default/current/ DataStorage to retrieve it.

In addition, our GUI currently has

  1. the MITK QmitkStdMultiWidget(and editor), and also the MITK imagenavigator plugin.
  2. our editor and our navigator plugin.

This led to a fair deal of confusion among some users. Admittedly, part of the problem was the fact that the GUI incorrectly kept switching back to the MITK editor, when the user was trying to use our editor, but essentially the problem was that it was not clear which set of controls went with which editor. So, these suggested changes should make it easier to keep control of the presence of editors.

I currently think, (but would like to hear peoples opinion) that if a plugin such as an editor is fundamentally linked to a set of controls, they should be in the same plugin, and in the same Qt panel, i.e. physically connected, or the controls should be completely optional. So, the MITK QmitkStdMultiEditor can be used, and navigated without the imagenavigator plugin, so thats OK.

But as soon as I added another editor and another set of controls, which were different, so the user should understand this, they still got confused.

So, I have refactored my editor and my controls to share the same plugin view, so they are explicitly coupled together, so the user does not get lost.

Anyway, after all this rambling, finally a question:

If the QmitkStdMultiWidget editor is moved to its own plugin, and I can turn it off/on, and I turn of the imagenavigator, will the ImageNavigator icon on the main toolbar disappear aswell? or is that icon hard-coded? If it's hard coded, then a bit of tidying will be necessary as for any user that turns off the editor, and the imagenavigator, and goes their own way, they won't want an additional extra icon.

(In reply to comment #5)

(In reply to comment #4)

Do you guys think that a

LevelWindowManager* IRenderWindowPart::GetLevelWindowManager()

method would be useful?

I am not sure that it would necessarily go on the IRenderWindowPart. Has it
been decided whether there will be one global LevelWindowManager, or one per
DataStorage? In either of these cases, the LevelWindow property is attached to
each image node, and hence can be retrieved from the DataStorage, so it might
be simpler to query your default/current/ DataStorage to retrieve it.

That hasn't been decided yet. I am not sure if a global LevelWindowManager makes sense at all. Further, I don't think that it should be one per DataStorage. My initial reasoning was that it should be possible to have multiple "render editors" - possibly rendering the same DataStorage - which use their own LevelWindowManager instance. This is also roughly how it works now with QmitkLevelWindowWidget used in the QmitkStdMultiWidget. It instantiates it's own LevelWindowManager object.

If another View would now want to modify *the* level window, it should get the currently acitve editor, see if it implements IRenderWindowPart, get the LevelWindowManager and be able to consistently modify it.

Does that make sense?

I currently think, (but would like to hear peoples opinion) that if a plugin
such as an editor is fundamentally linked to a set of controls, they should be
in the same plugin, and in the same Qt panel, i.e. physically connected, or the
controls should be completely optional. So, the MITK QmitkStdMultiEditor can
be used, and navigated without the imagenavigator plugin, so thats OK.

But as soon as I added another editor and another set of controls, which were
different, so the user should understand this, they still got confused.

I agree with you here. If additional controls have a dependency to a specific editor, they should generally go into the same plug-in. If they are essential for the editor to work, they should probably be integrated into the editor GUI itself.

If the QmitkStdMultiWidget editor is moved to its own plugin, and I can turn it
off/on, and I turn of the imagenavigator, will the ImageNavigator icon on the
main toolbar disappear aswell? or is that icon hard-coded? If it's hard coded,
then a bit of tidying will be necessary as for any user that turns off the
editor, and the imagenavigator, and goes their own way, they won't want an
additional extra icon.

The toolbar button for the ImageNaviagtor View is already shown only if the View is available. So no additional work necessary :-)

(In reply to comment #7)

That hasn't been decided yet. I am not sure if a global LevelWindowManager
makes sense at all. Further, I don't think that it should be one per
DataStorage. My initial reasoning was that it should be possible to have
multiple "render editors" - possibly rendering the same DataStorage - which use
their own LevelWindowManager instance. This is also roughly how it works now
with QmitkLevelWindowWidget used in the QmitkStdMultiWidget. It instantiates
it's own LevelWindowManager object.

OK, understand that we might want multiple managers belonging to multiple widgets, but would the LevelWindow property still be stored on the DataNode? If that is the case, then you would still expect the same WindowLevel to be visible in all editors? Or were you hoping for something different? I like it being consistent across every view, but I can also imagine use cases where you want different WindowLevel in different views.

If another View would now want to modify *the* level window, it should get the
currently acitve editor, see if it implements IRenderWindowPart, get the
LevelWindowManager and be able to consistently modify it.

Does that make sense?

Sure.

OK, understand that we might want multiple managers belonging to multiple
widgets, but would the LevelWindow property still be stored on the DataNode?
If that is the case, then you would still expect the same WindowLevel to be
visible in all editors? Or were you hoping for something different? I like it
being consistent across every view, but I can also imagine use cases where you
want different WindowLevel in different views.

I like it consistent too. The LevelWindow property would still be just a property of the DataNode. All editors rendering the same datanode (by using either the same DataStorage instance, or the same datanode which happens to be contained in multiple DataStorage instances) would display the same Level/Window for the image contained in the DataNode.

If someone would want to have different level window properties for different render windows, they should use render window specific properties.

That is how I see it...

[be6457]: Merge branch 'bug-10963-decouple-views-from-stdmultiwidget'

Merged commits:

2012-02-23 15:14:36 Sascha Zelzer [0e8d90]
Removed the "Plugin" word from the module name.


2012-02-23 15:17:35 Sascha Zelzer [f49e3a]
Added missing doxygen file.


2012-02-23 09:20:02 Sascha Zelzer [a26e2a]
Make the measurement plug-in QmitkStdMultiWidget independent.


2012-02-23 09:31:36 Sascha Zelzer [6477ac]
Support multiple editors registered for the "mitk" extension.


2012-02-23 09:28:32 Sascha Zelzer [000386]
Make the Data Manager QmitkStdMultiWidget independent.

Additionally, a QList is used for passing mitk::DataNode::Pointer objects
instead of a std::vector.


2012-02-23 08:47:11 Sascha Zelzer [1ae19d]
Make the image navigator independent of the stdmultiwidget.


2012-02-23 08:38:44 Sascha Zelzer [0488be]
Refactored org.mitk.gui.qt.common plug-in to have no stdmultiwidget dependencies.

A new plug-in org.mitk.gui.qt.common.legacy was created to support the old style.
The org.mitk.gui.qt.application plug-in is now a general purpose plug-in
containing useful classes for application developers.


2012-02-22 22:22:58 Sascha Zelzer [bebcde]
Added mitk::IDataNodeReader interface using micro services.


2012-02-22 17:55:57 Sascha Zelzer [cc4aa2]
Added drag and drop support in the BlueBerry workbench.


2012-02-22 17:50:10 Sascha Zelzer [0c47e2]
Enhanced state machine specific logging macros.


2012-02-22 17:48:43 Sascha Zelzer [7ce09b]
Style fixes.


2012-02-22 17:41:32 Sascha Zelzer [950ae6]
Fixed several warnings.


2012-02-22 17:00:58 Sascha Zelzer [110177]
Fixed license information.

[017352]: Merge branch 'bug-10963-decouple-views-from-stdmultiwidget'

Merged commits:

2012-02-23 20:44:46 Sascha Zelzer [1f9cc3]
Updated PluginGenerator changelog and version number.

[8372be]: Merge branch 'bug-10963-decouple-views-from-stdmultiwidget'

Merged commits:

2012-02-24 10:45:38 Sascha Zelzer [353c4c]
Added minimum required hash info.


2012-02-24 10:44:01 Sascha Zelzer [b86bf2]
Support MITK compatibility checks based on rolling internal version numbers.


2012-02-23 23:09:36 Sascha Zelzer [361fc8]
Added method documentation.

[88f4ad]: Merge branch 'bug-10963-decouple-views-from-stdmultiwidget'

Merged commits:

2012-02-24 10:55:00 Sascha Zelzer [805734]
Check if function mitkFunctionCheckMitkCompatibility is available.

[0acbc8]: Merge branch 'bug-10963-decouple-views-from-stdmultiwidget'

Merged commits:

2012-02-26 14:19:39 Sascha Zelzer [8fa4c3]
Improved documentation.

The infrastructure is in place. Please open specific tickets for bugs or View migrations.