BUG: Get image geometry from GDCM Secondary Capture

[pieper]

Secondary capture images can have pixel spacing and image orientation, but the GDCM code explicitly ignored these values (whereas DCMTK code read them).

Per David Clunie (@dclunie) the editor of the standard, it was clarified in CP-586 that PixelSpacing is the correct place to store the best known value (e.g. one that has been corrected for distortion) while other fields like the NominalScannedPixelSpacing, which GDCM used before this commit, if stored at all should have the uncorrected values.

For these reasons this commit makes the default behavior of GDCM for SC match what it would do if SC images were MR or CT in terms of handling the image geometry, which is a better default than the previos behavior.

https://dicom.nema.org/medical/dicom/Final/cp586_ft.pdf

Fixes #4109

[issakomi]

IMHO, the idea to read IPP/IOP/PixelSpacing from the SecondaryCaptureImageStorage is good, there are many such data sets (they can be considered Extended Secondary Capture), but it should be done in GDCM or documented that the changes have to be re-applied after every GDCM update.

---

Image Position (Patient) should be updated here, otherwise it will be always 0,0,0 as before:

ITK/Modules/ThirdParty/GDCM/src/gdcm/Source/MediaStorageAndFileFormat/gdcmImageHelper.cxx

Line 580 in cdbbdf9

if( ms != MediaStorage::SecondaryCaptureImageStorage && ds.FindDataElement( timagepositionpatient ) )

---

Edit:

The change is the reason for failed tests, BTW

2023-07-15T21:18:21.4183809Z The following tests FAILED:
2023-07-15T21:18:21.4184368Z 	992 - itkGDCM_ComplianceTestRGB_JPEG2000-YBR_RCT (Failed)
2023-07-15T21:18:21.4184764Z 	993 - itkGDCM_ComplianceTestRGB_JPEGLS-RGB (Failed)
2023-07-15T21:18:21.4185148Z 	994 - itkGDCM_ComplianceTestRGB_losslessJPEG-RGB (Failed)
2023-07-15T21:18:21.4185534Z 	995 - itkGDCM_ComplianceTestRGB_lossyJPEG-YBR_FULL_422 (Failed)
2023-07-15T21:18:21.4185914Z 	996 - itkGDCM_ComplianceTestRGB_raw-RGB (Failed)
2023-07-15T21:18:21.4186271Z 	997 - itkGDCM_ComplianceTestRGB_raw-YBR_FULL (Failed)
2023-07-15T21:18:21.4186656Z 	998 - itkGDCM_ComplianceTestRGB_raw-YBR_FULL_422 (Failed)
2023-07-15T21:18:21.4187027Z 	999 - itkGDCM_ComplianceTestRGB_RLE-RGB (Failed)

because the PR does not read NominalScannedPixelSpacing (0018,2010). In fact, both, (0018,2010) Type 3 and (0028,0030) Type 1C, are valid for SC Image module. I am afraid that ignoring (0018,2010) is not the best way too.

Edit:

CC: @malaterre Please look at this post

Edit:

Probably it should work if both spacing attributes will be read for SC Image and (0028,0030) will have higher priority, if both are available.

[pieper]

it should be done in GDCM or documented that the changes have to be re-applied after every GDCM update.

Very good point! I wasn't sure how ITK interacts with the upstream repository and mostly wanted this change to go into Slicer's fork, but yes, coming up with the best conforming solution in GDCM makes good sense and then it can work its way into ITK and then Slicer.

Probably it should work if both spacing attributes will be read for SC Image and (0028,0030) will have higher priority, if both are available.

Yes, I interpreted CP 586 as saying that PixelSpacing is the preferred way to express the true value. It might make sense to have a fallback to look for one of the other spacing tags if PixelSpacing is missing, but it felt like an obscure corner case and I didn't see any easy way to restructure the code since I'm not familiar with it.

I agree with your other points @issakomi and would be happy to see a better implementation of this that covers more cases. Let's see if @malaterre or others have suggestions on how best to approach this.

[jcfr]

Waiting #4120 is finalized, I will integrate this path into our Slicer/ITK fork.

Then, once #4120 has been finalized, I will revert from our fork and implement the improve approach.

Thanks @issakomi for moving forward with a more robust approach.

[thewtex]

Superceded by #4521