-
-
Notifications
You must be signed in to change notification settings - Fork 705
BUG: Fix case where DICOM series order has negative spacing #5357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
BUG: Fix case where DICOM series order has negative spacing #5357
Conversation
relates to SimpleITK/SimpleITK#2292 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good after a quick look.
0ba9197
to
cb4f83f
Compare
I update the test case to use the sample MRI series in ITK test data, and use the ReverseOrder flag. The test is compare the pixel values at the physical locations match. |
Maybe we could have both of those tests? |
The DICOM tags were not correct in the prior example test case where the DICOM files were manually created. And oddly the X,Y spacing was being swapped for some reason likely related to the TAGs. Also this test case for the file has a non identify cosine matrix, which helps verifies the correct column was multiplied and not a row by -1. |
Thanks @blowekamp I'm just trying to understand the issue. It reminds me of an ITKElastix issue reported by @thewtex: Which appeared related to ITK/Modules/IO/ImageBase/include/itkImageFileReader.hxx Lines 222 to 235 in b375cef
|
The original issue was reported in SimpleITK here: The cause appeared to be that the z-slice direction was negative. This was more easily reproduce by enabling the "ReverseOrder" flag. It may very well be likely related to this change too: ITK/Modules/IO/ImageBase/include/itkImageFileReader.hxx Lines 222 to 235 in b375cef
It's not clear to me if the original DICOM reported in SimpleITK had negative spacing. The "ReverseOrder" reproduction of the issue, has negative inter-slice distance. There is certainly areas that need further investigation. The principle used in the test, that for DICOM if the "reverse order" flag is set then the physical location of the voxel should be maintained, should be correct. But DICOM is complicated. |
If the spacing between slices is negative, then direction cosine matrix was not correct with reguards to the sign.
cb4f83f
to
d46d322
Compare
This patch does not directly deal with the DICOM tags, and specific DICOM information. It only correct a sign change in the DirectionCosine matrix when detected to maintain consistent physical spacing when the order is reversed. The specifics of multi-frame DICOM are not handled here. If there is a specific DICOM series that should be tested, we can manually do that to ensure consistency. Otherwise, I think this patch should be good? |
@thewtex @hjmjohnson is there any update on this PR? Would be great to get this issue resolved as it's affect pre-processing pipelines! |
{ | ||
for (unsigned int j = 0; j < TOutputImage::ImageDimension; ++j) | ||
{ | ||
direction[j][this->m_NumberOfDimensionsInImage] *= std::signbit(dirN[j]) ? -1.0 : 1.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, I do not think this is necessarily 1.0, -1.0.
If the spacing between slices is negative, then direction cosine matrix was not correct with reguards to the sign.
PR Checklist
Refer to the ITK Software Guide for
further development details if necessary.