[Insight-developers] Image memory model

[Karthik Krishnan]

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.