ENH: Added IndexRange for efficient region and grid space iteration

[N-Dekker]

Added Experimental::IndexRange, to allow efficient iteration over
the indices of an image region or grid space by means of a C++11
range-based for-loop or Standard C++ algorithm. Added type aliases,
ImageRegionIndexRange<Dimension> and ZeroBasedIndexRange<Dimension>.

This class appears useful in combination with ShapedImageNeighborhoodRange, e.g.:

ImageRegionIndexRange<Dimension> indexRange{ imageRegion };

for (IndexType index : indexRange)
{
  shapedImageNeighborhoodRange.SetLocation(index);

  for (PixelType neighborPixel : shapedImageNeighborhoodRange)
    // Process neighbor pixel...
}

[N-Dekker]

This pull request is intended to supersede PR #106 from @hjmjohnson his Gerrit migration fork!

[N-Dekker]

I hope 🙏 that this one has SFINAE implemented correctly 😓

[N-Dekker]

SFINAE still not entirely correct. My bad! Should have read more carefully! https://stackoverflow.com/questions/46603353/sfinae-on-constructors-works-in-vc2017-but-not-in-clang-gcc Will try another fix!

[N-Dekker]

Update: the compilers appear to accept the patch now, including SFINAE, but KWSyle still complains:

2018-11-08T16:28:15.3739841Z 120: Test command: /home/vsts/work/1/s-build/KWStyle-build/KWStyle "-xml" "/home/vsts/work/1/s/CMake/../Utilities/KWStyle/ITK.kws.xml" "-v" "-o" "/home/vsts/work/1/s/Modules/Filtering/ImageFilterBase/ITKKWStyleOverwrite.txt" "-D" "/home/vsts/work/1/s-build/Modules/Filtering/ImageFilterBase/ITKKWStyleFiles.txt" "-gcc"
2018-11-08T16:28:15.3739942Z 120: Test timeout computed to be: 1500
2018-11-08T16:28:15.4533401Z 116: /home/vsts/work/1/s/Modules/Core/Common/include/itkIndexRange.h:311: error: Template definition (typename) doesn't match regular expression

I guess I should just add a dummy template parameter, to please KWSyle.

[hjmjohnson]

Please add an override statement for in the ITKKWStyleOverwrite.txt instead.

[N-Dekker]

Sorry @hjmjohnson I read your suggestion to modify "ITKKWStyleOverwrite.txt" too late. Already added a 'dummy' template parameter (TVoid). Which is not so bad anyway, as it documents the expected type (void) for this template argument: std::enable_if<!VIsSubstitutionFailure>::type should yield void, if anything at all.

[N-Dekker]

Update: the latest amend has an extra type alias, ZeroBasedIndexRange<Dimension>, optimized for zero-based index iteration.

[thewtex]

Sweet! 🍬

[thewtex]

This is extremely exciting @N-Dekker; for the C++ API, for the performance, and for the potential C++11 parallel STL algorithm applications! Awesome. 🏆

The implementation looks great, building on everything learned from ShapedImageNeighborhoodRange, zero-index optimization 👌, and well tested 🥇 .

For the naming 😇 , I am wondering if ImageRegionRange would be clearer for someone new to the toolkit? While we live in ITK all the time and think itk::Index when we see index, the "normal" C++ developers will likely think this is a array container linear index at first glance. What do you think?

[N-Dekker]

@thewtex Thanks for you kind words and the accompanied emoticons :-) I do agree that it might not be directly clear to every new ITK user that the index of IndexRange (and its type aliases ZeroBasedIndexRange and ImageRegionIndexRange) is N-dimensional. However, I guess ITK users just have to get used to it, as the term Index is used so frequently in ITK as being N-dimensional.

Also I think it's an important property of this class that it only yields index values (and not pixel values). So I would prefer to keep the term Index in there. Hope my reasoning makes sense to you :-)

[hjmjohnson]

I agree with @N-Dekker here.

[thewtex]

👍

[hjmjohnson]

@N-Dekker A few warnings on the dashboard showed up after this:
https://open.cdash.org/viewBuildError.php?type=1&buildid=5625083

[N-Dekker]

@hjmjohnson wrote:

A few warnings on the dashboard showed up after this:
https://open.cdash.org/viewBuildError.php?type=1&buildid=5625083

Thanks Hans, I think I can fix them easily (Linux-x86_64-gcc4.8-m32: "declaration of 'size' shadows a member" and "missing initializer for member").

Do you know why these warnings did not appear before? At least I did not see them at the commit https://github.com/InsightSoftwareConsortium/ITK/pull/155/commits or at the Linux build results from Azure https://dev.azure.com/itkrobotlinux/ITK.Linux/_build/results?buildId=194&view=logs

[hjmjohnson]

@N-Dekker

FYI: https://open.cdash.org/viewBuildError.php?type=1&buildid=5629918

Modules/Core/Common/include/itkIndexRange.h:302:46: warning: missing initializer for member 'itk::Index<1u>::m_InternalArray' [-Wmissing-field-initializers]
m_MaxIndex(CalculateMaxIndex({}, gridSize))
^
/.../ITK/Modules/Core/Common/include/itkIndexRange.h: In instantiation of 'itk::Experimental::IndexRange<VDimension, VBeginAtZero>::IndexRange(const SizeType&) [with unsigned int VDimension = 2u; bool VBeginAtZero = false; itk::Experimental::IndexRange<VDimension, VBeginAtZero>::SizeType = itk::Size<2u>]':
Modules/Core/Common/test/itkIndexRangeGTest.cxx:28:35: required from here