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

WIP fix(SR): Fix/sr SCOORD3D not loading #3631

Closed

Conversation

rodrigobasilio2022
Copy link
Collaborator

@rodrigobasilio2022 rodrigobasilio2022 commented Sep 4, 2023

Context

This PR is related to IDC issue #1215 and wants to enable OHIF to support SCOORD3D annotations and SR polyline annotations in IDC that didnt follow the OHIF / Cornerstone standards. It introduces the idea to customize the dicomSr extension by checking if 'dicomSrExtensionCustomizations' id is defined in a modeCustomization. If yes it checks if a convertAnnotation function is defined and execute it if it exists. This function will be responsible for converting the non conformant SRs to OHIF/Cornerstone compatible ones by rewriting the SR metadata. An example of such function is defined in here. ImagingDataCommons/ohif-idc-mode#3

Changes & Results

This PR adds checking inside dicomsr extension to use, if defined a function called converAnnotations that should perform the proper and necessary converstions to support non conformant SRs, in this case SCOORD3D point annotations and polyline annotations.

The changes only affect modes that register the dicomSRExtensionCustomizations in customizationServices.

Testing

To test it, one should create a new mode or modify the longitudinal mode following this PR ImagingDataCommons/ohif-idc-mode#3.

And need to setup a local dicomweb server and add the studies in the following dropbox link:
https://www.dropbox.com/s/98rylgt25b2sm9r/planar_annotations.zip?dl=0

And open the respective studies. Once opened, just click in the SR series, that originally has only two annotations, but one will see that ten new arrow annotations were added in the measurements tabel, were one can jump to all of them.

image

image

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 10
  • Node version: 16.14.0
  • Browser: Chrome 116.0.5845.97, Firefox 116.0.3

@netlify
Copy link

netlify bot commented Sep 4, 2023

Deploy Preview for ohif-platform-docs ready!

Name Link
🔨 Latest commit 24b4174
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/65784258d939e400088acc03
😎 Deploy Preview https://deploy-preview-3631--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.

@netlify
Copy link

netlify bot commented Sep 4, 2023

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit 24b4174
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/6578425881eea80008c9ab19
😎 Deploy Preview https://deploy-preview-3631--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.

@rodrigobasilio2022 rodrigobasilio2022 added the IDC:priority Items that the Imaging Data Commons wants to help sponsor label Sep 4, 2023
@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Merging #3631 (7d7d6a6) into master (8a335bd) will decrease coverage by 0.67%.
Report is 155 commits behind head on master.
The diff coverage is 4.25%.

❗ Current head 7d7d6a6 differs from pull request most recent head 24b4174. Consider uploading reports for the commit 24b4174 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3631      +/-   ##
==========================================
- Coverage   46.23%   45.57%   -0.67%     
==========================================
  Files          78       79       +1     
  Lines        1276     1299      +23     
  Branches      312      319       +7     
==========================================
+ Hits          590      592       +2     
- Misses        548      562      +14     
- Partials      138      145       +7     
Files Coverage Δ
platform/app/src/routes/WorkList/filtersMeta.js 0.00% <ø> (ø)
platform/core/src/utils/downloadCSVReport.js 0.00% <ø> (ø)
platform/core/src/utils/index.js 100.00% <ø> (ø)
...form/core/src/utils/isDisplaySetReconstructable.js 5.00% <ø> (+0.18%) ⬆️
...ils/metadataProvider/getPixelSpacingInformation.js 0.00% <0.00%> (ø)
platform/core/src/log.js 14.28% <0.00%> (-85.72%) ⬇️
...services/DicomMetadataStore/createStudyMetadata.js 0.00% <0.00%> (ø)
...ervices/DicomMetadataStore/createSeriesMetadata.js 0.00% <0.00%> (ø)
platform/core/src/utils/addAccessors.js 9.09% <9.09%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d14a8f0...24b4174. Read the comment docs.

@rodrigobasilio2022 rodrigobasilio2022 changed the title [WIP] fix(SR): Fix/sr not loading fix(SR): Fix/sr SCOORD3D not loading Sep 14, 2023
Copy link
Contributor

@igoroctaviano igoroctaviano left a comment

Choose a reason for hiding this comment

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

Code Review boostrap

@fedorov
Copy link
Member

fedorov commented Sep 18, 2023

I would like to have a test instance of this PR with OHIF that can be pointed to a Google DICOM store, before this is merged.

@rodrigobasilio2022
Copy link
Collaborator Author

In my tests I am doing exactly that.

@fedorov
Copy link
Member

fedorov commented Sep 18, 2023

In my tests I am doing exactly that.

Very good. In this case I hope it should be easy to create a setup where I would be able to do it as well.

@rodrigobasilio2022
Copy link
Collaborator Author

I will talk to @igoroctaviano so I can create something like the https://ohif-v3.web.app/

@fedorov
Copy link
Member

fedorov commented Sep 18, 2023

The reason I would like to do this is because I want to confirm it works not only for the test samples, but also for the similar annotations we create. In the past, with v2, the implementation was not working the way we expected. I don't think there is any urgency to merge this PR, it is more important to confirm it works as expected first.

@igoroctaviano
Copy link
Contributor

@sedghi can't we have something like https://github.com/OHIF/Viewers/blob/v2-legacy/extensions/cornerstone/src/tools/DICOMSRDisplayTool.js for v3/cornerstone3d? This way we have a generic cs tool that renders any kind of scoord3d graphic type.

@sedghi
Copy link
Member

sedghi commented Sep 29, 2023

@igoroctaviano Don't we have it already in OHIF in measurement tracking?

@igoroctaviano
Copy link
Contributor

@igoroctaviano Don't we have it already in OHIF in measurement tracking?

@rodrigobasilio2022 please check if we could use this tool then.

@rodrigobasilio2022
Copy link
Collaborator Author

It already has. The problem is that it doesnt enable measure jump and, of course, it didn't open the any of these SR examples

@igoroctaviano
Copy link
Contributor

It already has. The problem is that it doesnt enable measure jump and, of course, it didn't open the any of these SR examples

Can we make it work then? what we can do to fix the jump and the support?

@rodrigobasilio2022
Copy link
Collaborator Author

The reason I would like to do this is because I want to confirm it works not only for the test samples, but also for the similar annotations we create. In the past, with v2, the implementation was not working the way we expected. I don't think there is any urgency to merge this PR, it is more important to confirm it works as expected first.

https://d3w0jbfdn2j3pj.cloudfront.net/

@sedghi
Copy link
Member

sedghi commented Oct 17, 2023

@igoroctaviano is this ready for review? have you done your pass?

@rodrigobasilio2022
Copy link
Collaborator Author

No this is not ready for review, as we need to add the functionality described in the issue, maybe creating a new MeasurementTable

@sedghi
Copy link
Member

sedghi commented Oct 17, 2023

You mean on the SR view, you want measurement panel to show annotations? but right now we only show for hydrated?

@fedorov
Copy link
Member

fedorov commented Oct 17, 2023

@rodrigobasilio2022 before the code review, I would like to have access to a test instance to confirm it works as intended.

@igoroctaviano
Copy link
Contributor

@rodrigobasilio2022 whats the status of this PR based on our last iteration? Have you made the changes? I think you doing it its going to be a lot faster than handing it off. Let me know what you think.

@igoroctaviano igoroctaviano changed the title fix(SR): Fix/sr SCOORD3D not loading WIP fix(SR): Fix/sr SCOORD3D not loading Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDC:priority Items that the Imaging Data Commons wants to help sponsor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants