-
Notifications
You must be signed in to change notification settings - Fork 47
ITK5 migration #384
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
ITK5 migration #384
Conversation
Missing typename directives in declarations. Confusion between long and long long Use of itkMutexLock instead of std::mutex. Failure to include <itkMacro.h>. Use of ComposeRGBImageFilter instead of ComposeImageFilter. Use of NULL instead of nullptr. Use of vcl_sqrt instead of std::sqrt, etc. itk::MultiThreader --> itk::MultiThreaderBase. Removal of inappropriate Seg3D:: explicit qualification.
This is a squashed commit with erroneous changes of NULL to nullptr and its revert to fix the introduced SQL errors and Tinyxml updates.
so we don't need to have it as a dependency. Error was: M:\Dev\Seg3D\src\Application\LayerIO\Matlab73LayerImporter.cc(32,10): fatal error C1083: Cannot open include file: 'itkhdf5/H5LTpublic.h': No such file or directory
registration->StartRegistration() ==> registration->Update() GetJacobian(...) ==> ComputeJacobianWithRespectToPosition(...)
Here's the build log on linux(gcc 10.2):
|
@rubiojr That error appears with non-default option |
@allywarner I cherry-picked your revert of |
Re: Mac-build github build error, they (Github build farm) updated to the latest XCode and now Seg3d's ancient Python and Boost dependencies don't compile. There's an issue for that already, #315. |
The build works now on my linux machine now. |
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.
Just some small suggestions. After you decide to add them or not, I can approve the PR.
@RubioJr9 I just applied all your suggestions in a new commit. |
Counting on compilers to be smart enough to vectorize the new style as well as, or better than, the old unrolled style.
I force-pushed to this branch to retain the const-related suggestion. Loop unrolling is now in my fork's branch. As that commit contains a lot of whitespace trimming done in this branch, rebasing on current master causes conflicts. I will make a new PR once this PR is merged. This should avoid confusion. |
Sounds good, thanks a lot |
Thanks, @dzenanz! The mac build is showing a failure related to python. Would you like me to test a mac build on my local machine? |
@RubioJr9 yes! Python build fails on my local machine too ( |
The python crash is likely related to the new clang version used in github actions. We can deal with it in a new PR if it builds on other mac machines. |
I have this building with clang 11, but it is crashing due a previous opengl problem. If @RubioJr9 and @allywarner can build and run it, let's merge it. |
@jessdtate It built fine on my machine. I'm running 10.14 with clang 10.0.1. |
@jessdtate also builds for me on 10.14.6. |
Let's merge this and then immediately fix #385. |
Hooray!😄 |
💯 |
Counting on compilers to be smart enough to vectorize the new style as well as, or better than, the old unrolled style.
Review suggestion by @RubioJr9 in PR #384
This is a cleaned-up version of #382. Closes #382.