[Insight-users] Fixing the Image Orientation issue for ITK 3.10

[Luis Ibanez]

  Code duplication will be close to impossible to maintain.

These 100 lines of code, existing in five different classes will
evolve and degenerate over time, to the point of becoming 
unrecognizable. Bugs will be fixed in some of the copies, while
other will be forgotten. (e.g. I didn't recall the LabelMap and
ComponentTree having a copy of this code).

I'll post the profiling results in a minute.


    Luis


-----------------------
Karthik Krishnan wrote:
> On Thu, Oct 16, 2008 at 1:23 PM, Luis Ibanez <luis.ibanez at kitware.com> wrote:
> 
>>Hi Karthik,
>>
>>Thanks for the clarification.
>>
>>It seems that we should move the Transform Index <--> Point
>>methods to the ImageBase class and have them to be shared
>>by the Image, OrientedImage and VectorImage.
> 
> 
> Or we could copy the methods over from OrientedImage into the
> VectorImage class, so as to make VectorImage also "oriented".
> 
> They are pretty small methods. The whole OrientedImage class in
> itself is less than 100 lines. I think virtualizing those methods might
> introduce a performance hit. I agree that profiling is a great idea.
> 
> Thanks a lot
> --
> karthik
> 
> 
> 
>>I'm profiling the overhead of the virtual call with respect
>>to the multiplications and additions that are currently
>>computed in these methods.
>>
>>Once we have timing numbers we can review how important is
>>for these methods to avoid being virtual, and whether a
>>different implementation can provide the desired behavior.
>>
>>
>>  Luis
>>
>>
>>-----------------------
>>Karthik Krishnan wrote:
>>
>>>Luis Ibanez wrote:
>>>
>>>
>>>>Hi Karthk, Gordon,
>>>>
>>>>
>>>>This problem has been solved since September 13th 2008.
>>>>
>>>>
>>>>Simply turn on the CMake flag:
>>>>
>>>>
>>>>         ITK_IMAGE_BEHAVES_AS_ORIENTED_IMAGE
>>>>
>>>>
>>>>That will make the itk::Image behave like the itk::OrientedImage.
>>>>
>>>>
>>>>Since the itk::VectorImage derives from the itk::Image,
>>>
>>>It derives from itk::ImageBase. If I recall correctly, at that time,
>>>deriving
>>>from itk::Image required me to make certain methods like
>>>TransfromPhyscalPointToContinuousIndex virtual. Same reason why
>>>OrientedImage derives from ImageBase and not Image.
>>>
>>>Thanks
>>>--
>>>karthik
>>>
>>>
>>>>then
>>>>the correct behavior will also be inherited.
>>>>
>>>>
>>>>  Thanks
>>>>
>>>>
>>>>     Luis
>>>>
>>>>
>>>>
>>>>-------------------------
>>>>Karthik Krishnan wrote:
>>>>
>>>>
>>>>>Dan, Gordon:
>>>>>
>>>>>As you've already realized Dan, itk::VectorImage provides no more (and
>>>>>no less)
>>>>>support for direction cosines than itk::Image. They simply ignore it and
>>>>>do not,
>>>>>unlike itk::OrientedImage, take it into account when translating indices
>>>>>into
>>>>>world coordinates, or use it when computing derivatives on images.
>>>>>
>>>>>NRRD, I believe, allows for encoding both the patient coordinate system
>>>>>(represented by space_origin and space_directions) and additional
>>>>>coordinate systems pertinent to DWI images (gradient coil's directions).
>>>>>Whatever direction cosines it will set on itk::Image or itk::VectorImage
>>>>>are
>>>>>ignored.
>>>>>
>>>>>In retrospect, I mistakenly derived VectorImage from ImageBase, rather
>>>>>than OrientedImage, but both classes came out the same year, when
>>>>>OrientedImage was still in its infancy with a CVS log stating
>>>>>"It may be removed be before the next release".
>>>>>
>>>>>As a solution, the VectorImage class could quite easily be made to
>>>>>take into account of the direction cosines exactly as OrientedImage
>>>>>does,
>>>>>by just copying the methods over from OrientedImage. Perhaps that
>>>>>could be on the list of things ToDo whenever the next lengthy
>>>>>discussion on Orientation comes up in ITK. I think we have one
>>>>>every month. :)
>>>>>
>>>>>Thanks
>>>>>
>>>
> 
> 
>