[Insight-developers] std::vector inide ImportImageContainer

[Henning Meyer]

Hello Jim,

Thank you for your explanations. That makes things more clear.

But...I think ITK should not stick to the old mistakes.
As far as I understood the API of ITK would not change, when we would
rewrite ImportImageContainer to use a generic random accessible
container. All we had to do is to rewrite the depending code (
Iterators and IO).
I'm not really sure about it, but it seems, that when using
std::vector< Pixeltype, SpecialAllocator> this could even be used to
wrap ITK around images generated outside ITK.
We could also use some more advanced containers like boost::multi_array....

I also think, that the iterators should be subclassed, but this could
wait until a major version change ( ITK 3.0?).


Another question: Why did you use valarray instead of vector?

Henning

2006/9/12, Miller, James V (GE, Research) <millerjv at crd.ge.com>:
> Henning,
>
> Originally, we had two ImageContainer classes.  One that used
> std::valarray and one that used raw pointers.  The version using
> std::valarray was used for images that ITK created itself.  The version
> using raw pointers was used for images created outside of ITK and
> "wrapped" to look at an ITK image.  The latter is frequently used when
> integrating ITK into an existing imaging applications.  This is a very
> important capability of ITK.
>
> When we had both ImageContainer classes, Image was templated over the
> type of container. We decided to reduce the number of template
> parameters to Image, and disposed of the std::valarray version since the
> version using raw pointers can be used for both cases (ITK created
> images as well as images created outside of ITK and wrapped to be ITK
> images).
>
> That is the legacy.
>
> The design flaw in the ImageContainer class and the Image class is that
> we (I) exposed the pointer to the underlying buffer. At the time, it
> seemed benign but this mistake has hindered ITK from natively supporting
> images with different memory layouts.  An ITK image (implicitly) has to
> be contiguous in memory.  A lot ITK machinery (IO, Iterators, etc.)
> implicitly rely on this fact.
>
> From a design standpoint, we would have been better off keeping the API
> to the ImageContainer abstract.  We should have used methods like
> Get(index)/Set(index, value) to modify pixels, presenting an abstract
> row-major order memory layout.  Under the hood, an ImageContainer could
> have the data organized any manner of ways: sparse images, slice
> contiguous images (the pixels for each slice contiguous in memory but
> slice to slice would not have to be contiguous), disk based images,
> compressed/run length encoded images, etc.
>
> By the time we (I) realized the magnitude of this mistake, it was too
> late to fix.  I still dream about returning to the ImageContainer,
> removing the GetBufferPointer() method, and modifying all the iterators
> to use the abstract API to index into the ImageContainer.  The
> RegionIterators would be easy to modify since they always operate on an
> offset from a base address.  The RegionIteratorsWithIndex iterators
> would require a bit of rework. The NeighborhoodIterators and the
> ShapedNeighborhoodIterators would require a lot of work.  Then a few
> things in IO would need to be fixed up as well.
>
> The downside to these changes (aside from the effort) is that in order
> to keep the rest of ITK backward compatible, we'd probably have to use
> virtual functions, which will slow down pixel access in an image.
>
> Another design that I have thought about is to use traits to select an
> appropriate iterator type for a given image.  So instead of using
> ImageRegionIterator<TInputImage> to define a region iterator inside of
> an algorithm, the code would read TInputImage::RegionIterator.  This
> would allow for iterators specific to the type image (dense, sparse,
> disk, etc.).  This would completely break people's code. Bad.  But it
> would make ITK look more like STL.
>
> (On a side note, we did not make the iterators inner classes of the
> image class because we could not get the compilers to compile an image
> class with the breadth of iterators ITK uses to compile when the
> iterators are defined as inner classes.)
>
> So, there you have it. A design decision made in the year 2000 and the
> desire to maintain backward compatibility is tying our hands.
>
> Jim
>
>
>
>
>
>
> -----Original Message-----
> From: insight-developers-bounces+millerjv=crd.ge.com at itk.org
> [mailto:insight-developers-bounces+millerjv=crd.ge.com at itk.org] On
> Behalf Of Henning Meyer
> Sent: Monday, September 11, 2006 6:09 AM
> To: insight-developers at itk.org
> Subject: [Insight-developers] std::vector inide ImportImageContainer
>
> Hello,
>
> I'm wondering why ImportImageContainer does not use std::vector and its
> iterator.
> Instead everywhere I see raw pointers and allocations via new.
> What was the reason for this decision?
> Is it feasable to modify everything to support std::vector (or even a
> generic random accessable container)?
> Where would be the difficult points? What would the drawbacks be?
>
> I would really like to do this, because then I could have an itk::Image<
> bool, 3 > why simply uses just one bit per pixel instead of 8 bits (due
> to partial specialization inside STL for std::vector< bool >).
> Also I think it could be quite easy then to create sparse image support
> ( with std::map< OffsetType, PixelType > inside the container ).
>
> What do you think about this?
> When using std::vector there shouldn't be any performance drawbacks...
>
> Henning
> _______________________________________________
> Insight-developers mailing list
> Insight-developers at itk.org
> http://www.itk.org/mailman/listinfo/insight-developers
>