Skip to content

Conversation

@zivy
Copy link
Member

@zivy zivy commented Jun 23, 2025

Provide the example in three additional languages.

@zivy zivy force-pushed the dicomConvert branch 21 times, most recently from 3dd8083 to 61b65e7 Compare June 28, 2025 18:06
@zivy zivy requested review from blowekamp and dave3d June 28, 2025 20:50
// 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.
Copy link
Member

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?)

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Member Author

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);
Copy link
Member

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.

Copy link
Member Author

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.

@zivy zivy force-pushed the dicomConvert branch 2 times, most recently from 0d5ab8c to 67b3b59 Compare July 3, 2025 17:24
Provide the example in three additional languages.
r-version: 'latest'
- name: Install Examples R package dependencies
if: contains(matrix.ctest-cache, 'WRAP_R')
run: |
Copy link
Member

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?

Copy link
Member

@blowekamp blowekamp left a 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.

@zivy zivy merged commit 9a02044 into SimpleITK:master Jul 6, 2025
7 checks passed
@zivy zivy deleted the dicomConvert branch July 6, 2025 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants