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

Tue Nov 16 16:59:10 EST 2010

Gaëtan,

I'll bet we can find a backward compatible solution. I'll try to find
some time to look into it. Better we spend time than our customers.

Remind me again the important bug/issue you are trying to resolve.

Bill

2010/11/16 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>:
>
> 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
>
>