Page MenuHomePhabricator | MITK

UpdateStatusBar duplicates business logic
Open, WishlistPublic

Description

Currently the UpdateStatusBar method in QmitkImageNavigatorView and in mitkDisplayInteractor duplicate a lot of business logic. The part of the function that extracts the pixel value from the image and updates the statusbar is the same. The redundant part should be extracted and be put in one function that can be called by both (QmitkImageNavigatorView::UpdateStatusBar() and mitkDisplayInteractor::UpdateStatusBar().

Event Timeline

floca created this task.Jul 21 2018, 7:40 PM
floca edited projects, added MITK (2018-04); removed MITK.Jul 21 2018, 7:43 PM

This should be fixed in conjunction with T25042: How to handle update informations in heterogeneous time geometries and out of bound situations and T24767: Workbench crashes when loading two 3D+t images with different number of time steps, because an unfixed UpdateStatusBar in the DisplayInteractor could also cause crashes like in T24767.

kalali added a subscriber: kalali.Jul 24 2018, 12:06 PM

The GetTopLayerNode has been moved to the data storage as FindTopmostVisibleNode.
Would it be reasonable to move the UpdateStatusBar functionality to the data storage as well, as it needs to extract image information from the topmost visible node?
Or is there some other place for this kind of image geometry processing?

floca added a comment.Oct 23 2018, 4:26 PM

Would it be reasonable to move the UpdateStatusBar functionality to the data storage as well

No, I don't think so. It is not job of the DataStorage to update the status bar.

Or is there some other place for this kind of image geometry processing?

UpdateStatusBar is not just image geometry processing.
Why not put it in mitkStatusBar.h? E.g. as global public method or put it into a mitkStatusBarHelper
I think both ways are more appropriate then putting it into the DataStorage or its files.

We do not need to move the whole functionality to the data storage, just the geometry processing part. The rest can be put somewhere else, but https://phabricator.mitk.org/D14 said, It was decided that the status bar should not be responsible for extracting information from an image.

So basically I was thinking about a function inside the data storage that would do the image processing and the compute / return the needed parameters to call DisplayImageInfo.

We do not need to move the whole functionality to the data storage, just the geometry processing part.

What do you exactly mean with geometry processing part? Does "to the storage" mean, into the class or into the file?

The rest can be put somewhere else, but https://phabricator.mitk.org/D14 said, It was decided that the status bar should not be responsible for extracting information from an image.

Please take care. I never said it should be part of the class StatusBar. I proposed that it should be a global public method in the header file mitkStatusBar.h. Thats a big difference in terms of dependencies etc.

So basically I was thinking about a function inside the data storage that would do the image processing and the compute / return the needed parameters to call DisplayImageInfo.

If you plan any rework, please keep T25043: Remove pixel informations from the statusbar and put it into views in mind. As discussed there the pixel information should not be in the status bar at all and is very use case, application specific.

kislinsk triaged this task as Wishlist priority.Nov 7 2018, 8:43 AM
kislinsk edited projects, added MITK; removed MITK (2018-04).