Page MenuHomePhabricator

Refactor of LevelWindow and serialization of LevelWindowProperty
Open, WishlistPublic

Description

Current status: The LevelWindow class has a weak cohesion and to many jobs

  1. It is used to implement business logic to deduce level window bounds (e.g from an image) or to check/ensure consistency
  2. As data class to communicate level window and possible ranges between rendering manger and LevelWindowWidgets
  3. As data class to store the level window bounds (in the LevelWindowProperty) as a rendering property for a data node

I think it would be better to seperate jobs. One class (case 1) that can be used to establish value bounds (given e.g, an image or just by direct setters) and another class (or two if we want to seperate case 2 and 3) to store rendering information in the node. For case 3 it would be also an option to store it in several simple value properties (2 or 4 double properties) instead of a custom one (like it is currently done). I also think that is redundant and potentially errorneous to store the value range in case 3 as node property. The value range is an intrinsic property of the data itself. No need to replicate it. In T24886: MITK workbench freezes due to infinite loop in QmitkSliderLevelWindowWidget::paintEvent this behavior is also part of the problem for data loaded from MITK session files.

Event Timeline

floca renamed this task from Refector of LevelWindow and serialization of LevelWindowProperty to Refactor of LevelWindow and serialization of LevelWindowProperty.Jun 25 2018, 11:18 AM

We discussed this task. It makes sense to refactor the class and to splitt into classes/structures with higher cohesion.

Comment: If we tackle part 3 (property storing and serialization) without reworking the whole business logic part (part 1+2), we would need to change code in the scene serialization itself (in a back compatible way!!!). Reason: Because otherwise we cannot be sure that the image min/may values are presents. The cleanest way then would be to get all statistics information on the fly and only the selected bounds are persisted.

kislinsk triaged this task as Wishlist priority.Nov 22 2018, 11:47 AM
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

floca removed a project: Auto-closed.
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

This could be potentially solved with T30293 if we also provide a writer (at least a very simple one that does the old style, besides that change.