[Insight-developers] Rigid3DTransform

[Bill Lorensen]

They have transform files changed that have the Rigid3DTransfrom in them.



On Sun, Jan 2, 2011 at 12:26 PM, Luis Ibanez <luis.ibanez at kitware.com> wrote:
> Hi Bill,
>
>
> The class itk::Rigid3DTransform  is used in Slicer in the following files:
>
>
> 1) Base/Logic/vtkSlicerTransformLogic.cxx
> 2) Applications/CLI/BRAINSTools/BRAINSCommonLib/GenericTransformImage.cxx
> 3) Applications/CLI/ResampleVolume2.cxx
> 4) Applications/CLI/DiffusionApplications/ResampleDTI/ResampleDTI.cxx
> 5) Applications/CLI/DiffusionApplications/ResampleDTI/itkDiffusionTensor3DRigidTransform.txx
> 6) Applications/CLI/DiffusionApplications/ResampleDTI/itkDiffusionTensor3DRigidTransform.h
>
>
>
> The uses in (1) and (2) are now obsolete.
>
> They were registering all the Transforms factories in order to
> deal with the absence of the ScaleVersor3DTransform in ITK 3.18.
> Now that this transform is in ITK 3.20 (and ITKv4) the factory
> registration is no longer needed.
>
> So, lines 170-232 of  (1)
> Base/Logic/vtkSlicerTransformLogic.cxx
> should be removed.
>
> The same goes for lines 410-476 in (2)
> Applications/CLI/BRAINSTools/BRAINSCommonLib/GenericTransformImage.cxx
>
>
> -----
>
> The uses in (3) lines 890-925 is actually an improper use of
> polymorphism. The code is using dynamic_casts to uncover
> the type of a derived class, just to call on it a function that was
> virtual in the first place. This one actually calls for a refactoring
> of the GetInverse() method in ITK. We should discuss that one
> with Brian Avants as part of the Image Registration refactoring
> effort.
>
>
> The use in (3) lines 228 can be replaced with MatrixOffsetTransformBase,
> since, the code is using only the SetMatrix() and SetTranslation() methods
> that are actually defined in this base class.
>
>
> The use in (3) 287 and 318 can also be replaced with
> MatrixOffsetTransformBase. It is actually casting the rigid tranform
> to that base class anyways in line 323.
>
> Same goes for the uses in (3) in lines 328 and 330.
>
> The declaration in (3) line 501 is not used at all,
> so it should just be deleted.
>
> ----
>
> The uses in (4)
> Applications/CLI/DiffusionApplications/ResampleDTI/ResampleDTI.cxx
>
> The use in ines 471, does not really requires instantiation with New()
> The code is using a dynamic_cast in order to discover the type
> of an abstract pointer, in order to assign the same pointer to a
> MatrixOffsetTransformBase in line 478, and register a code "rt"
> in line 476. The check could have been done with a raw pointer.
>
> The same applies in 483-491.
>
> ----
>
> The uses in (5,6)
>
> 5) Applications/CLI/DiffusionApplications/ResampleDTI/itkDiffusionTensor3DRigidTransform.txx
> 6) Applications/CLI/DiffusionApplications/ResampleDTI/itkDiffusionTensor3DRigidTransform.h
>
> The file (6) only has declarations,
> The file (5) uses the type in:
>
> Lines 47-56, where it uses the transform as a MatrixOffsetTransformBase.
> That is, it only uses the SetMatrix(), SetTranslation() API.
>
> In lines 32-41, the transform is also used just as a MatrixOffsetTransformBase.
>
> So, essentially, the uses of Rigid3DTransform in (5,6) can be replaced
> with MatrixOffsetTransformBase.
>
>
> ------
>
> I'll be happy to help convert all the code above.
>
> ------
>
> However,
> the only unsolved case is:
>
>                                      IO
>
> since the transform writers put the class names in the files *AND*
> we do not use a file format version (which is the real problem)...
>
> Then,
> previously generated files, that contain the code "Rigid3DTransform"
> can't be read with ITKv4.
>
> The lesson to learn there is that we should introduce versioning
> in all the file formats over which we have some control.
>
> ----
>
> Of course,
> it is easier to restore the NewMacro in Rigid3DTransform,
> but then...
>
> We have to find an alternative way of preventing new users
> from instantiating that transform and trying to use it in their
> first registration example.
>
>
> Any suggestions ?
>
> - Throwing exceptions when they call SetParameters ?
> - Displaying warnings ?
>
>
>
>     Luis
>
>
> ------------------------------------------------------------------
> On Sat, Jan 1, 2011 at 12:23 PM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
>> Luis,
>>
>> As part of our Slicer4/ITK4 experiment, there are a number of compile
>> errors because Slicer4(3) instantiate Rigid3dTransform.
>>
>> Recall that we removes the itkNewMacro for the class in response to
>> bug 8320: http://public.kitware.com/Bug/view.php?id=8320
>>
>> The corresponding gerrit topic is here:
>> http://review.source.kitware.com/#change,504
>>
>> I believe that Slicer4(3)'s use of Rigid3DTransform is valid.
>> Slicer4(3) have saved transforms that are part of the test suite. Note
>> that in your comments on the bug tracker you state:
>> "It doesn't hurt to have a valid implementation of Get/SetParamters at
>> the level of the Rigid3DTransform. However, this class simply
>> shouldn't be used as a full fledged transform. (e.g. for example as
>> part of an image registration framework). "
>>
>> Note the word "shouldn't". Well, Slicer4(3) did.
>>
>> I suggest that we restore the itkNewMacro and resolve bug 8320 another
>> way. I think it is hard to justify such an API change in light of a
>> valid use in Slicer4(3).
>>
>> Bill
>>
>