search

InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.42k stars 664 forks source link

Handeling RGB, RGBA and Vector pixel types with templates since #870 #945

Closed romangrothausmann closed 5 years ago

romangrothausmann commented 5 years ago

Description

870 introduced compatibility problems for programs that try to handle images with pixel types of RGB, RGBA and Vector by making use of templates (like this CLI anisoDiff-curv. While #929 addresses wrapped languages, there is no fix for C++ usage (see https://github.com/InsightSoftwareConsortium/ITK/pull/900#issuecomment-494806690).

When compiling anisoDiff-curv with RGB, RGBA enabled (https://github.com/romangrothausmann/ITK-CLIs/blob/9d20a12ea5367be6ae243aadc6be7049aee023ee/anisoDiff-curv.cxx#L110-L129) as it was possible before, the logs: https://gitlab.com/romangrothausmann/ITK-CLIs/-/jobs/217220852 report:

/code/anisoDiff-grad.cxx:315:5:   required from here
/opt/itk/include/ITK-5.0/itkCastImageFilter.hxx:126:3: error: static assertion failed: Vector dimensions are required to match!

Impact analysis

The former design allowed to handle RGB, RGBA and Vector pixel types while with #870 this so far (up to https://github.com/InsightSoftwareConsortium/ITK/commit/8dc783a95a20699c54f9ddb345f9cd156af6b7be) is not possible any more.

Additional Information

If no easy solution exists, this issue could just be accepted as an incompatibility from ITK-4 to ITK-5.

blowekamp commented 5 years ago

It's unclear to me from the link which template parameters are the problem. Could you provide a minimal test case?

romangrothausmann commented 5 years ago

Thanks @blowekamp for taking a look. Basically for the case of RGB, its https://github.com/romangrothausmann/ITK-CLIs/blob/9d20a12ea5367be6ae243aadc6be7049aee023ee/anisoDiff-curv.cxx#L115 and/or https://github.com/romangrothausmann/ITK-CLIs/blob/9d20a12ea5367be6ae243aadc6be7049aee023ee/anisoDiff-curv.cxx#L117 that are causing the compile errors. I could reduce the code of anisoDiff-curv if that would be of help or construct a new MNWE but would need some time for that.

blowekamp commented 5 years ago

What is InputImageType and RealImageType on that line?

romangrothausmann commented 5 years ago

The CLI is either compiled with f32 or f64 precision https://github.com/romangrothausmann/ITK-CLIs/blob/9d20a12ea5367be6ae243aadc6be7049aee023ee/anisoDiff-curv.cxx#L92-L98 which is then used to define the RealImageType https://github.com/romangrothausmann/ITK-CLIs/blob/9d20a12ea5367be6ae243aadc6be7049aee023ee/anisoDiff-curv.cxx#L114 InputImageType is defined https://github.com/romangrothausmann/ITK-CLIs/blob/9d20a12ea5367be6ae243aadc6be7049aee023ee/anisoDiff-curv.cxx#L113 from InputPixelType and Dimension which come from a higher template level, e.g. https://github.com/romangrothausmann/ITK-CLIs/blob/9d20a12ea5367be6ae243aadc6be7049aee023ee/anisoDiff-curv.cxx#L148-L166 to be able to cope with e.g. 2D and 3D input.

blowekamp commented 5 years ago

And the types which are the problem are... ( please explicitly list the types)

blowekamp commented 5 years ago

I tries several cast in this PR #953, they all worked except for CastImageFilter<Image<complex<double>>, Image<complex<float>>> where I had to make the Cast ImageFilter code change for. There are likely other pixel types that the explicit cast is also required over the implicit "convertible". Perhaps your code will work with this change now.

romangrothausmann commented 5 years ago

And the types which are the problem are... ( please explicitly list the types)

The problem seems to be where pixel type is RGB but components per pixel != 3 (similar RGBA and cpp != 4), which apparently worked before but ideally should only be covered by a general vector image.

See, https://github.com/InsightSoftwareConsortium/ITK/pull/953#pullrequestreview-241122505 for the current details.

So far I could not find a way to restrict the handling of RGB to cpp == 3 and RGBA to cpp == 4 (i.e. the clean way) while still allowing to handle cpp == 2 as a vector image with the current recursive template construct.

romangrothausmann commented 5 years ago

From https://github.com/InsightSoftwareConsortium/ITK/pull/953#issuecomment-495227841

old usage of VectorCastImageFilter had undefined behavior related to uninitialized pixel elements and out of bounds access. I would classify this behavior as a bug, at that it has been fixed in ITKv5.

So also the new design allows to handle RGB, RGBA and Vector pixel types but now fails in case of undefined behavior which was not the case with the old design. I agree, this is a good thing (since it happened to be overlook before), so a solution needs to be found on the application side to discriminate properly between RGB, RGBA and and general Vector pixel types. Suggestions for example for template constructs as in anisoDiff-curv are very welcome.

blowekamp commented 5 years ago

Suggestions for example for template constructs as in anisoDiff-curv are very welcome.

I don't think you need a cast in the program there. Could you read directly into an Image of Vector of the proper length?