First draft of the central classes.
Audit Required07c00faee192Unpublished
Actions

Unpublished Commit · Learn More

Not On Permanent Ref: This commit is not an ancestor of any permanent ref.

Description

First draft of the central classes.

Missing:

Decision about utils class for more fine grained manipulation operation (without impacting the MultiLabelImage interfact)
Additions for P1 (SelectionNode extension in data storage, which also will make the benifit of the proxy class clearer.
most likely many other things, but that what the review is for ;)

Details

Group Auditors

MITK Reviewer Group I

Provenance

floca	Authored on Jun 8 2021, 6:58 PM
floca	Pushed on Jun 9 2021, 8:41 AM

Parents

rMITK4e0dbe476383: Fixed T28487

Branches

Unknown

Tags

Unknown

Tasks

T28525: Design refactoring of LabelSetImage and associated classes

Event Timeline

floca added a task: T28525: Design refactoring of LabelSetImage and associated classes.Jun 9 2021, 8:43 AM

floca added an auditor: MITK Reviewer Group I.Jun 9 2021, 8:47 AM

This commit now requires audit.Jun 9 2021, 8:47 AM

kalali added a subscriber: kalali.Jun 9 2021, 9:41 AM

kalali added inline comments.

/Modules/Multilabel/mitkLabel.h
64	What is the reason for this function?
/Modules/Multilabel/mitkLabelProxyImage.h
78	Can you describe the purpose of the exterior label? Is there a more intuitive naming?
/Modules/Multilabel/mitkMultiLabelImage.h
23	Can we actually discuss "layers" and the reason to keep them? We are combining the idea of multilabel with multilayer without questioning the purpose - I'd like to discuss this.
97	What is the difference between `MergeStyle::Merge` and `OverwriteStyle::RegardLocks`?
192	Where is the `layer` given?

Do we have already decided that we want to stick with this single representation of segments as images? See Simon's bachelor thesis and the DICOM SEG standard for ideas to maybe have a little more flexible format in the design phase.

/Modules/Multilabel/mitkLabel.h
56–64	These methods are good candidates for a utility class or similar as they are not inherent properties but computable values. If we are not careful we get a super packed class.
/Modules/Multilabel/mitkLabelProxyImage.h
78	I vote for "background".

In rMITK07c00faee19294289eabffecd93b15a872f9bde3#174068, @kislinsk wrote:

Do we have already decided that we want to stick with this single representation of segments as images?

No. And I think at least mid term it might be very different.
I was also brainstorming about internally moving e.g. to itk::LabelMap and what that would mean (besides much smaller memory footprint). Other things would also possible, like continous representations as contour models or vertices representation.
But currently I think pragmatizem is key (as long as our public interface does not hinder future evolvements). Therefore I would focus on discretizes representations (because there we have the biggest need).
To a certain extend I think this should be implementation detail (especially within the discretitized sjope) and if our public interface is good, it does not make a difference.

This leads to the question of Amir regarding the layer and if we should keep it.

We have to differentiat two things.

A data concept: Layer as internal data structre (LayerImage)...
A semantic concept: Layer as semantical concept to express the fact that a pair of labels is spatial depend, this cannot be on the same place at the same time.

The data concept is needed for now if we keep the internal data structure and want to be open to other representations like @kislinsk mentioned. Reason: Concepts like DCM Seg, itk::LabelMap, ContourModels allow overlapping. This you cannot represent in one simple image (and yes I am aware that the current representation would be very memory inefficent in this case).

But surely we could/should remove GetLayerImage from the generic public (base) interface as it publishes an implementation detail and I am unsure about its practical value...

The semantic concept is a feature discussion. I think "lock feature" and "spatial dependency" are in the same league. For both you could discuss if either they are properties of the data structure, so part of its state, or they are properties of the tool/methods that manipulate the data structure and regard some constraints/boundary conditions. I think both have pros and cons.
Currently I would even say that "spatial dependency" has even more "rights" to be part of the data structure, because it is a propertie of the things that you label (e.g. liver and kidney cann not overlap).

And if we keep it a property of the data structure, we have to over posibilities to express it. You do not have to call it layers, you could also generalize and offer an interface to specify sets of spatialy dependent labels.

See Simon's bachelor thesis and the DICOM SEG standard for
ideas to maybe have a little more flexible format in the design phase.

You or @a178n are very welcome to elaborate on them. 😄

/Modules/Multilabel/mitkLabel.h
56–64	@kislinsk: You are right. With a second thought it might be better to either move it outside this class, or use a more generic approach in the class to store it. Center of mass was already in the class, I guess for the feature to init a label in the render windows. Volume I added because a Filter in ITK that computes the CoM offers Volume for free and I thought that it would be a feature (e.g. as annotation Overlay) that could be appriciated. An argument to keep these values in the label class (and there for under the control/ownership of the MultiLabelImage is that the MultiLabelImage knows when to update this information, because the label is just changed and can performe the update on the fly. May be we just exploit the IPropertyProviderInterface and provide every information that we want to attribute to the label via that interface. Then we can over utilitie classes to access dedicated properties via a getter function (that would not bloat the label class) even if we somewhen have other representations? What do you both think?
/Modules/Multilabel/mitkLabelProxyImage.h
78	Sounds good.
/Modules/Multilabel/mitkMultiLabelImage.h
23	I think we have to differenciate what "keep them" means. 1) use them internally or 2) express them conceptual in the public interface. For the rest the the answer to Stefans global comment below. Because those are interlinked.
97	See explination in the meeting today. First: how should be differences of the target label in source and destination being handled; Second: how should be lock conflicts between target label and other labels in the destination being handled.
192	Oops right. Wrong/Missleading documentation. Layer is implicitly given by label
320–323	Obsolete. Will be removed.