OpenHeartDevelopers / CemrgApp

An Interactive Medical Imaging Platform with Image Processing and Computer Vision Toolkits for Cardiovascular Research.
http://www.cemrgapp.com
BSD 3-Clause "New" or "Revised" License
18 stars 8 forks source link

Replace MIRTK: Surface creation #83

Open JostMigenda opened 4 months ago

JostMigenda commented 4 months ago

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:

github-actions[bot] commented 4 months ago

: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 ```diff !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 ```diff !Line: 1806 - note: scalars is initialized ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Modules/CemrgAppModule/src/CemrgCommonUtils.cpp#L1813-L1818 ```diff !Line: 1813 - note: scalars is overwritten ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresClipperView.h#L55-L60 ```diff !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 ```diff !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 ```diff !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 ```diff !Line: 450 - note: radii is initialized ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresClipperView.cpp#L451-L456 ```diff !Line: 451 - note: radii is overwritten ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresLandmarksView.h#L53-L58 ```diff !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 ```diff !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 ```diff !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 ```diff !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 ```diff !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 ```diff !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 ```diff !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 ```diff !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 ```diff !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 ```diff !Line: 189 - note: reply1 is initialized ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L192-L197 ```diff !Line: 192 - note: reply1 is overwritten ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/5b4fb6d9dabc87cb0dba8545629248fd6e355c02/CemrgApp/Plugins/kcl.cemrgapp.mmcwplugin/src/internal/MmcwView.cpp#L721-L726 ```diff !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 ```diff !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 ```diff !Line: 721 - note: Condition 'dcm_path_fix' is always true ```

JostMigenda commented 4 months ago

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.

JostMigenda commented 3 months ago

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.