Page MenuHomePhabricator

Setting "selected" property should highlight item in data manager
Closed, WontfixPublic


I would like to select an image node programmatically. I can set the 'selected' property of the node, but the selection does not change in the data manager.

This feature would be useful for our segmentation plugin (but for the MITK segmentation plugin as well, I think). When the user creates a new segmentation, the selection could be placed on the new image in the data manager.

This could be handled in QmitkDataStorageTreeModel that could assign a modification listener to the 'selected' property of a node when a new node is added, and remove it when the node is removed of when the data storage is changed. (If there is a scenario for having multiple data storage.)

What do you think? Would you accept pull request for this?

Event Timeline

kislinsk triaged this task as Wishlist priority.Aug 10 2016, 3:18 AM
kislinsk added a subscriber: kislinsk.

AFAIK, we had something like this behaviour in our Segmentation plugin some time ago. It somehow tried to stay in sync with the Data Manager. However, rather often we ran into issues as selection often triggers other functionality and most non-technical users clearly voted against it, which is why we're solely using the QmitkDataStorageComboBox approach now.

In consequence, I would personally tend to vote against this feature in MITK as it is today.

We follow a different way in our plugins. They show which images you are working on, but by labels, not combo boxes. We have not seen problem with this approach, but this does not mean that you need to change the MITK Segmentation plugin if you are happy with it.

I sent a PR:

It ensures that the highlighting of items in the data manager is consistent with the 'selected' property values.

It will not trigger selection change events in other functionalities / views, because the selection model signals are blocked for the time when the selection is changed in the tree view. I am not sure if it is good, though. I did it to avoid recursion, but maybe other views should still be notified.

I also tried to set the data manager selection through this function:

QmitkAbstractView::SetDataManagerSelection(const berry::ISelection::ConstPointer& selection,
                           QItemSelectionModel::SelectionFlags flags = QItemSelectionModel::ClearAndSelect) const;

but I could not get it work because it would only work if the 'selection' argument is a berry::QtItemSelection, otherwise it does nothing. I have not managed to make a QtItemSelection, though.

Actually I am currently working on a different data manager that allows to specifically set data nodes to a render window, change the view direction etc..
This new data manager offers a selection provider, so that it is possible to send data node selections to all views (QmitkAbstractView).

We are currently evaluating different possibilities of handling more than one data node selection provider and currently it seems to me that your approach is working with another selection provider, as your PR touches only the "selected" property.
We were also thinking about utilizing the "selected"-property in order to synchronize different data selection views across the application.

However, your solution is currently tailored to the data storage tree model and I'd like to discuss, if it is possible to move the PropertyChanged-functionality out of the QmitkDataStorageTreeModel. With the given solution, the PropertyChangedCommand has to be included in a second data manager as well!?

Furthermore, a view subclassing the QmitkAbstractView could also implement the NodeChange-function and set the view model selection according to the given data node selection. This function is called when a data node property has been changed. Unfortunately in this case I don't see any mechanism to find out which property has been changed so it might "reset" a selection, even if another property has been changed (e.g. visibility). Additionally, the block-mechanism you introduced have to be used, though. This has to be done anyway, if we want to combine a selection model in combination with the "selected"-property.

That sounds cool! Is there a prototype already to try out?

It is not related to the original issue, but one thing that I am missing from the data manager is that it can only show the global visibility. It would be nice to provide a way (even if through API) to toggle if the check boxes should control the global or the local visibility (in the active render window of the active render window part if any).

We have an editor that can contain multiple viewers, and I have to map the local visibilities of the selected viewer to the global visibilities and back. Every time when the user selects another viewer, I should set the global visibilities to the local ones in that viewer. Or if the user changes the global visibility of a node by a check box then I have to change its local visibility in the selected viewer. It's been implemented already and it kind of works OK, just I thought to mention in case you are looking for new features or ideas. :)

I really ... khm ... dislike the NodeChange function as it is called too often and you cannot control what events you want to get notified of. The worst is if there is a RequestUpdate in the NodeChange and the scene is re-rendered every time when you select an image. That can slow down the app a lot.

Of course, the PropertyChangeCommand class can be factored out and reused in other classes. In our app we use a more general class called DataNodePropertyListener. You can instantiate a property listener by specifying a data storage and a property name, and you can assign your callback function to this member of DataNodePropertyListener:

mitk::Message2<mitk::DataNode*, const mitk::BaseRenderer*> NodePropertyChanged;

We have it in our core module and found it very useful.

kislinsk claimed this task.
kislinsk added a project: Auto-closed.

Hi there! 🙂

This task was auto-closed according to our Task Lifecycle Management.
Please follow this link for more information and don't forget that you are encouraged to reasonable re-open tasks to revive them. 🚑

Best wishes,
The MITK devs

kislinsk removed kislinsk as the assignee of this task.May 26 2020, 12:05 PM
kislinsk removed a subscriber: kislinsk.