-
Notifications
You must be signed in to change notification settings - Fork 223
DOC: Add C#, C++ and java versions to the DicomConvert example. #2336
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
Conversation
3dd8083 to
61b65e7
Compare
| // MONOCHROME2 (minimum should be displayed as black) we don't need to | ||
| // do anything, if image has MONOCHROME1 (minimum should be displayed as | ||
| // white) we flip the intensities. This is a constraint imposed by ITK | ||
| // which always assumes MONOCHROME2. |
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.
So the goal here is that if the input in monochrome1 then to write the output as monochrome1? The important part to note here is that ITK inverts the intensity on read, but if output is monochrome1 then it does not automatically invert the intensity. (bug?)
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.
The script is converting the DICOM content to another file format which already requires intensity modifications from a high dynamic range to a low one. Because of the ITK convention of inverting MONOCHROME1 when reading, we need to invert it again so that when the file is saved it is visually similar to the original DICOM when displayed. I don't think this is a bug, it is the ITK convention and the desire to have consistent appearance of the converted image with the original DICOM content.
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.
I think I understand the logic less now.
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.
If the DICOM tag is MONOCHROME1 the DICOM image is displayed by DICOM viewers with low values as white/light and high values as black/dark. When ITK reads the image it inverts the intensity values.
When we convert the image to png/jpg etc. we want to maintain the same visualization as you would get viewing the original DICOM image in a DICOM viewer, so low values should be white and high values black. This means we need to invert the ITK image intensities.
| imageSize.push_back(static_cast<unsigned int>(iSize[2])); | ||
| } | ||
| } | ||
| reader.SetExtractSize(imageSize); |
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.
You are always reading the whole image. All these conditional are just to convert a single size image to 3D->2D. I would just read the image. Check is dimension, then if dim==3 then use the extract image filter to reduce the dimension. This should be more compact and skip this messiness with types.
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.
Sounds good, will replace the usage of the reader's setExtractSize with the extract image filter in the strongly typed languages.
0d5ab8c to
67b3b59
Compare
Provide the example in three additional languages.
| r-version: 'latest' | ||
| - name: Install Examples R package dependencies | ||
| if: contains(matrix.ctest-cache, 'WRAP_R') | ||
| run: | |
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.
This change set does not contain any R code changes. Is the a missing dependency with the R example?
blowekamp
left a comment
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.
The Examples look good. Although the change to the Build.yml looks off topic.
Provide the example in three additional languages.