Skip to content
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

Segment Misplacement #2833

Closed
PHonest opened this issue Jun 29, 2022 · 16 comments
Closed

Segment Misplacement #2833

PHonest opened this issue Jun 29, 2022 · 16 comments
Labels
Awaiting Data to Reproduce The maintainers have requested anonymized data to reproduce the issue. IDC:candidate Possible feature requests for discussion, and if we agree, they can be relabeled IDC:priority

Comments

@PHonest
Copy link

PHonest commented Jun 29, 2022

Hello,
I am trying to enable the visualization of segmentations. So far everything went smoothly.
I am converting segment masks with highdicom in a dicom segmentation format. When I visualized those segmentations in the viewer, I noticed that the segmentations are displayed in the wrong plane. Nonetheless, the inplane position seems to be correct.
Additionaly I am only noticing this behaviour on the first segment of each segmentation. The first one gets placed upfront on the first plane. The second one seems to be in the right plane.
Please let me know if more information is needed.

Thanks in advance,
Paul

@fedorov
Copy link
Member

fedorov commented Jun 29, 2022

It may help if you provide the link to the highdicom code you use to do the conversion. It is not clear if this is a problem with OHIF Viewer, highdicom, or the conversion code you wrote that uses highdicom. It would also be highly desirable if you could share a sample image series and the accompanying segmentation that allow reproduction of the issue. You can make for any public dataset if you are concerned about sharing your own data.

cc @hackermd @CPBridge

@PHonest
Copy link
Author

PHonest commented Jun 30, 2022

I will try to provide some examples.

I could already locate the problem:

    seg_dataset = hd.seg.Segmentation(
        source_images=image_datasets,
        pixel_array=mask,
        segmentation_type=hd.seg.SegmentationTypeValues.BINARY,
        segment_descriptions=[description_segment_1, description_segment_2],
        series_instance_uid=hd.UID(),
        series_number=5,
        sop_instance_uid=hd.UID(),
        instance_number=1,
        manufacturer='Manufacturer',
        manufacturer_model_name='Model',
        software_versions='v1',
        device_serial_number='Device XYZ',
        series_description='Segmentation',
    )

This is the highdicom class for converting a mask (given a source image) to a segmentation dicom file. The default behaviour is to drop all empty slices. The structure of the segmentation can be reconstructed by the tag PerFrameFunctionalGroupsSequence, which holds the SOP for the correlating slice in the source image.
If you upload a sparse segmentation dcmjs builds the reconstructed labelmap for you.
There is a lazy loading mechanism to find the correlating frames in the function at (https://github.com/dcmjs-org/dcmjs/blob/c209047861d2ef6c17526c6d1e29d5db7e895b29/src/adapters/Cornerstone/Segmentation_4X.js#L604):

let frameSourceImageSequence = undefined;
    if (SourceImageSequence && SourceImageSequence.length !== 0) {
        frameSourceImageSequence = SourceImageSequence[frameSegment];
    } else if (PerFrameFunctionalGroup.DerivationImageSequence) {
        let DerivationImageSequence =
            PerFrameFunctionalGroup.DerivationImageSequence;
        if (Array.isArray(DerivationImageSequence)) {
            if (DerivationImageSequence.length !== 0) {
                DerivationImageSequence = DerivationImageSequence[0];
            } else {
                DerivationImageSequence = undefined;
            }
        }

        if (DerivationImageSequence) {
            frameSourceImageSequence =
                DerivationImageSequence.SourceImageSequence;
            if (Array.isArray(frameSourceImageSequence)) {
                if (frameSourceImageSequence.length !== 0) {
                    frameSourceImageSequence = frameSourceImageSequence[0];
                } else {
                    frameSourceImageSequence = undefined;
                }
            }
        }
    }

As long as a SourceImageSequence is provided along the segmentation it won't look up the correct position, buy place the segment planes along the first n planes of the stack. n would be the sum of all segment planes.

A quick fix would be to never omit empty slices.

@Punzo
Copy link
Contributor

Punzo commented Jun 30, 2022

dmcjs logic for parsing segmentation is the following:

  1. first it checks if SourceImageSequence, if yes, it can either:
    A) use getImageIdOfReferencedFrame , in which the sopInstanceUid and frameNumber are cross-checked between the source image stack and the SourceImageSequence info (NOTE: frameNumber for the first slice is 1, i.e. https://github.com/dcmjs-org/dcmjs/blob/c209047861d2ef6c17526c6d1e29d5db7e895b29/src/adapters/Cornerstone/Segmentation_4X.js#L1413)
    B) if frame number info is missing, it will use only the sopInstanceUid getImageIdOfReferencedSingleFramedSOPInstance

  2. if the SourceImageSequence is missing, it uses the following parameters from the segmentation frame metadata: FrameOfReferenceUID, SeriesInstanceUID and ImagePositionPatient to place the segmentation frame on the source frame (i.e.,
    getImageIdOfSourceImagebyGeometry)

@Punzo
Copy link
Contributor

Punzo commented Jun 30, 2022

A quick fix would be to never omit empty slices.

empty slices should not be an issue, but my guess is that for your data dcmjs is using the logic (1A) and although empty slices are allowed, the frameNumber has to be built on the full images stack indexes

@CPBridge
Copy link

Hi all, I am a highdicom developer who has worked extensively on the segmentation code. I have actually encountered this behaviour myself before also. Though unfortunately we never had time to dig into the root cause to get far enough to file an issue. It would be great if we could work together to figure out where the problem lies, and I'm happy to help out as I can. We believe that the segmentations that we are producing are correct, although of course we are open minded there being a mistake in highdicom. A few things to note:

  • From memory, I believe this occurs in the situation where you have discontinuities within a single "segment" of the segmentation mask. E.g. within one segment there are a few slices containing segmented regions, then some slices that contain no pixels within that segment, then a few more slices that contain segmented regions. I.e. missing slices above or below the segmentation are not a problem, but missing slices within it are a problem (although I believe this is permissible per the standard). Again this is all from memory and we should probably confirm.
  • You can always optionally keep the empty slices in the segmentation by passing omit_empty_frames=False to the constructor of Segmentation. This may allow @PHonest to work around this until a true fix is in place.
  • I don't see any issue with the way that @PHonest is using the highdicom code assuming that the ordering of the datasets in image_datasets and the slices in mask match each other. So probably this is a bug in the viewer or in highdicom (and I suspect it's the viewer)

@PHonest would it be quick for you to confirm that this occurs if and only if there are discontinuities within a single segment for your application?

@PHonest
Copy link
Author

PHonest commented Jun 30, 2022

@CPBridge and @Punzo thank you for your help. I will evaluate everything and keep you up to date.
@CPBridge so far I can tell you, that there are no discontinuities in our example set. The test segmentation I used consists of 2 segments and there are no gaps between the slices of the two segments

@CPBridge
Copy link

Does the issue only occur when there are multiple segments?

@PHonest
Copy link
Author

PHonest commented Jun 30, 2022

No, I also saw this behavior when I visualized cases with a single segment

@CPBridge
Copy link

FYI this method contains the logic that highdicom uses to reconstruct the segmentation mask for a list of source images when allowing for empty frames and multiple segments. It seems to be working for us so far. Might be worth checking that this gives the expected result for your case @PHonest . Then at least we'd know that highdicom is internally consistent, though it wouldn't rule out an incorrect (but internally consistent) implementation of the standard. I'm afraid I have no javascript experience so I'm finding it difficult to figure out what exactly the dcmjs code is doing differently

@Punzo
Copy link
Contributor

Punzo commented Jun 30, 2022

if you think it is a bug at dcmjs level, it would be great if you can share public available data. we have few cases similar to yours in IDC, but the dcmjs logic works for those. However, it is true that dcmjs parses the indexes in 3 different ways (1A, 1B and 2 #2833 (comment)) and we may have never tested one of those with empty segmentation frames.

@fedorov
Copy link
Member

fedorov commented Jun 30, 2022

Yes, we need a sample.

Most of the segmentations in IDC were generated using dcmqi. We absolutely must make sure highdicom segmentations are valid, and if they are valid (which I do expect that are!) - that they work with OHIF. If we confirm this is an issue with OHIF/dcmjs, I am all for labeling this issue as a priority for IDC, and we can prioritize resources to fix it.

@fedorov fedorov added the IDC:candidate Possible feature requests for discussion, and if we agree, they can be relabeled IDC:priority label Jun 30, 2022
@mlaves
Copy link

mlaves commented Jun 30, 2022

I have the very same issue with OHIF viewer and want to add that Slicer3d with the QuantitativeReporting plugin renders the segmentation from highdicom correctly. I also think that the bug is in the OHIF viewer.

@fedorov
Copy link
Member

fedorov commented Jun 30, 2022

That is good to know, thank you for adding this note @mlaves! QuantitativeReporting is using dcmqi for loading DICOM segmentations.

@fedorov
Copy link
Member

fedorov commented Jun 9, 2023

This is a v2 legacy issue, no?

@sedghi
Copy link
Member

sedghi commented Oct 13, 2023

Can you try again in the latest master branch? There are has been a lot of improvements in Segmentation handling

Try viewer-dev.ohif.org instead of viewer.ohif.org
Our viewer.ohif.org is deployed from release branch while viewer-dev.ohif.org is our master branch
Read more about branch explanations here https://docs.ohif.org/development/getting-started#developing

@sedghi
Copy link
Member

sedghi commented Oct 13, 2023

Please re-open the issue when you have the data for us to test

@sedghi sedghi closed this as completed Oct 13, 2023
@sedghi sedghi added the Awaiting Data to Reproduce The maintainers have requested anonymized data to reproduce the issue. label Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Data to Reproduce The maintainers have requested anonymized data to reproduce the issue. IDC:candidate Possible feature requests for discussion, and if we agree, they can be relabeled IDC:priority
Projects
None yet
Development

No branches or pull requests

6 participants