OHIF / Viewers

OHIF zero-footprint DICOM viewer and oncology specific Lesion Tracker, plus shared extension packages
https://docs.ohif.org/
MIT License
3.35k stars 3.37k forks source link

Segmentations are misplaced in v3.7.0, it worked fine in v3.6.0 #3754

Closed pavel-kaiko closed 11 months ago

pavel-kaiko commented 1 year ago

Describe the Bug

Hello,

We faced the bug when segmentations weren't placed where they should be. We tried to use a different slice order, but it didn't help. In the provided example, there are two segmentations with regular and reversed order.

We create the segments based on the Segmentation class from highdicom library, so if there is a bug on ReferencedSeriesSequence, it would be there. This OHIF issue regarding the misplacement also mentions this library.

Example data: 2023.10.27 - Segmentation alignment issue files

To check that it worked as expected before.

  1. Open the Viewer v3.6.0
  2. Load the slides and segmentations
  3. Open them in Basic Viewer and apply the regular segmentation.

Steps to Reproduce

  1. Upload the provided data to OHIF Viewer v 3.7.0+. It can even be https://viewer-dev.ohif.org/local. There are two versions of segmentation in the dicom_seg_cielab folder – regular and reversed.
  2. Apply one of the versions in Basic Viewer or Segmentation mode.

The current behavior

The segments will be applied to the improper slides.

Screenshot 2023-10-30 at 17 23 24

The expected behavior

  1. Open the Viewer v3.7.0+
  2. Load the slides and segmentations
  3. Open them in Basic Viewer/Segmentation mode and apply the regular segmentation.
  4. Segmentation should be applied properly

Expected result:

Screenshot 2023-10-30 at 17 23 45

OS

macOS 14.1

Node version

v18.18.2

Browser

Version 118.0.5993.117 (Official Build) (arm64)

Vlad-C-Crisan commented 1 year ago

Colleague of @pavel-kaiko here. Following a recommendation and follow-up questions we received from previous office hours, we tried to use THIS SCRIPT for converting the files to DICOM-SEG.

HERE you will find both the input data and the script used for conversion.

To reproduce, download the data locally and run the notebook to recreate the segmentation file. The result can be displayed correctly in v3.6.0, but in v3.7.0 the order is broken.

03shek commented 12 months ago

Thought I'd chime in while I have 3D Slicer open. The example data Pavel provided seems to load properly for "reversed_order" in 3D Slicer. The "normal_order" did not in 3D Slicer. In OHIF v3.7.0+, the SEGs still look wrong indeed for both orders.

  1. [LOOKS RIGHT] reversed_order in 3D Slicer: image

  2. [LOOKS WRONG] reversed_order in OHIF v3.7.0+: image

I would've expected OHIF to match the 3D Slicer view

fedorov commented 12 months ago

@CPBridge FYI

You could also try converting using dcmqi: https://github.com/QIICR/dcmqi.

CPBridge commented 12 months ago

Highdicom author here. I am open of course to there being a bug in highdicom though I will say that I find that possibility increasingly unlikely: I have spent a lot of time in that part of the codebase recently and checked the logic many many times.

Couple of thoughts from me:

sedghi commented 11 months ago

Adding another point to the awesome explanation by Chris. I remember back in the day, the simpleITK toolings (potentially other toolings too) had the axis reversed. Is this happening for you?

See these

@CPBridge

I hope that OHIF does not use the information therein to order the frames.

We don't rely on ReferencedSeriesSequence really, in fact we throw an error if it is not available (shouldn't we? since it is Type 1) But during reading we just use PerFrameFunctionalGroupSequence

Vlad-C-Crisan commented 11 months ago

@sedghi we shared in the above comment a script that creates the DICOM segmentations, based on highdicom. As mentioned in that comment and also illustrated by @03shek

image

sedghi commented 11 months ago

Seems like the segmentation appears on the first slice and it should be on the lung as we saw in the office hours. So it might not be flipping but matching series for filling in

CleanShot 2023-11-23 at 10 41 03

pavel-kaiko commented 11 months ago

Hi @sedghi. As you recommended, we did git bisect to find where breaking changes were introduced. This is this PR: https://github.com/OHIF/Viewers/pull/3577 ~But what is more interesting is that breaking changes are already present on the Netlify deployment preview: https://deploy-preview-3576--ohif-dev.netlify.app.~

pavel-kaiko commented 11 months ago

One more addition. In v 3.7.0+, segmentation is reversed and starts from the beginning or the end of the study (depending on the order), ignoring the original offset (in comparison to 3.6.0 or Slicer3D)

sedghi commented 11 months ago

I found the issue which is we assumed wrongfully that the DICOM SEG always have all the slices of the referenced series, but it is a wrong assumption. I fixed it but it has some performance issues, which i will try to address and create a PR

CleanShot 2023-11-30 at 15 12 16

pavel-kaiko commented 11 months ago

Thank you, @sedghi, for resolving the issue and for your excellent work. I upgraded the Cornerstone library, and it is now functioning as expected.