Page MenuHomePhabricator

Incorrect ordering of nodes when initialising data manager from data storage
Closed, ResolvedPublic


Consider lines 125-128 here:


The 'PlaceNewNodesOnTop' property is not set via the constructor, although it provides an argument for that. The property defaults to 'false'. The constructor iterates through the data storage and populates the model according to this.

Then the 'PlaceNewNodesOnTop' property is set by the view from the "Place new nodes on top" preference value. The value defaults to 'true'. The model is not updated, the change will take effect only for the nodes that will be added later.

This looks wrong. An easy fix would be to pass the preference value to the constructor and remove the SetPlaceNewNodesOnTop call.

However, there is another problem. The source nodes are stored in a map in the data storage. Therefore, the GetAll() function can return the source data nodes in arbitrary order. (They are ordered by the comparison operator of mitk::DataNode::Pointer, aka. itk::SmartPointer what is kind of arbitrary.)

It means that if you have an application that adds images in the data storage in a certain order before the data manager is created, then the order of the nodes in the data manager can be different at different runs. I introduced some command line applications for opening images, but I assume that the same must happen with MitkWorkbench as well. Try to open a set of images from the command line, and run the app a few times and you will probably see that the order of the images is different in the data manager, even if it is the same on the command line.

Event Timeline

I am wondering what would be the most straightforward way to fix the second issue. Maybe providing a comparison operator for the map based on the layer of the nodes would be enough?


kislinsk triaged this task as Normal priority.Aug 12 2016, 12:29 PM
kislinsk updated the task description. (Show Details)
kislinsk edited projects, added MITK (2016-11); removed MITK.
goch claimed this task.

Thanks for the contribution

As for the random ordering within the data storage issue. I did include a change to the way the default layer is set when loading via the command line rMITK0ed347588bfb: Use command line argument position as default layer