[Insight-developers] std::vector inide ImportImageContainer

[Miller, James V (GE, Research)]

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
>