[Insight-developers] Small modification to ImportImageContainer

[Tom Vercauteren]

Hi Bill and Luis,

Thanks for the feedback. It might have taken some time to show up on
the dashboard but my second patch should have at least fixed the vs6
compilation issues:
http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Common/itkImportImageContainer.txx?root=Insight&r1=1.19&r2=1.20&sortby=date

See the last dash13 build
http://www.cdash.org/CDash/index.php?project=Insight&date=2009-03-30#Continuous


On Tue, Mar 31, 2009 at 02:49, Luis Ibanez <luis.ibanez at kitware.com> wrote:
> Tom,
>
> BTW,
> The same goes for the AllocateElements() method, and
> the argument of DeallocateElements().
>
> It seems that none of them should be const.
>
>
>       Luis
>
>
> --------------------------------------------------------------------------------------------------
> On Mon, Mar 30, 2009 at 8:44 PM, Luis Ibanez <luis.ibanez at kitware.com> wrote:
>> Hi Tom,
>>
>> Why is that  the method:
>>
>>     DeallocateElements()
>>
>> needs to be const ?
>>
>> It is only called from non-const methods,
>> and the very nature of the method is that it alters
>> the state of the class.
>>
>> Have you find an scenario where the const
>> signature is needed ?
>>
>>
>>     Thanks
>>
>>
>>          Luis
>>
>>
>> ----------------------------------------------------------------------------
>> On Mon, Mar 30, 2009 at 3:24 PM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
>>> Tom,
>>>
>>> Your changes are not compiling on VS6.
>>> http://www.cdash.org/CDash/viewBuildError.php?buildid=302665
>>> I don't have access to my VS6 system until next week. Perhaps Luis can helpout.
>>>
>>> Bill
>>>
>>> On Thu, Mar 19, 2009 at 9:14 AM, Tom Vercauteren
>>> <tom.vercauteren at m4x.org> wrote:
>>>> Thanks Bill for the very interesting pointer!
>>>>
>>>> I think the patch really makes sense in that exact same scenario. If I
>>>> get it right, the factory basically loads a TestImportImageContainer
>>>> which is defined in itkFactoryTestLib.[h/cxx].
>>>>
>>>> TestImportImageContainer inherits from ImportImageContainer and
>>>> redefines AllocateElements. IMHO, if allocation is redefined, it makes
>>>> sense to also redefine deallocation.
>>>>
>>>> By enclosing all deallocation in one virtual function, this is what
>>>> the patch allows. Thus enabling for example the use of a
>>>> std::allocator instead of calling new and delete.
>>>>
>>>> Tom
>>>>
>>>> On Thu, Mar 19, 2009 at 16:51, Bill Lorensen <bill.lorensen at gmail.com> wrote:
>>>>> Tom,
>>>>>
>>>>> Another possibility is to use the object factory mechanism to provide
>>>>> an override to ImportImageContainer. The test
>>>>> Testing/Code.Common/itkObjectFactoryTest2.cxx shows how
>>>>> ImportImageContainer can be overriden.
>>>>>
>>>>> Bill
>>>>>
>>>>>
>>>>> On Thu, Mar 19, 2009 at 7:51 AM, Tom Vercauteren
>>>>> <tom.vercauteren at m4x.org> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> At my company, we needed to have ITK images for which buffers are
>>>>>> ensured to be aligned (say on 8 bits). This can be done by overloading
>>>>>> itk::Image and itk::ImportImageContainer.
>>>>>>
>>>>>> However, it also requires a patch to ImportImageContainer for it to be
>>>>>> "overloadable". Basically, the attached patch only amounts to:
>>>>>> 1) Putting all the memory deallocation in a single virtual function
>>>>>> 2) Moving some member variables from private to protected
>>>>>>
>>>>>> There should be no backward compatibility issue with this. All unit
>>>>>> tests are still green on my computer.
>>>>>>
>>>>>>
>>>>>> By looking into ImportImageContainer, I also realized that Reserve is
>>>>>> wrong in the sense that it has a Resize semantics. This is similar to
>>>>>> the following VectorContainer and the MapContainer bug:
>>>>>> http://www.itk.org/Bug/view.php?id=2893
>>>>>>
>>>>>> Since it was decided not to fix bug 2893, I haven't fixed
>>>>>> ImportImageContainer::Reserve either but simply added some comments to
>>>>>> the file.
>>>>>>
>>>>>> Let me know if you see any potential problem with this patch.
>>>>>>
>>>>>> Thanks,
>>>>>> Tom
>>>>>>
>>>>>> _______________________________________________
>>>>>> Powered by www.kitware.com
>>>>>>
>>>>>> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.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
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>