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

Fix - Make the DICOM SR be able to load #4685

Closed

Conversation

ChaoStraxcorp
Copy link
Contributor

@ChaoStraxcorp ChaoStraxcorp commented Jan 14, 2025

Context

  • The DICOM SR file cannot be loaded in a viewport when loading data locally, as reported in GitHub Issue #4630. The error message is shown:
    Screenshot 2025-01-14 at 11 12 59 pm
  • This issue occurs in both the master branch and version 3.9.2 but does not appear in version 3.9.1.
  • This bug was introduced in [PR #4554]](fix(multiframe): metadata handling of NM studies and loading order  #4554 ). In [1], the change is made to handle multi-frame images. As a result, instead of using the imageId as the key of the data mapping, the latest update uses frameImageId (imageId&Frame=##). It leads to the metadataProvider.getUIDsFromImageID being unable to get the image UID by the image ID without the frame number.

Changes & Results

  • In [2], ensure that the imageId includes the frame number.

What are the effects of this change?

  • The local DICOM SR file will now load correctly into the viewport.

Testing

  • run yarn install
  • run yarn run dev
  • open localhost:3000/local
  • select a study with DICOM SR and load the data in the basic viewer
  • double click the DICOM SR, the DICOM SR shall be able to be loaded in the viewport.

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

Tested Environment

  • OS:
  • Node version:
  • Browser:

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for ohif-platform-docs ready!

Name Link
🔨 Latest commit 05a8a9c
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/67865a4160f41f0008db9d7e
😎 Deploy Preview https://deploy-preview-4685--ohif-platform-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.

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit 05a8a9c
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/67865a419f13090008de30cc
😎 Deploy Preview https://deploy-preview-4685--ohif-dev.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.

@ChaoStraxcorp ChaoStraxcorp changed the title append frame number at the end of image id Fix - Make the DICOM SR be able to load Jan 14, 2025
@sedghi
Copy link
Member

sedghi commented Jan 14, 2025

Awesome work, thanks, i'll test it

@sedghi
Copy link
Member

sedghi commented Jan 15, 2025

I would like to close this PR and i just add you as coauthor to PR i have in here #4693
is that ok?

@ChaoStraxcorp
Copy link
Contributor Author

Thank you! Your change makes more sense to me. I will close my PR.

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