@Nil reported on the mailing list that ITK fails to read a NIFTII image with 64-bit pixel type. NRRD images with 64-bit pixel types can’t be read either, at least on Windows with VS2015 and probably on any compiler where 4 == sizeof(long).
Should we repurpose ULONG and LONG to be 64-bit? Currently it is redundant with UINT and INT. This is defined in itkImageIOBase.h by typedef enum { UNKNOWNCOMPONENTTYPE, UCHAR, CHAR, USHORT, SHORT, UINT, INT, ULONG, LONG, FLOAT, DOUBLE } IOComponentType;
It is unfortunate but the current interface for ImageIOBase is system specific. So the size of ULONG depends on the system. (There also is a more subtle detail with the char and signed-ness of char types too.) There is a missing 64-bit integer type on Windows system (32-bit and 64-bit) along with GCC/GNU Linux 32-bit systems ( where the unsigned long type varies with this size of pointers. ) Also on more obscure systems, these intrinsic types may have different sizes than expected.
While ideally, I think the ImageIO should present fixed with integers for its interface. This is not the current design.
I am thinking the best option is to add long long types to the interface. However the complication is ( and currently is ) that multiple component types can be the same size. This is currently a problematic and error prone situation for many of the ImageIOs as they have to deal with interfacing the io libraries definition of pixel type sized with ITK.
Also remember that ITK by conventions uses these “intrinsic” C++ types for pixel types and not fixed width integers, the the current ImageIO interface follows that convention.
I propose that we change this. So far, there haven’t been too many images with 64-bit pixel size, but that is likely to change in the future. We might be a bit late with this fix, but better late then never.
We could add something like a new enum FixedWidthComponentType, and corresponding get methods. Then rewrite the Set/GetComponentType methods to translate the FixedWidthComponentType with the current policies. The ImageIO’s could be updated to the fixed width as time allows, and users can update to the fixed with as times allows. Hopefully this would enable compatibility for the current interface while migrating to the fixed interface ( pun intended ).
For reference this is what I am doing in SimpleITK:
Changing ULONG to always be a 64-bit integer would be VERY confusing and may break people’s code.
This could work, but care would need to be taken that there is only one ComponentType member that the Get and Set methods interpret it as needed. Duplication of state will cause more issues than it fixes.
In practice, I am not sure how true that is since it already yields inconsistent behavior by platform.
With the naive translation of the ULONG to “unsigned long” implemented in a case statement, I think there may be many people who are happily using 64-bit integers on linux system. The proposal for changing ULONG to mean 64-bit integer actually would not break this linux use case. But to enable 64-bit integers on windows or 32-bit linux it would not be clear that “uint64_t” should be used over the type described by the enum.
I think the real problem will be the unclarity with interfacing itk::ImageIO with the third party io libraries, the conversion for the library type to ITK type would be more obscured.
It is hard for me to imagine 3rd party libraries which prefer compiler and processor architecture variability of long over fixed certainty of 64-bit integer. As Windows is the most widespread OS, and 32-bit OSes are going away, I am in favor of fixing 64-bit pixel types for Windows even if it means breaking long type for 32-bit Linux.
I am certainly in favor of fixing things for windows.
For consideration consider NrrdIO:
IMHO, there are two options, use the intrinsic type names, or use the fixed width names. Adding arbitrary definitions to for the size of intrinsic names is not a good choice.
This is a legacy of pre-C++11 constraints. There were no uint64_t and friends back then. But if you don’t like this option even tough it would lead to least amount of modifications, we should go with fixed width names. We should keep the current enum numbers which are backwards compatible with LP64 data model.
That looks like a great start. The devil will be in the details with ensure the different file format behave properly across platforms. But that is a second step after establishing the interface.
I am fine with this approach taken where the documentation for the ComponentType is followed:
You are not annoying. Quite the contrary, we too are annoyed by inability to use modern C++. It is on our wish list of things for version 5, using modern C++ is one of the prominent things! But I don’t know when we will have resources to make a major overhaul to ITK.
I think having the resources to address this type of issue, and the ability to update the interfaces based on lessons learned and modern conventions is more limiting than just updating the language version requirement. We have had fixed width integers defined in ITK for nearly 10 years now but the interfaces were never updated to used them.
This sounds like a good idea to me… Those type names like itk::ImageIOBase::LONG are very ambiguous/confusing. Would be nice to deprecate them in favour of names like itk::ImageIOBase::SInt64 and define the old names in terms of the new names as a transition.
For my own selfish reasons :), I hope you decide not to break LP64 code (like mine).