ImageAdaptor should have its own ComputeOffset, ImageRegionIterator support broken

[N-Dekker]

Description

ImageRegionIterator does not work well with ImageAdaptor. For example, the following for loop appears to hang forever, because the iterator never reaches the end:

for (ImageRegionIterator iterator(adaptor, region); !iterator.IsAtEnd(); ++iterator)
{
  iterator.Set(42);
}

Steps to Reproduce

#include "itkImage.h"
#include "itkImageAdaptor.h"
#include "itkImageRegionIterator.h"
#include <gtest/gtest.h>

TEST(ImageAdapter, SupportsRegionIterator)
{
  class MyPixelAccessor
  {
  public:
    using ExternalType = int;
    using InternalType = int;

    static void
    Set(int & output, const int & input)
    {
      output = input;
    }

    static int
    Get(const int & input)
    {
      return input;
    }
  };

  using ImageType = itk::Image<int>;
  const auto image = ImageType::New();

  const auto adaptor = itk::ImageAdaptor<ImageType, MyPixelAccessor>::New();

  adaptor->SetImage(image);
  adaptor->SetRegions(itk::Size<>{ 4, 4 });
  adaptor->Allocate();

  const itk::ImageRegion region{ itk::Index<>{ 1, 1 }, itk::Size<>{ 2, 2 } };

  // Loops "forever" on a small (2x2) region!
  for (itk::ImageRegionIterator iterator(adaptor, region); !iterator.IsAtEnd(); ++iterator)
  {
    iterator.Set(42);
  }
}

Expected behavior

The for loop should be executed quickly, for such a small region.

Actual behavior

The for loop takes forever (endless loop).

Reproducibility

100%

Versions

main branch, commit c29a8e1

Additional Information

It appears that ImageRegionConstIterator::Increment() gets the wrong value from m_Image->ComputeOffset(ind), here:

ITK/Modules/Core/Common/include/itkImageRegionConstIterator.hxx

Line 67 in fcee3e5

this->m_Offset = this->m_Image->ComputeOffset(ind);

I think this should be solved by adding a ComputeOffset member function to itk::ImageAdaptor, which simply calls ComputeOffset on its internal image (not on its base class)!

[N-Dekker]

Looks like this issue has been there for a very, very long time already!

Some historical facts:

• ImageAdaptor was introduced by commit ab81321, Luis Ibanez (@luisibanez), Nov 12, 2000. It had both ComputeIndex and ComputeOffset, implemented by calling the corresponding member function of the internal image.
• Both ComputeIndex and ComputeOffset were then removed by commit bea33f4, Jim Miller (@millerjv), Feb 21, 2001. Instead, ImageAdaptor inherited these member functions from its base class, ImageBase, from then.
• Luis Ibanez added ComputeIndex back to ImageAdaptor with 9526fc8, Apr 17, 2001, calling the corresponding member function of the internal image instead of depending on its base class, ImageBase. Could it be that he might have intended to add ComputeOffset back to ImageAdaptor as well...? 🤷
• The very first version of ImageRegionIterator, commit 0ace998, Jim Miller, Aug 10, 2000 already called m_Image->ComputeOffset( ind ) in its operator++.

[N-Dekker]

Note that for an ImageAdaptor, the offset table of its internal image (adaptor->m_Image->m_OffsetTable) may be different from the offset table of its base class (adaptor->ImageBase::m_OffsetTable). Looks like that caused the unexpected values from adaptor->ComputeIndex(index)