-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: Represent calculated isodose surfaces as a segmentation node #191
base: master
Are you sure you want to change the base?
Conversation
Isodose/Logic/vtkMRMLIsodoseNode.h
Outdated
@@ -141,6 +146,9 @@ class VTK_SLICER_ISODOSE_LOGIC_EXPORT vtkMRMLIsodoseNode : public vtkMRMLNode | |||
/// Whether use relative isolevels representation | |||
/// for absolute dose (Gy) and unknown units or not | |||
bool RelativeRepresentationFlag; | |||
|
|||
/// Use segmentation flag for isodose models | |||
bool SegmentationFlag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite unfortunate name I think, because only vaguely refers to its purpose. How about UseSegmentationToStoreIsodoseSurfaces
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will rename it.
scene->AddNode(segmentationNode); | ||
|
||
// Set master representation to planar contour | ||
segmentationNode->GetSegmentation()->SetMasterRepresentationName(vtkSegmentationConverter::GetSegmentationPlanarContourRepresentationName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please explain in detail how you intend to use these segmentation objects, including why you use planar contour as master?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an alternative option how to store isosurfaces.
the code:
segmentationNode->SetMasterRepresentationToClosedSurface();
doesn't work. It doesn't show isodose surfaces on the views. It works with the planar contours.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Planar contours" are not suitable as master representation because they only store 2D contours and only in one specific orientation. Probably the best representation for storing isodose contours/surfaces is "closed surface".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can set the preferred display representations in the display node as far as I remember. That should fix the "doesn't show" part.
} | ||
else | ||
{ | ||
d->toggleButton_IsodoseRepresentation->setIcon(QIcon(":/Icons/Segmentation.png")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have a button for this? I need to know more about this whole work, please give us an overview, and rather write more than less. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a toggle button, if the button is pressed, then segmentation node would be used to store isodose surfaces. If the button isn't pressed, then original isodose representation would be used (model nodes into folder).
Toggle button is pressed in the red square.
Isodose surfaces are stored as a segmentation node named "RTDOSE[1]_IsodoseSurfaces"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This I understood of course. Why do you need to be able to switch between the two? Why expose it to the user? What is the reason you needed the segmentation representation in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be enough to only use segmentation. It takes two clicks in Data module to export model hierarchy from a segmentation node.
This work is the result of this discussion: #80. @MichaelColonel add #80 to the commit comment to make it clear what issue this development resolves. I like this development, because it reduces the complexity of the scene (less nodes) and the visualization is nicer (filled color region and not just the contour).
It might be useful to have two modes:
- A. each segment contains a solid region of dose > threshold
- B. each segment contains "ring" shaped region of thresholdA < dose < thresholdB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I completely agree with these. I was mostly curious why do does this branch introduce this complexity of being able to switch from the module. Switching to segmentation representation instead of model nodes makes sense, but there was nothing in this PR referring to its motivation.
So I'd suggest removing the button and the logic for the switching. It adds unnecessary complexity as much on the UI as in the source code (making it more error-prone). Unless you have a good reason offering this option. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will do as you suggested. At the beginning nobody told me to delete initial version of isodose representation, that is why i made a toggle button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added better description into commit, and implementation of two modes: Single border mode (A) and Double border mode (B). Mode can be switched before calculation by the toggle button.
11cafc5
to
ad2189f
Compare
This is a partial solution of the issue SlicerRt#80. There are two modes of isodose representation have been implemented, single border and double border mode. Single border mode (solid surface) shows isosurface for dose high than a thresholdMin. Double border mode (hollow or ring-shaped surface) shows isosurface in dose range from thresholdMin up to thresholdMax. For example dose values: 10 Gy, 25 Gy, 30 Gy, 50Gy. Single border mode will generate isosurfaces: higher than 10Gy, higher than 25 Gy, higher than 30 Gy, higher than 50 Gy. Double border mode will generate isosurfaces: from 10 Gy to 25 Gy, from 25 Gy to 30 Gy, from 30 Gy to 50 Gy.
ad2189f
to
0775c60
Compare
Toggle button near "Number of iso levels:" QSpinBox is added to GUI to select type of representation.
Original representation as models into folder
New representation as a single segmentation node