-
Notifications
You must be signed in to change notification settings - Fork 19
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
UI updates to Segment Mesher #7
Conversation
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.
Thank you, very nice improvements. If you plan to make future GUI changes, it would be better to upgrade this module to genereate GUI using .ui file instead of Python code.
It would be great if you could make a small GUI adjustment: make "General" tab the first, and only show the tab that belongs to the selected meshing method. If you don't plan to make these changes then fine, just let me know and I merge this as is. |
Sure, I can do those changes. I agree, we should update this to use a UI file. I can do that in this PR, there will likely be more updates to SegmentMesher from this project. |
Converted to ui file interface, and reordered/hide tabs. |
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.
Thank you. There are a few regressions (mostly related to changed parametrization in Cleaver) that must be fixed, and a few small improvements got removed that should be restored, but otherwise it looks good.
SegmentMesher/SegmentMesher.py
Outdated
self.ui.advancedTabWidget.setTabEnabled( self.ui.advancedTabWidget.indexOf(self.ui.tetgenTab), False) | ||
self.ui.advancedTabWidget.setTabEnabled( self.ui.advancedTabWidget.indexOf(self.ui.cleaverTab), True) | ||
self.ui.advancedTabWidget.setStyleSheet("QTabBar::tab::disabled {width: 0; height: 0; margin: 0; padding: 0; border: none;} ") | ||
|
||
enabled = True |
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.
is this used anywhere?
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.
typo, will remove
SegmentMesher/SegmentMesher.py
Outdated
@@ -403,13 +331,13 @@ def getTetGenPath(self): | |||
def getCustomCleaverPath(self): | |||
settings = qt.QSettings() | |||
if settings.contains(self.customCleaverPathSettingsKey): | |||
return settings.value(self.customCleaverPathSettingsKey) | |||
return slicer.util.toVTKString(settings.value(self.customCleaverPathSettingsKey)) |
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 reverts an intentional change. toVTKString
is not needed anymore.
SegmentMesher/SegmentMesher.py
Outdated
return '' | ||
|
||
def getCustomTetGenPath(self): | ||
settings = qt.QSettings() | ||
if settings.contains(self.customTetGenPathSettingsKey): | ||
return settings.value(self.customTetGenPathSettingsKey) | ||
return slicer.util.toVTKString(settings.value(self.customTetGenPathSettingsKey)) |
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 reverts an intentional change. toVTKString
is not needed anymore.
SegmentMesher/SegmentMesher.py
Outdated
@@ -449,7 +377,7 @@ def startMesher(self, cmdLineArguments, executableFilePath): | |||
|
|||
logging.info("Generate mesh using: "+executableFilePath+": "+repr(cmdLineArguments)) | |||
return subprocess.Popen([executableFilePath] + cmdLineArguments, | |||
stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True, startupinfo=info) |
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 reverts an intentional change.
SegmentMesher/SegmentMesher.py
Outdated
@@ -489,10 +417,11 @@ def createTempDirectory(self): | |||
qt.QDir().mkpath(dirPath) | |||
return dirPath | |||
|
|||
def createMeshFromSegmentationCleaver(self, inputSegmentation, outputMeshNode, additionalParameters = None, removeBackgroundMesh = False, paddingRatio = 0.10): | |||
def createMeshFromSegmentationCleaver(self, inputSegmentation, outputMeshNode, segments = [], additionalParameters = None, removeBackgroundMesh = False, paddingRatio = 0.10, scale = 0.2, multiplier=0.5, grading=1.0): |
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.
Parametrization has changed in Cleaver. Parameters are now: feature scaling, sampling rate, and maximum rate of change
SegmentMesher/SegmentMesher.py
Outdated
|
||
inputLabelmapVolumeFilePath = os.path.join(tempDir, "inputLabelmap.nrrd") | ||
slicer.util.saveNode(labelmapVolumeNode, inputLabelmapVolumeFilePath, {"useCompression": False}) | ||
inputParamsCleaver.extend(["--input_files", inputLabelmapVolumeFilePath]) | ||
inputParamsCleaver.append("--segmentation") |
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 reverts an intentional change.
SegmentMesher/SegmentMesher.py
Outdated
@@ -553,6 +492,11 @@ def createMeshFromSegmentationCleaver(self, inputSegmentation, outputMeshNode, a | |||
slicer.mrmlScene.RemoveNode(labelmapVolumeNode) | |||
slicer.mrmlScene.RemoveNode(colorTableNode) | |||
|
|||
#User set parameters | |||
inputParamsCleaver.extend(["--scale", "{:.2f}".format(scale)]) |
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.
These parameter values are not valid anymore.
SegmentMesher/SegmentMesher.py
Outdated
@@ -563,7 +507,7 @@ def createMeshFromSegmentationCleaver(self, inputSegmentation, outputMeshNode, a | |||
inputParamsCleaver.append("--verbose") | |||
|
|||
# Quality | |||
inputParamsCleaver.extend(additionalParameters.split(' ')) | |||
inputParamsCleaver.extend(slicer.util.toVTKString(additionalParameters).split(' ')) |
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 reverts an intentional change.
def cleanup(self): | ||
pass | ||
|
||
|
||
def updateMRMLFromGUI(self): |
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 would be nice to save widgets state into a parameter node (see how it is done in the module template)
Ah, it looks like i screwed up the rebase. |
- Allow TetGen to use surface mesh as input - Expose useful Cleaver options - Expose useful TetGen options
Adds a checkable combobox for selecting segment(s) to mesh
Previously they were un-formatted at full machine precision.
Fixed the unintended reverts, and the Cleaver parameters. Working on parameter node saving |
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.
Thanks a lot for the thorough update. There are so many changes that I cannot review them in detail, but overall everything looks good and we can fix things up later if we find problems.
breaks out useful command line options to GUI options
Separate advanced parameters into tabs
Allows TetGen to mesh directly from a surface
Instead of selecting segments to mesh based on visibility, a check-able drop down is now provided