Update ITK to v5.2.0 to fix error during OpenJPEG build

[yoda-vid]

This ITK update fixes a build error I encountered on Windows using Microsoft Visual Studio 16.9.3. The error occurs while compiling OpenJPEG for ITK with this message (multiple times):

Y:\builds_se_win\build_se_py3.6.8\ITK\Modules\ThirdParty\OpenJPEG\src\openjpeg\opj_includes.h(106,35): error C2169: 'lr
intf': intrinsic function, cannot be defined [Y:\builds_se_win\build_se_py3.6.8\ITK-build\Modules\ThirdParty\OpenJPEG\s
rc\openjpeg\itkopenjpeg.vcxproj] [Y:\builds_se_win\build_se_py3.6.8\ITK.vcxproj]

This error appears to the be the same as the one fixed recently in ITK:
InsightSoftwareConsortium/ITK#1967
InsightSoftwareConsortium/ITK#2388

I do not know of a way to pull this fix into SimpleElastix other than to pull in a more recent ITK commit (if there's another way, please let me know!). I simply bumped the tag from v5.1rc02 to v5.2.0 since this appears to be the only tag currently with this fix. I also found this update in SimpleITK (SimpleITK/SimpleITK#1366), so feel free to ignore this PR if upstream SimpleITK will be pulled in anyway.

With this update, the ITK compilation completes. Note that I could not fully complete the SimpleElastix compilation until I also checked out the latest branch here, update-simpleitk, which I built on commit 175864a , to avoid an Elastix build error.

I also had to use the official Cmake (tested on the latest version, v3.20.0) rather than MSVC Cmake (tested on v3.19.20122902-MSVC_2) to avoid an error when configuring Eigen3, which may be related to these issues:
InsightSoftwareConsortium/ITK#1888
ANTsX/ANTsR#275

[N-Dekker]

Thanks very much David. Actually I just encountered the very same compilation error, this afternoon!

@kaspermarstal @ViktorvdValk Is it also fine to you to upgrade to ITK 5.2.0? And do you happen to know why the CI rejected this pull request?

[N-Dekker]

FYI, We're preparing to upgrade ITK for elastix as well: https://github.com/SuperElastix/elastix/tree/Upgrade-ITK-to-v5.2.0

[yoda-vid]

Great! I hope this might be helpful at least for troubleshooting.

[N-Dekker]

@yoda-vid For what it's worth, I reproduced the original compilation error at https://godbolt.org/z/rbnsxarYW

#include <math.h>
long lrintf(float) { return 0; }
int main() {}

Compiler command line: /Oi /Bv
(/Oi enables intrinsic functions.)

Output:

error C2169: 'lrintf': intrinsic function, cannot be defined

Compiler version:

C:\data\msvc\14.28.29914\bin\Hostx64\x64\cl.exe: Version 19.28.29914.0

[N-Dekker]

For the record, the lrintf function from OpenJPEG was originally renamed to "opj_lrintf" by Matthieu Darbois (@mayeut), 26 July 2015, "Remove some warnings when building" uclouvain/openjpeg@c423cc8#diff-91c70438e617694516cb0ab60919517ff67034354cf73070cb243a7680411f89R126

https://github.com/uclouvain/openjpeg/blob/c423cc84e7be79051a7f9631fa26aa7d072361f2/src/lib/openjp2/opj_includes.h#L126

This commit is included with OpenJPEG tag v2.1.1

[yoda-vid]

This is really cool, thanks for sleuthing that out! So from my understanding:

• OpenJPEG fixed this awhile back
• ITK ships an older version of OpenJPEG that did not include this fix
• The recent MSVC update now gives an error (eg as suggested in [openjpeg] build failure x64-windows with features (jp3d,jpip,jpwl,mj2) on VS 2019 latest update 16.9.1 microsoft/vcpkg#16645 (comment))?
• The ITK PR fixed this issue for their version

Seems to make sense now why my previous build of SimpleElastix (on commit 2a79d15) with Visual Studio 2019 (MSVC 14.22.27905) worked, whereas my recent build on the same commit did not on MSVC 14.28.29910.

[N-Dekker]

• OpenJPEG fixed this awhile back
• ITK ships an older version of OpenJPEG that did not include this fix
• The recent MSVC update now gives an error (eg as suggested in microsoft/vcpkg#16645 (comment))?
• The ITK PR fixed this issue for their version

@yoda-vid Well summarized, David 😃 By the way, ITK's fix is not entirely complete, as it defines opj_lrintf for MSVC, but not for Clang and GCC. As I mentioned at InsightSoftwareConsortium/ITK#2388 (comment) But in practice it doesn't matter much, I guess, because ITK does not use opj_lrintf anyway 🤔

[yoda-vid]

Good to know! At least we will have a place to start looking if the builds start to break on Clang or GCC.

[kaspermarstal]

Great, thanks @yoda-vid! Could you make the PR against develop instead of master? Develop was just updated to SimpleITK v2.0.2 and I hope to merge into master soon. Better check these changes are compatible :)

[yoda-vid]

Thanks, @kaspermarstal ! I made a new PR on develop: #428 . Initially I tried to use the GitHub function to change the PR base here, but it didn't seem to update the base when I pulled it, so I just created a new PR instead.

[davidweioct]

Hi, not sure if this issue has solved or not for simpleElastix. I tried with both the master branch and the develop branch over the weekend, but the issue still exists. error C2169 repeated 19 times. Is there a solution? An old version (early 2020) of the package would also work for me. Is there a way to clone that? Thanks a million!

[N-Dekker]

@davidweioct

Hi, not sure if this issue has solved or not for simpleElastix. I tried with both the master branch and the develop branch over the weekend, but the issue still exists. error C2169 repeated 19 times.

As a quick-fix you may simply remove both static lrintf function definitions from you copy of ITK/Modules/ThirdParty/OpenJPEG/src/openjpeg/opj_includes.h The function isn't being called by anyway, neither by ITK nor by elastix.

HTH, Niels

[yoda-vid]

As a quick-fix you may simply remove both static lrintf function definitions

This is good to know, thanks @N-Dekker!

Just to add on, the build is working in PR #428 on develop (at least at the time of posting). I've checked it out locally by:

git fetch origin pull/428/head:test_pr428
git checkout test_pr428

(Assuming origin points to the main repo.)

[davidweioct]

Thank you both @N-Dekker @yoda-vid .

I tried by commenting out those lines, and this works for me without any error.