Skip to content

fix(SegmentationStateManager): fix loading in segmentation labelmaps #1871

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

tye-shanklin
Copy link
Contributor

Context

Fixes loading segmentation labelmaps. Before this fix, when loading in segmentation data from a DICOM SEG file, the Segmentation State Manager would always use the last overlay image, even though that would not correspond to most of the images in a Stack viewport. After this fix, the Segmentation State Manager uses the overlay for slice 1 when viewing slice 1 of a stack viewport, overlay slice 2 for slice 2 of the viewport, etc.

For more context and to see an example of what's broken, please refer to OHIF/Viewers#4789.

Changes & Results

I updated the Segmentation State Manager to reference the overlay image based on a slice's index when loading a segmentation DICOM file and populating the segmentation state. Since the overlay images are stored in-order based on the order of the slices, this appears to be a safe approach.

An example video of the fixes in action are attached to this PR.

Segmentation.Fixes.mp4

Testing

To test these changes, you can open the segmentation mode for a study in the OHIF Viewers project and mark up the stack with different labels, then export the segmentation as a DICOM SEG file and save it to a backing PACS server. When reopening the study and selecting the segmentation structured report, it should load the segmentations in order.

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • "OS: Windows 11
  • "Node version: 23.5.0
  • "Browser: Edge 133.0.3065.69

Copy link

netlify bot commented Mar 3, 2025

Deploy Preview for cornerstone-3d-docs ready!

Name Link
🔨 Latest commit 4917aeb
🔍 Latest deploy log https://app.netlify.com/sites/cornerstone-3d-docs/deploys/67c6183d598c620008d7cf39
😎 Deploy Preview https://deploy-preview-1871--cornerstone-3d-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment on lines +462 to +463
const labelForImageIndex = labelmapImageIds[currentImageIndex];
if (labelForImageIndex) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i always wondered why we don't do this instead of looping over the labelmapImageIds and finding the match.

@tye-shanklin
Copy link
Contributor Author

Hi @sedghi! Please take a look at this PR when you have a chance and let me know if you have any questions or if you know of a better approach. Thanks in advance.

@sedghi
Copy link
Member

sedghi commented Mar 13, 2025

can you provide an example (data anonymized) that currently showing the issue in our main branch. and also reproducible steps either here or in OHIF?

Thanks

@tye-shanklin
Copy link
Contributor Author

Hi @sedghi, please see the attached ZIP file that contains an anonymous patient's heart scan dicom files and a segmentation file, named "random_segmentation.dcm". I've tested using the OHIF viewers repo. You can perform the following steps:

  1. Start the web application using yarn run dev:orthanc (note: when preparing this guide, I noticed that the most recent commit on main has performance issues that prevent this segmentation from hydrating properly. I was able to get it to work by checking out commit 7a13c28eb3560466896bbf1c76fc178e02d96fee).
  2. Navigate to localhost:8042 (the othanc server) and load in the dicom files from the attached zip file.
  3. Navigate to http://localhost:3000/segmentation?StudyInstanceUIDs=1.2.276.0.7230010.3.1.2.0.1512.1708227708.873358 to view the series that you loaded in from step 2.
  4. In the left thumbnail series panel, double click the SEG segmentation report to load up the data. Click "Yes" to rehydrating the report.

Expectation: when scrolling through the stack of images, there should be a different segment overlay for each image.
Actual: each image in the stack uses the same segment overlay, being the last one in the SEG file.

Let me know if you need anything else. Thanks.

Anonymous Test Data.zip

@tye-shanklin
Copy link
Contributor Author

Hi @sedghi, following up to see if you've had a chance to take a look at this with the example data or if you have questions on how to use it.

Copy link
Member

sedghi commented Mar 19, 2025

Not yet, unfortunately. We're in the middle of a release, and time is really tight. I'll get to this, though.

@tye-shanklin
Copy link
Contributor Author

Closing this PR since some changes to the codebase seem to have resolved this issue without the need for my PR.

Copy link
Member

sedghi commented Apr 29, 2025

Could you clarify what key changes users would experience when searching and landing on this page?

@tye-shanklin
Copy link
Contributor Author

Sure. In a previous version of the cornerstonejs tools package, if you had a series of images for a particular slice of the body over time (e.g. a lung scan over 3 seconds) and unique labelmap segmentations for each image in the series, cornerstone would only show the labelmap segmentation for the last image, regardless of which image you are on. At some point between when I had opened this PR and when I closed it, cornerstonejs received a fix so that each labelmap segmentation corresponds to the correct image.

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.

3 participants