[Insight-developers] itk::Index, itk::Offset, ... as subclass of itk::FixedArray?

Tue Nov 16 16:52:50 EST 2010

Le 16 nov. 10 à 22:25, Bill Lorensen a écrit :

> The backward compatibility issue is not good. We need a better  
> solution.

IMO, using C arrays was wrong and must be fixed. It could have been  
worth, but the C arrays are used in a quite small number of places.

I'd like to enhance the backward compatibility - that's why I'm  
looking for better constructors for Index Offset and Size - but I  
doubt a fully backward compatible solution can be found, especially  
for the return values.

I may also not found the best approach...

>
> 2010/11/16 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>:
>>
>> Le 16 nov. 10 à 16:56, Tom Vercauteren a écrit :
>>
>>> Hi all,
>>>
>>> On Tue, Nov 16, 2010 at 16:35, Luis Ibanez <luis.ibanez at kitware.com>
>>> wrote:
>>>>
>>>> Hi Gaetan,
>>>>
>>>> Probably only historical reasons...
>>>>
>>>> I don't see why we couldn't make the itk::Index and
>>>> the itk::Offset be derived classes of the itk::FixedArray.
>>>>
>>>> Note however that Dave Cole is working hard on
>>>> fixing the Windows 64 bits issues, so you may want
>>>> to coordinate those changes (since the itk::Index and
>>>> the itk::Offset are at the heart of the 64bits problem).
>>>>
>>>> --
>>>>
>>>> Regarding the intentional lack of a default constructor,
>>>> we may have to run these experiments again.  I would
>>>> suspect that it is still a valid issue, given that this relates
>>>> to making the itkFixedArray appear as plain-old-data,
>>>> instead of as a class. (there was also an issue of padding
>>>> if I remember correctly...)
>>>
>>> Indeed, currently it is still easy to access the memory help by an
>>>  itk::Image< itk::Vector<float,2>, 2 >
>>> by a simple
>>>  float * floatptr = image->GetBufferPointer()[0].GetDataPointer()
>>>
>>> Not sure if it is standard conformant since itk::Vector seems to be
>>> only POD like and not strictly POD but at least it works on the
>>> systems I have tested so far. This is important to avoid copying
>>> memory between different libraries.
>>>
>>> This works because itk::Vector inherits from itk::FixedArray and is
>>> also POD-like. The less POD it becomes the more fragile my  
>>> hypothesis
>>> becomes. Remember that we had to remove an empty destructor to make
>>> itk::FixedArray closer to being POD. This led to a better memory
>>> alignment and improved the performance of some of our code by a  
>>> factor
>>> 2:
>>>
>>> http://www.itk.org/mailman/private/insight-developers/2008-June/010444.html
>>>
>>> Note that I am not the only one caring about this:
>>> http://www.itk.org/pipermail/insight-users/2010-October/038525.html
>>>
>>> Anyhow, we may want to improve the documentation of itk::FixedArray
>>> with respect to that point.
>>>
>>
>> ok, so itk::FixedArray is the class tweaked for efficiency and  
>> which act
>> like POD.
>> And itk::FixedArray, unlike itk::Index, itk::Offset or itk::Size,  
>> also
>> provide the constructor I'm looking for.
>>
>>  http://www.itk.org/Doxygen318/html/classitk_1_1FixedArray.html
>>
>> So I guess it should be possible to implement those 3 classes as  
>> subclasses
>> of FixedArray, or at least to implement the same kind of  
>> constructor for
>> those classes.
>>
>>> My two cents,
>>> Tom
>>>
>>>>
>>>> What would be the argument in favor of adding the
>>>> default constructor ?
>>>>
>>>> (initialization ?,
>>>> note that we don't know what T is, in advance...
>>>> I'm not sure that we can provide a reasonable
>>>> initialization anyways.  T could be a std::string,
>>>> or a highly complex class...)
>>
>>  Backward compatibility.
>>
>> This is related to this change:
>>
>>  http://review.source.kitware.com/#change,374
>>
>> I had to modify a lot of tests to make it build. For example:
>>
>>  http://review.source.kitware.com/#patch,sidebyside,374,1,Testing/ 
>> Code/Algorithms/itkNormalizedCorrelationImageMetricTest.cxx
>>
>> In that test, only the size parameter break, because itk::Size  
>> doesn't
>> provide the right constructor. The other parameters I have changed  
>> like the
>> spacing or the origin are not breaking anything, I think because  
>> their type
>> is derived from itk::FixedArray
>>
>> Regards,
>>
>> Gaëtan
>>
>>
>>>>
>>>>
>>>>     Luis
>>>>
>>>>
>>>> ------------------------------------------------------------------------
>>>> 2010/11/16 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>
>>>>>
>>>>> Hi,
>>>>>
>>>>> Is there a reason to have the basic array types itk::Index and
>>>>> itk::Offset, and maybe others, not implemented as a subclass of
>>>>> itk::FixedArray?
>>>>> Also, I see in the doc that no constructor are provided for  
>>>>> performance
>>>>> reasons. Is it still true with the compilers supported by itk 4?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Gaëtan
>>>>>
>>>>>
>>>>> --
>>>>> Gaëtan Lehmann
>>>>> Biologie du Développement et de la Reproduction
>>>>> INRA de Jouy-en-Josas (France)
>>>>> tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
>>>>> http://voxel.jouy.inra.fr  http://www.itk.org
>>>>> http://www.mandriva.org  http://www.bepo.fr
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Powered by www.kitware.com
>>>>>
>>>>> Visit other Kitware open-source projects at
>>>>> http://www.kitware.com/opensource/opensource.html
>>>>>
>>>>> Kitware offers ITK Training Courses, for more information visit:
>>>>> http://kitware.com/products/protraining.html
>>>>>
>>>>> Please keep messages on-topic and check the ITK FAQ at:
>>>>> http://www.itk.org/Wiki/ITK_FAQ
>>>>>
>>>>> Follow this link to subscribe/unsubscribe:
>>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Powered by www.kitware.com
>>>>
>>>> Visit other Kitware open-source projects at
>>>> http://www.kitware.com/opensource/opensource.html
>>>>
>>>> Kitware offers ITK Training Courses, for more information visit:
>>>> http://kitware.com/products/protraining.html
>>>>
>>>> Please keep messages on-topic and check the ITK FAQ at:
>>>> http://www.itk.org/Wiki/ITK_FAQ
>>>>
>>>> Follow this link to subscribe/unsubscribe:
>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>
>>>>
>>
>> --
>> Gaëtan Lehmann
>> Biologie du Développement et de la Reproduction
>> INRA de Jouy-en-Josas (France)
>> tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
>> http://voxel.jouy.inra.fr  http://www.itk.org
>> http://www.mandriva.org  http://www.bepo.fr
>>
>>
>> _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>>
>> Kitware offers ITK Training Courses, for more information visit:
>> http://kitware.com/products/protraining.html
>>
>> Please keep messages on-topic and check the ITK FAQ at:
>> http://www.itk.org/Wiki/ITK_FAQ
>>
>> Follow this link to subscribe/unsubscribe:
>> http://www.itk.org/mailman/listinfo/insight-developers
>>
>>

-- 
Gaëtan Lehmann
Biologie du Développement et de la Reproduction
INRA de Jouy-en-Josas (France)
tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
http://voxel.jouy.inra.fr  http://www.itk.org
http://www.mandriva.org  http://www.bepo.fr

-------------- next part --------------
A non-text attachment was scrubbed...
Name: PGP.sig
Type: application/pgp-signature
Size: 203 bytes
Desc: Ceci est une signature ?lectronique PGP
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20101116/25f92532/attachment.pgp>