Page MenuHomePhabricator

LevelWindowManager::SetLevelWindow might find a wrong 'current image'.
Closed, WontfixPublic

Description

When a level window is set for a level window manager, the manager tries to find the image in the data storage that the level window belongs to, and set it as the 'current image'. A node predicate is used to search for the node with the given level window.

However, the predicate compares the property values, not the property instances (not the pointers), which results that any image node with the same level window will match the condition. The found node might not be the one whose addition triggered setting a new level window for the level window manager.

We discovered this because of another bug in our application. With a certain setting and when the image contains NaN values, we set a level window with NaN lower and upper bounds when the when the image node is added to the data storage. But since NaN values are not considered equal, the predicate does not find the node that has just been added.

Although this is a bug in our app, the current logic of the level window manager is also wrong.

If the level window manager requires a current image (it seems to be the case) then it should not provide a public interface for setting the level window without providing the image. I search for the code for calls to mitk::LevelWindowManager::SetLevelWindow and found

  • some calls from LevelWindowManager
  • one call from the unit test that checks if an exception is through if the image is not found with the given level window
  • two calls from GUI classes.

I propose the following changes:

  • make LevelWindowManager::SetLevelWindow private
  • provide a public SetCurrentImageNode function that calls SetLevelWindow internally
  • remove finding and setting the current image from SetLevelWindow
  • remove or modify the unit test. It is not relevant with the new API.
  • modify the GUI classes to use SetCurrentImageNode instead of SetLevelWindow. (The image node is available where SetLevelWindow is called, I checked.)

Event Timeline

Could be related to T19150 - Levelwindow: Image for Levelwindow seems not to be selected correctly

Talk to Tobias Norajitra who potentially saved this.

wirkert added a subscriber: wirkert.
kislinsk claimed this task.
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

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