Remove itkv3compat dependency fix warnings and errors fix extension package #45
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit updates the required version to 3.5
This commit fixes the following warning: ``` CMake Warning (dev) at DTIPrepTools.cmake:195: Syntax Warning in cmake code at column 50 Argument not separated from preceding token by whitespace. Call Stack (most recent call first): CMakeLists.txt:31 (include) This warning is for project developers. Use -Wno-dev to suppress it. ```
…mpatibility This commit updates code to directly use KernelFunctionBase instead of KernelFunction.
This commit fixes warnings like the following:
```
/tmp/DTIPrep/src/itkDWIQCSliceChecker.hxx: In member function ‘void itk::DWIQCSliceChecker<TImageType>::calculateCorrelations(bool)’:
/tmp/DTIPrep/src/itkDWIQCSliceChecker.hxx:347:71: warning: declaration of ‘FilterType’ shadows a previous local [-Wshadow]
ShortSliceImageType> FilterType;
^
/tmp/DTIPrep/src/itkDWIQCSliceChecker.hxx:286:71: note: shadowed declaration is here
GradientImageType> FilterType;
^
```
This commit explicitly links against the libraries as it would be done using SEMMacroBuildCLI macro. It fixes the following error: ``` [ 22%] Linking CXX executable /tmp/DTIPrep-build/bin/DTIPrepLauncher CMakeFiles/DTIPrepLauncher.dir/Launcher.cxx.o: In function `main': Launcher.cxx:(.text.startup+0x7dd): undefined reference to `Json::Value::Value(Json::ValueType)' Launcher.cxx:(.text.startup+0x7ec): undefined reference to `Json::Reader::Reader()' [...] ```
…ed "GetRotationMatrix"
This commit fixes the build errors like the following when building against
ITK without ITKV3Compatibility module:
```
/tmp/DTIPrep/src/itkLinearHeadEddy3DCorrection.hxx: In member function ‘const RotationMatrixType& itk::LinearHeadEddy3DCorrection<TScalarType, NInputDimensions, NOutputDimensions>::GetHeadMotionRotationMatrix() const’:
/tmp/DTIPrep/src/itkLinearHeadEddy3DCorrection.hxx:174:35: error: ‘itk::SmartPointer<itk::VersorRigid3DTransform<double> >::ObjectType {aka struct itk::VersorRigid3DTransform<double>}’ has no member named ‘GetRotationMatrix’
return m_head_motion_transform->GetRotationMatrix();
^
```
…cal-typedefs] This commit fixes warnings like the following: ``` /tmp/DTIPrep/src/itkDWIQCGradientChecker.hxx: In member function ‘void itk::DWIQCGradientChecker<TImageType>::rigidRegistration(itk::Image<short unsigned int, 3u>::Pointer, itk::Image<short unsigned int, 3u>::Pointer, unsigned int, double, bool, itk::DWIQCGradientChecker<TImageType>::GradientResult&)’: /tmp/DTIPrep/src/itkDWIQCGradientChecker.hxx:123:3: warning: typedef ‘SpacingType’ locally defined but not used [-Wunused-local-typedefs] SpacingType; ^ /tmp/DTIPrep/src/itkDWIQCGradientChecker.hxx:125:3: warning: typedef ‘OriginType’ locally defined but not used [-Wunused-local-typedefs] OriginType; ^ /tmp/DTIPrep/src/itkDWIQCGradientChecker.hxx:127:3: warning: typedef ‘RegionType’ locally defined but not used [-Wunused-local-typedefs] RegionType; ^ /tmp/DTIPrep/src/itkDWIQCGradientChecker.hxx:129:3: warning: typedef ‘SizeType’ locally defined but not used [-Wunused-local-typedefs] SizeType; ^ ```
…ble]
This commit fixes warnings like the following:
```
/tmp/DTIPrep/src/itkDWIEddyCurrentHeadMotionCorrector.hxx:240:7: warning: variable ‘DWICount’ set but not used [-Wunused-but-set-variable]
int DWICount, BaselineCount;
^
/tmp/DTIPrep/src/itkDWIEddyCurrentHeadMotionCorrector.hxx:240:17: warning: variable ‘BaselineCount’ set but not used [-Wunused-but-set-variable]
int DWICount, BaselineCount;
^
```
…logic issue Based on the operator precedence [1], adding parenthesis around "!qcResult.Get_result()" would be the right things to do address the warning, that said this is incorrect since the value returned by "Get_result()" always evaluates to 1 or 0. [1] http://en.cppreference.com/w/c/language/operator_precedence See NIRALUser#44 This commit improves readability introducing intermediate variables and move the logical not operator outside the bitwise test. This commit fixes the following warning: ``` tmp/DTIPrep/src/IntensityMotionCheckPanel.cxx: In member function ‘void IntensityMotionCheckPanel::ResultUpdate()’: /tmp/DTIPrep/src/IntensityMotionCheckPanel.cxx:3978:33: warning: suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’ to ‘~’ [-Wparentheses] if( ( ( (!qcResult.Get_result() & InterlaceWiseCheckBit) == InterlaceWiseCheckBit ) || ^ ```
…tImage" The function "GetCoefficientImage" is not available when building with ITKV3_COMPATIBILITY disabled.
…ializer See "B-spline Registration Refactoring" migration guide available at https://itk.org/migrationv4/index.php?action=artikel&cat=3&id=82&artlang=en and also documentation associated with itk::BSplineTransform::SetFixedParameters() at https://itk.org/Insight/Doxygen/html/classitk_1_1BSplineTransform.html#aff697faefc33b586c4560114226bc499 Note that call to "GetGridRegion()" was replaced with call to "GetTransformDomainMeshSize()". This commit fixes build errors like the following when building against ITK without ITKV3Compatibility module: ``` /tmp/DTIPrep/src/itkMultiImageRegistrationFilter.h:242:11: error: ‘BSplineDeformableTransformInitializer’ does not name a type typedef BSplineDeformableTransformInitializer<BSplineTransformType, ImageType> BSplineInitializerType; ^ /tmp/DTIPrep/src/itkMultiImageRegistrationFilter.h:330:15: error: ‘BSplineInitializerType’ was not declared in this scope std::vector<BSplineInitializerType::Pointer> m_BsplineInitializerArray; ^ /tmp/DTIPrep/src/itkMultiImageRegistrationFilter.h:330:46: error: template argument 1 is invalid std::vector<BSplineInitializerType::Pointer> m_BsplineInitializerArray; ^ /tmp/DTIPrep/src/itkMultiImageRegistrationFilter.h:330:46: error: template argument 2 is invalid In file included from /tmp/DTIPrep/src/GMainWindow.h:6:0, from /tmp/DTIPrep/src/main.cxx:11: ```
…formInitializer" This reverts commit c08f230a0cb00204b352ae8820ec19a1c6dbb606. It turns out that updating the code to use BSplingTransform is not trivial. Indeed, there is no direct replacement for the SetBulkTransform, function.
Since updating the code to use BSplineTransform (instead of the deprecated BSplineDeformableTransform) is not trivial (see 86c8966 where we reverted our attempt), this commit introduces copy of BSplineDeformableTransform and BSplineDeformableTransformInitializer specific to this project. These new files are prefixed with "Niral" and uses the namespace "itk::niral::" instead of "itk::".
ExternalProject parameter GIT_CONFIG was introduced in CMake 3.8 See https://cmake.org/cmake/help/v3.8/module/ExternalProject.html
This will allow to more easily understand while options are added or removed in the future.
Testing for the value of DTIPrep_BUILD_SLICER_EXTENSION allows to implement the same logic.
…ension Now that "niral_utilities", "UnivariateEntropyMultiImageMetric" and "DTIPrep" can all be built using a regular build of ITKv4 without requiring the option ITKV3_COMPATIBILITY to be enabled, this commit updates the requirement of DTIPrep Slicer extension so that the build trees provided by Slicer can be reused. Additionally, the build time of the Slicer extension will also be reduced.
This commit fixes a regression introduced in 2ea5e5b (ENH: Update SuperBuild process and package extension for SLICER) and ensures niral tools are packaged.
…ype of its field
This commit fixes warnings like the following:
```
In file included from /tmp/DTIPrep/src/GMainWindow.h:6:0,
from /tmp/DTIPrep/src/main.cxx:11:
/tmp/DTIPrep/src/IntensityMotionCheckPanel.h:21:7: warning: ‘IntensityMotionCheckPanel’ declared with greater visibility than the type of its field ‘IntensityMotionCheckPanel::QCedDwiReader’ [-Wattributes]
class IntensityMotionCheckPanel : public QDockWidget,
^
In file included from /tmp/DTIPrep/src/main.cxx:11:0:
/tmp/DTIPrep/src/GMainWindow.h:57:7: warning: ‘GMainWindow’ declared with greater visibility than the type of its field ‘GMainWindow::DwiReader’ [-Wattributes]
class GMainWindow : public QMainWindow, private Ui_MainWindow
^
/tmp/DTIPrep/src/GMainWindow.h:57:7: warning: ‘GMainWindow’ declared with greater visibility than the type of its field ‘GMainWindow::QCedDwiReader’ [-Wattributes]
In file included from /tmp/DTIPrep/src/GMainWindow.h:6:0,
from /tmp/DTIPrep/src/GMainWindow.cxx:3:
/tmp/DTIPrep/src/IntensityMotionCheckPanel.h:21:7: warning: ‘IntensityMotionCheckPanel’ declared with greater visibility than the type of its field ‘IntensityMotionCheckPanel::QCedDwiReader’ [-Wattributes]
class IntensityMotionCheckPanel : public QDockWidget,
^
```
jcfr
deleted the
remove-itkv3compat-dependency-fix-warnings-and-errors-fix-extension-package
branch
|
Thanks for reviewing and integrating. As a side note, now change have also been integrated in DTIPrep/SuperBuild/External_niral_utilities.cmake Lines 82 to 83 in 5dd0a4d |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@juanprietob @styner While testing the recent updates associated with Slicer build system, I ended updating your extension.
I also realized that the extension was building its own copy of
BRAINSTools,ITKandSlicerExecutionModelbecause it was depending on a version of ITK built withITKV3_COMPATIBILTY.Since this wasn't a strategic use of build time we provide on the Slicer build machine, I spent some time improving the extension.
Where possible, I updated the code to use ITKv4 constructs and where it wasn't doable quickly I ended up copying the ITK classes adding the
Niralprefix to the file and moving the class in aitk::niral::namespace instead of the usualitk::one.Commit message have been made to as informative as possible:
These should facilitate review.
All of that said, please, consider reviewing and testing this set of changes.
Similarly, I contributed changes to the following repository:
Fix warnings, build errors, improve build system and remove need for itk deprecated niral_utilities#16MergedFix build error using sem macro build cli ImageNoise#1 (included byMergedniral_utilities)Fix warnings improve buildsystem PolyDataCompression#1 (included byMergedniral_utilities)Require slicer execution model CurveCompare#1 (included byMergedniral_utilities)If you prefer, I can update this branch after your are done reviewing and updating the other projects.
As a side note, the extension still require some work to work with Qt5.
Let us know with you have any questions. The Slicer discussion forum is a good outlet for questions.
As a side note, the migration guide associated with VTK6, VTK7, VTK8, Qt5 and Slicer are now all linked of https://www.slicer.org/wiki/Documentation/Nightly/Developers/Tutorials/MigrationGuide
Cc: @fedorov @lassoan @pieper @bpaniagua @fbudin69500