PixelAccessor or NeighborhoodAccessor?

Engineering
1

Last Tuesday I implemented another modern C++ range of iterators: ImageRange should support iterating over all pixels of an image buffer. (Still work in progress – WIP.)

Now I wonder if I chose the right accessor to access the pixels of the image. I chose the one returned by itk::Image::GetPixelAccessor(). My implementation of iterator::operator*() retrieves a reference to the pixel pointed to by iterator::m_CurrentPixel by doing m_PixelAccessor.Get(*m_CurrentPixel). Is that OK? It seems to me that this approach does not support vector images (itk::VectorImage). The accessor returned by itk::VectorImage::GetPixelAccessor() appears incompatible with the one from itk::Image.

Should I use ITK’s NeighborhoodAccessor instead, as in ShapedImageNeighborhoodRange?

1 Like
2

Awesome, @Niels_Dekker!! :sparkles:

What happens when itk::VectorImage is added to the unit test?

3

Thanks @matt.mccormick. An attempt to use Experimental::ImageRange<itk::VectorImage<int>> triggers compilation errors:

modules\core\common\include\itkimagerange.h(160): error C2660: 'itk::DefaultVectorPixelAccessor<int>::Get': function does not take 1 arguments
modules\core\common\include\itkdefaultvectorpixelaccessor.h(78): note: see declaration of 'itk::DefaultVectorPixelAccessor<int>::Get'

So am I using the wrong pixel accessor here, or am I just using the pixel accessor (from itk::Image::GetPixelAccessor()) in the wrong way, within the ImageRange class? :thinking: A more interestingly question is, of course: how to do it right?

4

Have you looked at the TImage::AccessorFunctorType typedef? It is used with the ImageConstIterator.

Will a similar Proxy design pattern for the return “reference” work here too?

1 Like
5

Thanks @blowekamp ! I see now, AccessorFunctorType can be used in a more generic way than my initial attempt, which only used AccessorType (as returned by GetPixelAccessor()). And indeed, it looks like the Get and Set of AccessorFunctor can be used within a “reference-like” proxy class, as with the internal PixelProxy from ShapedImageNeighborhoodRange.

Still I wonder why there are different accessors for image-based pixel iteration and neighbor iteration. Do you know why? Would it be OK to use the NeigborhoodAccessor to iterate over the whole image?

6

OK: just committed a new version of ImageRange that does support itk::VectorImage:

Still Work In Progress (WIP) though. When ready, I think I’ll squash the WIP commits into one.

The new commit applies the Proxy design pattern, using TImage::AccessorFunctorType, as suggested by @blowekamp Please have a look!

With this commit, ImageRange::iterator::operator*() returns a proxy, instead of a “real” reference to the current pixel. This is necessary for itk::VectorImage, but not for a regular itk::Image. With the next version I would like operator*() to return a proxy for itk::VectorImage, and a reference for a regular itk::Image.

1 Like
7

Hey @Niels_Dekker, this looks great.
Related with this and other patches in this same direction, I assume using Accessors is going to hit performance (also it is stated in a comment here). Do you think is doable to use SFINAE to only use accessors for VectorImages and such, and to avoid them for “regular” images?

I am not familiar with the internal use of ImageAdaptors, but it might be worth to make the iterators as zero-cost as possible without sacrificing type flexibility. Pinging @blowekamp

8

Thatnks for your encouraging reply, Pablo!

I think it would be possible, at compile-time, possibly even without SFINAE. :slight_smile:

In pseudo-code:

if TImage::PixelType is TImage::InternalPixelType
and TImage::AccessorType is DefaultPixelAccessor
and TImage::AccessorFunctor is DefaultPixelAccessorFunctor
then
  operator*() should return a real reference to the pixel
else
  operator*() should return a "pixel proxy"

Just have to make sure that that’s indeed the right condition for choosing between a reference and a proxy!

1 Like
9

@phcerdan Please review how I implemented that logic for ImageRange, to only use accessors for VectorImages and such, and to avoid them for “regular” images:

The compile-time constant bool SupportsDirectPixelAccess specifies whether ImageRange<TImage> supports direct pixel access for this particular image type (TImage): itkImageRange.h#L86 For regular image types, direct pixel access is supported, and the accessor is avoided.

The range has an OptionalAccessorFunctor, which is empty (EmptyAccessorFunctor) when SupportsDirectPixelAccess == true. Otherwise, it is equal to the AccessorFunctor of the specified image. itkImageRange.h#L93

The OptionalAccessorFunctor is initialized by AccessorFunctorInitializer, which converts to either an EmptyAccessorFunctor or a properly initialized AccessorFunctor. itkImageRange.h#L485

So you see, I tried to avoid any overhead from those accessors when SupportsDirectPixelAccess is true.

iterator::operator*() returns a reference to the pixel when SupportsDirectPixelAccess == true, and otherwise, it returns a PixelProxy object. itkImageRange.h#L327 (And PixelProxy does use the AccessorFunctor.)

I hope it’s clear enough!

Pull request opened by @dzenanz: https://github.com/InsightSoftwareConsortium/ITK/pull/245

Update, Dec 1st, 2018: The latest ImageRange pull request (that should supersede the previous ones) is here:

2 Likes