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

Bill Lorensen bill.lorensen at gmail.com
Tue Nov 16 18:46:45 EST 2010 

I'm still confused. We have had wrapping for years. What has changed
to bring this problem to light? I admit that I have never use ITK
wrapping.

Please don't confuse the issue with method names in filters that are
inconsistent. I'm sure we have lots of those.

2010/11/16 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>:
>
> Le 16 nov. 10 à 22:59, Bill Lorensen a écrit :
>
>> 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.
>
> The principal problem is for the wrapping: those methods are simply not
> usable. This is something which must be fixed for ITK v4.
>
> This is because we don't know what the pointer is for. Is it an array? Of
> which size? It is a pointer to a buffer? Is it a pointer to a modifiable
> variable?
>
> Using stronger types, as done everywhere else in ITK, doesn't exhibit this
> kind of problem.
> Stronger types can also be more efficiently checked at compile time -- I'm
> really not sure why they haven't been used in the first place!
>
> One option to avoid the compatibility issue is to use another method name.
> Of course it won't be always possible to find another meaningful name, but
> it may be useful in some cases. For example, in PadImageFilter, the method
> names are not consistent with the methods name in CropImageFilter.
> Get/SetPadLowerBound() and Get/GetPadUpperBound() could be renamed to
> Get/SetUpperBoundaryPadSize() and Get/SetLowerBoundaryPadSize() and
> Get/SetPadLowerBound() and Get/GetPadUpperBound() reimplemented to use the
> new methods and still provide the same API.
>
> The constructors similar to the ones in itk::FixedArray would also help a
> lot, but not for return values.
> I'll continue to look that way, because I think that having to deal only
> with the return values, which are a lot less used, is acceptable.
>
> Gaëtan
>
>
>>
>> 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
>>>
>>>
>
> --
> 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
>
>

More information about the Insight-developers mailing list