[Insight-developers] std::vector inide ImportImageContainer

[Henning Meyer]

Hello,

I changed a bit inside my ITK sources, and now everything runs with
std::vectors inside ImportImageContainer. I still need to fix IO,
because they still need the raw pointer, but basically it works.

> I don't recall whether std::vector allows one to pass in the memory
> block to use for the vector.  Perhaps it can be done with a special
> allocator type.
Yes, that possible and shouldn't be very complicated.

> What are your thoughts for providing alternative pixel container types
> without requiring an API change to ITK?
My high level API did not change a bit. But now I could also use a
std::map based ImportImageContainer to support sparse images or RLE or
a combination of both.

> There doesn't seem to be much of a motivation to have
> ImportImageContainer use an std::vector (and modifying the rest of the
> system) unless these changes facilitate using pixel containers with
> alternative memory layouts (without modifying the API or imposing a
> performance penalty).

The shouldn't be any performance penalty for normal types used with
std::vector. May a small drop for small bool images, but maybe even a
gain for large bool images because of the better cache usage due to a
smaller memory footprint.


Where and how should I post my changes for reviewing?

Henning

2006/9/13, Miller, James V (GE, Research) <millerjv at crd.ge.com>:
> Henning,
>
> In the first draft of the STL standard, there was nothing that dictated
> that std::vector's had to be continguous memory.  Std::valarray, on the
> other hand, had to be contiguous memory.  It turned out that all the
> implementations of std::vector chose to make the memory contiguous.
>
> I don't recall whether std::vector allows one to pass in the memory
> block to use for the vector.  Perhaps it can be done with a special
> allocator type.
>
> What are your thoughts for providing alternative pixel container types
> without requiring an API change to ITK?
>
> There doesn't seem to be much of a motivation to have
> ImportImageContainer use an std::vector (and modifying the rest of the
> system) unless these changes facilitate using pixel containers with
> alternative memory layouts (without modifying the API or imposing a
> performance penalty).
>
> Jim
>
> -----Original Message-----
> From: Henning Meyer [mailto:tutmann at gmail.com]
> Sent: Wednesday, September 13, 2006 3:39 AM
> To: Miller, James V (GE, Research)
> Cc: insight-developers at itk.org
> Subject: Re: [Insight-developers] std::vector inide ImportImageContainer
>
> 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
> >
>