Segment Misplacement - Githubissues

PHonest commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

Does the issue only occur when there are multiple segments?

PHonest commented 2 years ago

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

CPBridge commented 2 years ago

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 commented 2 years ago

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 https://github.com/OHIF/Viewers/issues/2833#issuecomment-1171216353) and we may have never tested one of those with empty segmentation frames.

fedorov commented 2 years ago

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.

mlaves commented 2 years ago

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 commented 2 years ago

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

fedorov commented 1 year ago

This is a v2 legacy issue, no?

sedghi commented 9 months ago

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 commented 9 months ago

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

OHIF / Viewers

Segment Misplacement #2833