[Insight-developers] Image memory model

[Jim Miller]

I think the issue with this design is that the NeighborhoodIterators do not
go through the PixelAccessors (for efficiency).  In fact, the
NeighborhoodIterators keep a "neighborhood" or pointers to pixels and know
how to update those pointers via the iteration.

Introducing the GetBufferPointer() method is one of things I regret most in
the design of the ITK image. I never should have done that.  If all the
iterators worked of the concept of an "offset" from the first pixel (i.e.
the number of pixels since the beginning of the buffer), then they could
have indexed the pixels as PixelContainer[i] to get the ith pixel.  The
PixelContainer could then hide the memory layout from the Image class and
from the Iterators. This would even allow PixelContainers that internally
compressed the image data or store part of the image data on disk and part
in memory.

ImageRegionIterator can support such a referencing scheme easily.
ImageRegionIteratorWithIndex would require a little bit of work to support
the memory model.  NeighborIterator would require a good deal of work to
support such a model. These are all doable.  It is just a matter of
resources and a matter of efficiency (eg. do the neighborhood iterators get
too slow in this new model).

Outside of the Iterators, the IO mechanisms (including the Import/Export
facilities for interfacing to other systems), and few poorly written
filters, the use of GetBufferPointer() should be fairly limited and
isolated.

In practice, when I interface into systems that have a slice contiguous
model, I either use ITK just on 2D slices and forgo 3D processing, or I make
a volume contiguous image that is a copy of the slice contiguous image.
These are not the best solutions but todays 64 bit world is currently much
larger than medical image data set sizes.



On Sun, May 25, 2008 at 8:28 PM, Karthik Krishnan <
karthik.krishnan at kitware.com> wrote:

> Dan:
>
> On Sun, May 25, 2008 at 2:41 AM, Dan Mueller <dan.muel at gmail.com> wrote:
> >
> > Image::GetBufferPointer() is used extensively within the toolkit. Take
> > a look at the constructors for ImageConstIterator,
> > ConstNeighborhoodIterator, ImageReverseConstIterator,
> > ImageConstIteratorWithIndex, etc. I have added a list to
> >    http://www.itk.org/Wiki/Proposals:Slice_contiguous_images
> > however I realise now that some of these are
> > ImportImageContainer::GetBufferPointer() (I'll fix this up when time
> > permits).
>
> That should not be a cause of concern
>   image->GetBufferPointer() is just a pointer to the first pixel in
> the image. It is the dereferencing (m_BufferPointer + offset) that's
> of concern.
>
> >
> > Unfortunately, while your suggestion of using the PixelAccessor and
> > PixelAccessorFunctor would have been easy, I think it is too good to
> > be true.
>
> It is true.
>
> > The assumptions regarding contiguous memory allocation are
> > deeply entrenched.
>
> These classes were introduced a year or two ago, precisely because we
> had to support an alternate memory model image for NAMIC.
>
> >
> > Take for example the very first step you discussed, the act of passing
> > the input pixel value from the iterator.Get() function to the
> > PixelAccessorFunctor:
> >>   PixelType iterator.Get()   reads
> >>    { return pixelAccessorFunctor.Get(*(m_Buffer+m_Offset)); }
>
> The variable is passed by reference, however it is later dereferenced
> by the functor to the corrent pointer. The VectorPixelAccessor reads:
>
>  inline ExternalType Get( const InternalType & input, const unsigned
> long offset ) const
>    {
>    ExternalType output( (&input)+(offset*m_OffsetMultiplier) ,
> m_VectorLength );
>    return output;
>    }
>
> ie the offset is passed in and based on the vector length instead of
> returning *(m_Buffer+m_Offset) that an itk::Image would do, the
> itk::VectorImage returns *(m_Buffer + m_Offset * m_VectorLength ) .
>
> >
> > This deference *(m_Buffer+m_Offset) will access potentially invalid
> > memory in the slice-contiguous case (m_Buffer is the result of
> > m_Image->GetBufferPointer(), the start of the contiguous array, or the
> > start of the first slice for my case).
>
> I realize that it would have been better to pass in the offset and the
> buffer_start_pointer in, to avoid invalid de-references. (I think
> passing them by reference avoids the de-reference). So we could change
> the signature of the the accesssor to
>
>  inline ExternalType Get( const InternalType & begin, const unsigned
> long offset ) const
>
> and the iterator would read:
>
>  PixelType Get(void) const
>    { return m_PixelAccessorFunctor.Get(m_Buffer, m_Offset); }
>
> instead of
>
>  PixelType Get(void) const
>    { return m_PixelAccessorFunctor.Get(*(m_Buffer+m_Offset)); }
>
>
> > There are other places this assumption is also made: for example
> > ImageConstIteratorWithIndex::SetIndex() uses the buffer pointer to
> > compute the index.
>
> Again the starting pointer should not matter. All attempts to
> dereference the pointers, via Set or Get go through the accessors.
>
> >
> > Have I misinterpreted your meaning? I think either m_Buffer and/or
> > m_Offset would need to altered from their original meaning to work for
> > slice-contiguous memory...
>
> No. I don't think so. In your case,
>
> m_Buffer would be any arbitrary value say 0.
> m_Offset would be the Nth pixel into the image.
>
> Any attempt to access the Nth pixel would go through the accessor,
> which would have start pointers to all the slices in your image and
> would read something like:
>
>  inline ExternalType itk::SlicePixelAccessor::Get( const InternalType
> & begin, const unsigned long offset ) const
>    {
>    const unsigned long slice = offset/(x_size*y_size);
>    const unsigned long offset_into_slice = offset % (x_size*y_size);
>    return *(m_SlicePointer[slice] + offset_into_slice);
>    }
>
>
> --
> Hope this helps.
>
> Thanks
> --
> Karthik Krishnan
> R&D Engineer,
> Kitware Inc.
> _______________________________________________
> Insight-developers mailing list
> Insight-developers at itk.org
> http://www.itk.org/mailman/listinfo/insight-developers
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20080601/3d5cd541/attachment.htm>