[Insight-developers] Image memory model

[Karthik Krishnan]

On Fri, May 30, 2008 at 4:03 AM, Dan Mueller <dan.muel at gmail.com> wrote:
> Hi Karthik (and developers),
>
> Thanks for your detailed response regarding the implementation of a
> slice contiguous image. Sorry I have taken so long to write back, but
> it has been hard to find the time to work on this issue.
>
> Regarding the design of the pixel accessors: you're right, it's not
> "too good to be true", it is just true! What a wonderful design!
>
> I have used your ideas and now have a working
> itk::SliceContiguousImage into which I can import slices from
> non-contiguous memory for use with linear iterators (and filters which
> only use linear iterators). I still need to polish up the API and
> think about the neighborhood accessor, but things are looking good!

Great!.

> 1. What should SliceContiguousImage do with GetBufferPointer()?
> In my current design, GetBufferPointer() simply returns 0. I guess
> this is logical because there is no "buffer", rather an array of
> pointers to slices -- currently stored as std::vector< PixelType * >.
> With this design the new SliceContiguousPixelAccessor must be given
> access to both the array of slices and the buffered region size. I
> think this could be improved if GetBufferPointer() were to return the
> pointer to the list of slices rather than 0. Do you think this is too
> misleading (the name of the method no longer describes fully what is
> being returned)? If it is not too misleading, then the only extra
> information required by the pixel accessor is the buffered region
> size; it could recover the slices from the input pointer. I like this
> design, but it somewhat involves hijacking the GetBufferPointer()
> method...

Yes, You can return whatever is most meaningful to you using the
GetBufferPointer() method. (probably returning a pointer to the list
of slices).  Just mention it in the documentation.

As mentioned earlier, we cannot deprecate this method.


> 2. Is it misleading if SliceContiguousPixelContainer actually
> allocates slices in contiguous memory?
> The new container will allow users to import memory from
> non-contiguous slices, but what should it do if allocating the memory
> itself? I guess it is easier to allocate contiguous memory (ie. a
> single buffer in contiguous memory), as long as the user is aware
> there is no guarantee SliceContiguousImage has a contiguous memory
> footprint...

I agree.

> 3. Not all filters will work with the new SliceContiguousImage. How
> should this be indicated?

Why don't all filters work with the SliceContiguousImage ?
Anything that works with Image<T, N> should work with
SliceContiguousImage<T,N> right ?


> In the case of VectorImage, new vector-based filters had to be added
> because the image type did not work with some of the existing filters.

Yes. Unlike the SliceContiguousImage, not only is the memory model
different, the return type is different too.

Still, anything that works with itk::Image< Array<T>, 3> works with
VectorImage<T,3>, while providing a non-fragmented memory model. That
was the goal.

> This is (thankfully) not the case for SliceContiguousImage -- it
> *should* work with most existing filters/functions/etc. Some examples
> of potential non-interoperable filters include:
> RayCastInterpolateImageFunction

Why does RayCastInterpolateImageFunction not work ? You are overriding
 ComputeOffset(index) in the SliceContiguousImage to provide the
correct offset with respect to the first slice's buffer pointer right
?

> WatershedImageFilter,
> itkVTKImageExport,

> itkImageFileReader, itkImageFileWriter.

The ImageSeriesReader and ImageSeriesWriter should work for you
without modification right ? These are the ones that probably make
most sense of the SliceContiguousImage. It is probably best
represented in disk as well as a set of files.

These writers and readers instantiate standard ImageFileReaders and
ImageFileWriters on each slice.

These will already work fine for you, since a slice is represented as
a contiguous entity. So the WriteBuffer/ReadBuffer in ImageIOBase
which do a one shot write/read should not be of concern.


> Obviously
> it would be good to document and/or inform the user of incorrect
> usage. Perhaps an "\ingroup SliceContiguousImageFilter" tag can be
> added to supported filters? Could concept checking also be used to
> help guide the user?

Yes.

> That's all for now, although I might have a few more questions when I
> take a look at the neighborhood accessor.
>
> Thanks again, Dan

Feel free to propose API changes to the PixelAccessors and
PixelAccessorFunctors.
They are recent. None of the methods there are virtual.
And I bet no one is using them directly.


Thanks
-- 
Karthik Krishnan
R&D Engineer,
Kitware Inc.
Ph: 518 371 3971 x119
Fax: 518 371 3971