Page MenuHomePhabricator

OnSelectionChanged event is sent twice sometimes
Closed, WontfixPublic

Description

The first of the duplicate events yields the "old" nodes again. QmitkStreamlineTrackingView::OnSelectionChanged is an example were this is handled by check if the old nodes equal the new nodes.

Event Timeline

Does this happen, when you select a node in the data manager, other than the currently selected, while the data manager was not the active part?
Because each activation of a workbench part (focus to this "plugin") calls the AbstractSelectionService::SetActivePart in berryAbstractSelectionService.cpp. This in turn calls FirePostSelection which leads to a call to OnSelectionChange of the selection listener (e.g. views that subclass QmitkAbstractView.

I don't have any solution to this, just a hint to where things are coming from.

Yes, this might be the cause.

We looked at this again and want to open some points for discussion:

First of all: When a workbench part is activated, the current selection of its selection model is sent to the workbench. The idea behind this might have been that the workbench should be notified about the currently valid selection, which seems to be always the one of the selected part. There are two problems with this approach.

  • the current selection is not the newest selection, if a new node in the data manager was selected, while the data manager view part was activated simultaneously (as mentioned in the description).
    • the reason for this is that the berry mechanism to send the current selection is done before the underlying QtSelectionModel has changed. So it contains the old selection and then the QtSelectionModel changes (which leads to another selection changed event).
  • Do we really want the originally intended behavior? Currently the data manager is the only view part, who works as a selection provider. And no other view can change the selection of the data manager. So if the data manager was deactivated and then activated again, it will always have the same selection. So why do we need the "update on part activation"-mechanism? The QtSelectionModel changed event would suffice to update the selection (on user click).
    • Suppose we have another view part (let's call it DataManager2) that serves as a selection provider. A plugin view might listen to the selection service and get its selection from the DataManager1. But then, when DataManager2 gets activated, it might change the selection according to the new selection provider. So you would not be able to set the selection by DataManager2 and then inspect some nodes in DataManager1. We think this is counterintuitive.
    • To avoid this, we need to synchronize Datamanager1 and DataManager2. But then again we always have one globally valid selection and would not need the "update on part activation"-mechanism!
kalali lowered the priority of this task from Normal to Low.Aug 31 2018, 6:11 PM
kalali removed a project: Request for Discussion.

We discussed this and came to the following results:
Actually the behavior is useful and needs to be kept - also for other developments that rely on this mechanism.

However, since this is also a problem for e.g. time consuming actions that happen on both selection send events (which may be the reason why not many have encountered this problem), we plan to find a way around this while including the new selection concept (T23751).

  • the new selection concept will lead to the data manager not being the main source of data node selections so we can handle the events differently
  • the new selection concept allows to specifically listen to selection change events and thus allows to avoid the described problem

We might also think about a timer that tracks the multiple selections and only sends one selection changed event (the latest) within its time frame.

kislinsk added a project: Auto-closed.
kislinsk added a subscriber: kislinsk.

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