OpenHeartDevelopers
Replace MIRTK: Surface creation
This is step 1 of the larger project to remove the dependency on MIRTK; an alternate implementation of CemrgCommandLine::ExecuteSurf that uses CemrgCommonUtils::ExtractSurfaceFromSegmentation() instead of MIRTK’s extract-surface and smooth-surface binaries.
This is a draft PR right now, because there are a number of to-do items:
- [x] For now, there’s a boolean to easily switch between the old and new implementation during development and testing; that (plus the old implementation and its helper functions) needs to be deleted eventually
- [x] It still uses the MIRTK
close-imagebinary at the start - [ ] Need to check that the semantics of the threshold/blur/smoothing parameters are identical and there aren’t any subtle differences between numerical values
- [x] Optional: Check whether it makes sense to input and return pointers to an
mitk::Image/mitk::Surfacedirectly, instead of writing them to disk and passing around the file path. Updating the calling code may be more work, but this could reduce overhead quite a bit.
:zap: Code Analysis Results :zap:
:red_circle: Cppcheck found 22 issues! Click here to see details.
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Modules/CemrgAppModule/src/CemrgCommonUtils.cpp#L1813-L1818
!Line: 1813 - style: Redundant initialization for 'scalars'. The initialized value is overwritten before it is read. [redundantInitialization]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Modules/CemrgAppModule/src/CemrgCommonUtils.cpp#L1806-L1811
!Line: 1806 - note: scalars is initialized
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Modules/CemrgAppModule/src/CemrgCommonUtils.cpp#L1813-L1818
!Line: 1813 - note: scalars is overwritten
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresClipperView.h#L55-L60
!Line: 55 - style: The class 'AtrialFibresClipperView' does not have a constructor although it has private member variables. [noConstructor]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.h#L66-L71
!Line: 66 - style: The class 'AtrialFibresView' does not have a constructor although it has private member variables. [noConstructor]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresClipperView.cpp#L451-L456
!Line: 451 - style: Redundant initialization for 'radii'. The initialized value is overwritten before it is read. [redundantInitialization]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresClipperView.cpp#L450-L455
!Line: 450 - note: radii is initialized
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresClipperView.cpp#L451-L456
!Line: 451 - note: radii is overwritten
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresLandmarksView.h#L53-L58
!Line: 53 - style: The class 'AtrialFibresLandmarksView' does not have a constructor although it has private member variables. [noConstructor]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresLandmarksView.cpp#L392-L397
!Line: 392 - style: The scope of the variable 'distance' can be reduced. [variableScope]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresLandmarksView.cpp#L248-L253
!Line: 248 - style: Unused variable: fileRough [unusedVariable]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresVisualiseView.h#L50-L55
!Line: 50 - style: The class 'AtrialFibresVisualiseView' does not have a constructor although it has private member variables. [noConstructor]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L1794-L1799
!Line: 1794 - style: Condition '!userInputAccepted' is always true [knownConditionTrueFalse]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L1792-L1797
!Line: 1792 - note: Assignment 'userInputAccepted=false', assigned value is 0
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L1794-L1799
!Line: 1794 - note: Condition '!userInputAccepted' is always true
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L2336-L2341
!Line: 2336 - style: Variable 'segImage' is assigned a value that is never used. [unreadVariable]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L192-L197
!Line: 192 - style: Redundant initialization for 'reply1'. The initialized value is overwritten before it is read. [redundantInitialization]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L189-L194
!Line: 189 - note: reply1 is initialized
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L192-L197
!Line: 192 - note: reply1 is overwritten
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.mmcwplugin/src/internal/MmcwView.cpp#L721-L726
!Line: 721 - style: Condition 'dcm_path_fix' is always true [knownConditionTrueFalse]
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.mmcwplugin/src/internal/MmcwView.cpp#L720-L725
!Line: 720 - note: Assignment 'dcm_path_fix=true', assigned value is 1
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.mmcwplugin/src/internal/MmcwView.cpp#L721-L726
!Line: 721 - note: Condition 'dcm_path_fix' is always true
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Reverted this commit after discussing with @OrodRazeghi on Slack. Writing the mitk::Surface to disk between steps is useful, e.g. if clinicians want to look at the mesh and check its quality before moving on to the next steps.
It looks like there are some subtle differences between the surfaces generated by MIRTK (on the development branch) and by CemrgCommonUtils::ExtractSurfaceFromSegmentation (our designated replacement). Note in particular the top left image in the attached screenshots, where the white line generated by MIRTK seems like a better fit.
I’m not sure whether that’s caused (at least in part) by the default parameters used for MIRTK being less well suited for the new implementation; so as discussed during previous meeetings, let’s wait for @alonsoJASL to experiment with that.