MantisBT - ITK
View Issue Details
0009622ITKpublic2009-09-30 08:152010-10-26 05:49
Daniele E. Domenichelli 
Mathieu Malaterre 
normalmajoralways
acknowledgedreopened 
ITK-3-16 
 
0009622: ImageSeriesWriter + GDCMImageIO loses information about direction cosines
When directional cosines are copied from 3D volume to 2D slices by ImageSeriesWriter, only the upper-left 2x2 matrix can be copied, due to the dimension of m_Direction matrix.
Then GDCMImageIO sets zeros in the missing values of the first 2 lines of the matrix.
This is not a problem if the directional cosines matrix is the identity matrix, otherwise DICOM tag (0020,0037) Image Orientation (Patient) of the output DICOM files is not valid.
I suggest that an ITK_Direction entry containing Direction Matrix could be added in MetaDataDictionary. In this way there is no need to change the behaviour of this filter and ImageIOs have an easy way to access this information.

The attached patch add an "ITK_Direction" entry in MetaDataDictionary and read this tag in GDCMImageIO
No tags attached.
related to 0007748closed Mathieu Malaterre itk::GDCMImageIO::Write() Image Orientation (Patient) 
diff ITK_Direction.diff (3,873) 2009-09-30 08:15
https://public.kitware.com/Bug/file/2499/ITK_Direction.diff
diff ITK_Direction2.diff (3,874) 2009-09-30 10:41
https://public.kitware.com/Bug/file/2501/ITK_Direction2.diff
patch 3dseriesimage_vs_2d.patch (7,524) 2009-12-03 09:07
https://public.kitware.com/Bug/file/2702/3dseriesimage_vs_2d.patch
patch ITK_Direction4.patch (4,702) 2010-04-20 06:27
https://public.kitware.com/Bug/file/3039/ITK_Direction4.patch
Issue History
2009-09-30 08:15Daniele E. DomenichelliNew Issue
2009-09-30 08:15Daniele E. DomenichelliFile Added: ITK_Direction.diff
2009-09-30 08:30Mathieu MalaterreStatusnew => assigned
2009-09-30 08:30Mathieu MalaterreAssigned To => Mathieu Malaterre
2009-09-30 10:41Daniele E. DomenichelliFile Added: ITK_Direction2.diff
2009-09-30 10:44Daniele E. DomenichelliNote Added: 0017843
2009-12-03 06:26Mathieu MalaterreNote Added: 0018687
2009-12-03 06:27Mathieu MalaterreRelationship addedrelated to 0007748
2009-12-03 06:31Mathieu MalaterreNote Added: 0018688
2009-12-03 06:32Mathieu MalaterreNote Added: 0018689
2009-12-03 06:32Mathieu MalaterreStatusassigned => closed
2009-12-03 06:32Mathieu MalaterreResolutionopen => fixed
2009-12-03 09:02Mathieu MalaterreAssigned ToMathieu Malaterre =>
2009-12-03 09:02Mathieu MalaterreNote Added: 0018690
2009-12-03 09:02Mathieu MalaterreStatusclosed => feedback
2009-12-03 09:02Mathieu MalaterreResolutionfixed => reopened
2009-12-03 09:07Mathieu MalaterreFile Added: 3dseriesimage_vs_2d.patch
2009-12-17 14:27Daniele E. DomenichelliNote Added: 0018942
2009-12-17 14:28Daniele E. DomenichelliNote Edited: 0018942
2009-12-17 14:28Daniele E. DomenichelliNote Edited: 0018942
2009-12-17 14:28Daniele E. DomenichelliNote Edited: 0018942
2009-12-30 17:35Mathieu MalaterreNote Added: 0019016
2010-04-20 06:27Daniele E. DomenichelliFile Added: ITK_Direction4.patch
2010-04-20 06:32Daniele E. DomenichelliNote Added: 0020272
2010-05-26 03:43Mathieu MalaterreStatusfeedback => assigned
2010-05-26 03:43Mathieu MalaterreAssigned To => Mathieu Malaterre
2010-10-26 05:49Mathieu MalaterreNote Added: 0022700
2010-10-26 05:49Mathieu MalaterreStatusassigned => acknowledged

Notes
(0017843)
Daniele E. Domenichelli   
2009-09-30 10:44   
Reading in the discussion of bug 0007748 I understand that

> The short hand explanation should be that the member m_Direction of
> itk::GDCMImageIO (inherited from itk::ImageIOBase) is a vector that
> points to vectors, where the first vector contains the direction
> cosines of the image rows, the second the direction cosines of the
> image columns and the third contains the direction cosines of the
> image slices. So, m_Direction[0][0], m_Direction[0][1],
> m_Direction[0][2] are the direction cosines of the image rows.
> itk::GDCMImageIO::m_Direction stores the direction cosines as rows.

Then in the first patch m_Direction is transposed

ITK_Direction2.diff corrects this bug
(0018687)
Mathieu Malaterre   
2009-12-03 06:26   
> ITK_Direction2.diff corrects this bug

Hum, since 0007748 is now fixed, I believe I need to apply patch ITK_Direction.diff

I'll do that ASAP.

Thanks
(0018688)
Mathieu Malaterre   
2009-12-03 06:31   
$ cvs ci -m"BUG: 0009622 Thanks to Daniele E. Domenichelli for providing the patch" ~/Kitware/Insight
Committer: Mathieu Malaterre <mathieu.malaterre@gmail.com>
Running style check
Running style check
Processing itkGDCMImageIO.cxx



Running style check
Processing itkIOCommon.cxx



Running style check
Processing itkIOCommon.h



Running style check
Processing itkImageSeriesWriter.txx



/cvsroot/Insight/Insight/Code/IO/itkGDCMImageIO.cxx,v <-- Code/IO/itkGDCMImageIO.cxx
new revision: 1.168; previous revision: 1.167
/cvsroot/Insight/Insight/Code/IO/itkIOCommon.cxx,v <-- Code/IO/itkIOCommon.cxx
new revision: 1.30; previous revision: 1.29
/cvsroot/Insight/Insight/Code/IO/itkIOCommon.h,v <-- Code/IO/itkIOCommon.h
new revision: 1.28; previous revision: 1.27
/cvsroot/Insight/Insight/Code/IO/itkImageSeriesWriter.txx,v <-- Code/IO/itkImageSeriesWriter.txx
new revision: 1.34; previous revision: 1.33
(0018689)
Mathieu Malaterre   
2009-12-03 06:32   
After ITK 3.16
(0018690)
Mathieu Malaterre   
2009-12-03 09:02   
Patch attached to bug tracker was not tested.

Reverting changes done to CVS.
(0018942)
Daniele E. Domenichelli   
2009-12-17 14:27   
(edited on: 2009-12-17 14:28)
> Hum, since 0007748 is now fixed, I believe I need to apply patch
> ITK_Direction.diff

Actually I believe that 0007748 corrects just the way direction cosines
are written (see [1]) and not the internal representation, that seems
to be transposed compared to itkImage (see Goethe comment 0014265 for
bug 0007748)

> Patient Orientation: 0/0/-1/1/0/0
>
> GDCMImageIO::m_Direction:
>
> 0 0 -1
> 1 0 0
> 0 -1 0
>
> Image::m_Direction:
>
> 0 1 0
> 0 0 -1
> -1 0 0
>
> The conversion/inversion from one to another is done by
> ImageFileReader, ImageFileWriter.


That means that:

 - the patch to apply was ITK_Direction2.diff that transposes
   direction matrix when readed by GDCMImageIO.

or

 - to keep coherence with ImageFileWriter so that the conversion is
   done by itkImageSeriesWriter, ITK_Direction.diff should be applied
   but line 49

   + directionMatrix[i][j] = direction2[i][j];

   should be replaced with

   + directionMatrix[j][i] = direction2[i][j];

   (but I just realised this, so I didn't try it yet)


Anyway, as far as I understand this should not be the cause of
segmentation faults.


Applying ITK_Direction2.diff I don't get segmentation fault on tests
failing here [2].
itkImageReadDICOMSeriesWriteWithDirection003Test still fails, but it
fails also without the patch
(itkImageReadDICOMSeriesWriteWithDirection002Test is also failing
without the patch)

----

$ ctest -R itkImageReadDICOMSeriesWriteTest -V
[...]
1/1 Test 0000943: itkImageReadDICOMSeriesWriteTest ... Passed 7.59 sec

----

$ ctest -R itkImageReadDICOMSeriesWriteWithDirection002Test -V
[...]
1/1 Test 0000944: itkImageReadDICOMSeriesWriteWithDirection002Test ... Passed 0.43 sec

----

$ ctest -R itkImageReadDICOMSeriesWriteWithDirection003Test -V
[...]
945: ERROR: Origin values do not match
945: Original image origin = [0, 0, 0]
945: Recovered image origin = [0, 164, 0]
945: ERROR: found pixel value difference at index [1, 0, 0]
945: Original pixel value = 2
945: Recovered pixel value = 1
1/1 Test #945: itkImageReadDICOMSeriesWriteWithDirection003Test ...***Failed 0.27 sec

----



P.S. line 59 of 3dseriesimage_vs_2d.patch

- else if( key == ITK_XDirection )

should be

- else if( key == ITK_ZDirection )

Anyway I don't understand why renaming it should avoid segmentation
fault, maybe I'm missing something...


[1]http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/IO/itkGDCMImageIO.cxx?r1=1.161&r2=1.161.2.1&pathrev=ITK-3-16&sortby=date&root=Insight [^]
[2]http://www.cdash.org/CDash/viewTest.php?onlyfailed&buildid=485280 [^]

(0019016)
Mathieu Malaterre   
2009-12-30 17:35   
[from the top of my head]

>Anyway I don't understand why renaming it should avoid segmentation
>fault, maybe I'm missing something...


Because you need to call v.resize(3) before doing v[2]. This is what was happening. Since keys are ordered by name ITK_Direction would come before ITK_NumberOfDimensions, then the vector had an improper size.

Just do some simple debugging and you'll see (print the size of the vector/matrix)

Cheers
(0020272)
Daniele E. Domenichelli   
2010-04-20 06:32   
Hi, I attached a new patch (ITK_Direction4.patch) that resumes all changes.
I've been using this for a while and it works for me.
(0022700)
Mathieu Malaterre   
2010-10-26 05:49   
I will apply patch and check again. Thanks !