COMP: Fix image pointer casting error

[jhlegarreta]

Fix image pointer casting error.

Fixes

In file included from
ITK/Modules/Core/FiniteDifference/include/itkDenseFiniteDifferenceImageFilter.h:194:0,
from
ITK/Modules/Filtering/AnisotropicSmoothing/include/itkAnisotropicDiffusionImageFilter.h:21,
from
ITK/Modules/Filtering/AnisotropicSmoothing/include/itkGradientAnisotropicDiffusionImageFilter.h:21,
from
src/Filtering/AnisotropicSmoothing/ComputePeronaMalikAnisotropicDiffusion/Code.cxx:22:
/.../ITKExamples-build/ITK/Modules/Core/FiniteDifference/include/itkDenseFiniteDifferenceImageFilter.hxx:
In instantiation of
'void itk::DenseFiniteDifferenceImageFilter<TInputImage, TOutputImage>::CopyInputToOutput()
[with TInputImage = itk::Image<unsigned char, 2>; TOutputImage = itk::Image<float, 2>]':
src/Filtering/AnisotropicSmoothing/ComputePeronaMalikAnisotropicDiffusion/Code.cxx:72:1:
required from here

ITK/Modules/Core/FiniteDifference/include/itkDenseFiniteDifferenceImageFilter.hxx:44:35:
error: conversion from 'itk::SmartPointer<itk::Image<float, 2> >::ObjectType*
{aka itk::Image<float, 2>*}' to non-scalar type
'itk::Image<unsigned char, 2>::Pointer
{aka itk::SmartPointer<itk::Image<unsigned char, 2> >}' requested

typename TInputImage::Pointer tempPtr =	output.GetPointer();
                              ^~~~~~~

raised at:
https://open.cdash.org/viewBuildError.php?buildid=7083938

PR Checklist

• No API changes were made (or the changes have been approved)
• No major design changes were made (or the changes have been approved)

[jhlegarreta]

Note that this was triggered by an ITKExamples build:
https://open.cdash.org/build/7083938

The same blaster.kitware machine builds ITK sources nightly; not sure why the error is not being reported in those.

[N-Dekker]

@jhlegarreta Interesting 🤔 Do I understand correctly that the example tries to convert a itk::Image<float, 2> pointer to a itk::Image<unsigned char, 2> pointer type? That doesn't look right, does it?

For the record, the compilation error must have been introduced with pull request #2360 commit 5b3fc71 "STYLE: Remove 6 no-op dynamic_casts (casting T* to T*) from Modules/Core".

[dzenanz]

Have you looked closer at the sensibility of that cast? Are we just hiding some problem underneath the dynamic cast?

[jhlegarreta]

For the record, the compilation error must have been introduced with pull request #2360 commit 5b3fc71 "STYLE: Remove 6 no-op dynamic_casts (casting T* to T*) from Modules/Core".

👍 Thanks. That explains why this appeared only recently:
https://open.cdash.org/build/7077668
vs:
https://open.cdash.org/build/7079738

@jhlegarreta Interesting 🤔 Do I understand correctly that the example tries to convert a itk::Image<float, 2> pointer to a itk::Image<unsigned char, 2> pointer type? That doesn't look right, does it?

Thanks for the heads-up @N-Dekker. Yes, you are right/the error description says so. Having a look at the code triggering the error, those are indeed the pixel types defined for the input and output images (files are defined here and the example can be visually inspected here). I might be missing something, but I do not see anything wrong in the cast from the moment that the class is templated over its input and output image types. But please correct me if I'm wrong.

[dzenanz]

You are right, that pointer is only used for comparing pixel container.

[N-Dekker]

Thank you Jon! 👍

[jhlegarreta]

@Leengit thanks for the proposal. If people think that the option you proposed is less error prone, strictly more appropriate and more readable, I would not mind changing it, but I do not have strong arguments for/against.

[N-Dekker]

While I think the current code is just fine, here is yet another alternative:

if (std::equal_to<const void *>{}(output->GetPixelContainer(), input->GetPixelContainer()))
{
  // the input and output container are the same - no need to copy
  return;
}

You see, tempPtr is not really necessary, as it is just output.GetPointer(), and output is a smart pointer 😃 Moreover, it was checked a few lines before that output is not null! And std::equal_to<const void *> allows comparing PixelContainer pointers of different types.

[jhlegarreta]

@N-Dekker I'm fine if what you are proposing is the solution, but am not willing to push force that immediately to later find another alternative. So I will wait for a couple of days before adopting that change and push forcing to leave time for discussion if people find downsides to it.

[jamesobutler]

I ran into a compile issue with SimpleITK's current master building against ITK's current master seemingly due to the changes in 5b3fc71 as well. The SimpleITK build appears to have succeeded after applying the current changes proposed in this PR.

I'm currently testing to upgrade the ITK version used in 3D Slicer.

[issakomi]

So I will wait for a couple of days before adopting that change and push forcing to leave time for discussion if people find downsides to it.

Sure, thank you for looking at the issue, i am waiting, can not build project with ITK master branch

[N-Dekker]

@jhlegarreta

So I will wait for a couple of days before adopting that change and push forcing to leave time for discussion if people find downsides to it.

Thanks for your patience 😃 Again, I think the current commit 57e5f81 is just fine. Given the comments from James (@jamesobutler) and Mihail (@issakomi), I would prefer to have it merged today already. Would that be OK to you as well?

We could always discuss possible further style improvements afterwards, right?

[jhlegarreta]

@jamesobutler and @issakomi thanks for the report and @jamesobutler for giving the fix a try.

We could always discuss possible further style improvements afterwards, right?

@N-Dekker Please feel free to submit your proposal as a PR. Thanks for the effort to make the code even more modern and robust.