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

itk::ImageRegionIterator<TImage>::Value() does not build with itk::VectorImage #157

Open
jmichel-otb opened this Issue Nov 8, 2018 · 7 comments

Comments

Projects
None yet
5 participants
@jmichel-otb
Contributor

jmichel-otb commented Nov 8, 2018

[Before submitting an issue, please check that your issue hasn't been already
filed]

Description

It seems that itk::ImageRegionIterator::Value() can not be used with itk::VectorImage. Same goes for itk::ImageScanLineIterator.

Steps to Reproduce

The following sample code does not compile:

#include "itkVectorImage.h"
#include "itkImageRegionIterator.h"

void test()
{
  using ImageType = VectorImage<double>;
  using RegionType      = typename ImageType::RegionType;
  using SizeType        = typename RegionType::SizeType;
  
  auto vimage = ImageType::New();
    
  SizeType size = {{200,200}};
  
  vimage->SetRegions(size);
  vimage->SetNumberOfComponentsPerPixel(2);
  vimage->Allocate();
  itk::VariableLengthVector<double> v(2);
  v.Fill(0);
  vimage->FillBuffer(v);

  itk::ImageRegionIterator<ImageType> it(vimage,vimage->GetLargestPossibleRegion());

  it.GoToBegin();

  // This does not compile
  auto a = it.Value();
}

Error from compiler (gcc 6.3):

/home/michelj/dev/local/itk/include/ITK-4.13/itkImageRegionIterator.h:123:12: error: non-const lvalue reference to type 'PixelType' (aka 'VariableLengthVector<double>') cannot bind to a value of unrelated type 'InternalPixelType'
      (aka 'double')
  { return *( const_cast< InternalPixelType * >( this->m_Buffer + this->m_Offset ) ); }
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: in instantiation of member function 'itk::ImageRegionIterator<otb::VectorImage<double, 2> >::Value' requested here
  auto a = it.Value();
              ^
1 error generated.
@N-Dekker

This comment has been minimized.

Contributor

N-Dekker commented Nov 8, 2018

Very properly reported, @jmichel-otb! And yes, I'm able to reproduce the compilation error. However, I think the Value() member function just isn't meant to support for this particular use case. As some ITK officials may confirm... Is it.Get() equally good to you?

@phcerdan

This comment has been minimized.

Contributor

phcerdan commented Nov 8, 2018

From here it.Value() does not support ImageAdaptors, use it.Get() instead of it.Value()

@phcerdan

This comment has been minimized.

Contributor

phcerdan commented Nov 8, 2018

I am not official at all though! @thewtex @dzenanz @blowekamp

@thewtex

This comment has been minimized.

Member

thewtex commented Nov 8, 2018

Well analyzed! 👁

Perhaps we should document that itk::VectorImage is not supported with .Value() in itkImageRegionConstIterator.h.

@blowekamp

This comment has been minimized.

Member

blowekamp commented Nov 8, 2018

As I recall the "Get()" method returns a VariableLength vector which is a "reference" to the VectorImage. That is the VariableLengthVector contains a pointer to the pixel in the vector image buffer. I don't believe it was intended to be modified, but is more efficient since if avoids a memory allocation.

@jmichel-otb

This comment has been minimized.

Contributor

jmichel-otb commented Nov 9, 2018

I ran into this while trying to workaround the fact that using outIt.Set(f(inIt.Get()); requires the allocation of a temporary VariableLengthVector when dealing with VectorImage as output. I wanted to directly set values in the allocated output buffer instead, and getting a non-const ref to it was looking good ;)

Long story short, I probably do not understand / need Value(), but since it is a public method from a class of a public API, and that VectorImage is obviously a valid template parameter for it, I think it should compile without error, hence my report.

@N-Dekker

This comment has been minimized.

Contributor

N-Dekker commented Nov 9, 2018

If you really want to, you could probably adjust itk::ImageConstIterator<TImage> to make the Value() member function unavailable for VectorImage, by means of SFINAE. Otherwise, maybe a static_assert within the function could clarify the situation...? I'm not volunteering :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment