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

Feat/v3 rtss #3092

Closed
wants to merge 2 commits into from
Closed

Feat/v3 rtss #3092

wants to merge 2 commits into from

Conversation

dxlin
Copy link
Contributor

@dxlin dxlin commented Jan 3, 2023

PR Checklist

  • Brief description of changes
  • Links to any relevant issues
  • Required status checks are passing
  • User cases if changes impact the user's experience
  • @mention a maintainer to request a review

Hi @sedghi, sorry for the delay with this.

This is the code that exports/converts current segmentations to RTSS dicom files. This is invoked via the commands exportRTSS or uploadRTSS (depending if want to download to local or upload to PACS).

Originally we had discussed putting this code within cornerstone or dcmjs, but ultimately I felt like it may have been easier and more concise to keep this as an extension. At the moment the RTSS upload portion only uploads a new RTSS dicom, but once I can see the import code I can also work to revise it so that it replaces if the segmentation affected was from a previously existing RTSS dicom.

Sorry for the delay, had some difficulties with bugs the conversion when segmentations intersect with the border of the image volume.

@netlify
Copy link

netlify bot commented Jan 3, 2023

Deploy Preview for ohif-platform-viewer ready!

Name Link
🔨 Latest commit b162ae0
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-viewer/deploys/63b391f98632f90009f84a30
😎 Deploy Preview https://deploy-preview-3092--ohif-platform-viewer.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 settings.

@netlify
Copy link

netlify bot commented Jan 3, 2023

Deploy Preview for ohif-platform-docs canceled.

Name Link
🔨 Latest commit b162ae0
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/63b391f96aa9ee00090a400e

@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Merging #3092 (b162ae0) into v3-stable (9f1e813) will increase coverage by 14.67%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##           v3-stable    #3092       +/-   ##
==============================================
+ Coverage      25.15%   39.83%   +14.67%     
==============================================
  Files            119       96       -23     
  Lines           2862     2036      -826     
  Branches         555      420      -135     
==============================================
+ Hits             720      811       +91     
+ Misses          1856     1002      -854     
+ Partials         286      223       -63     
Impacted Files Coverage Δ
...tform/core/src/services/CineService/CineService.js 12.50% <0.00%> (-6.25%) ⬇️
.../services/MeasurementService/MeasurementService.js 47.00% <0.00%> (-2.19%) ⬇️
...ore/src/services/_shared/pubSubServiceInterface.js 80.00% <0.00%> (-1.82%) ⬇️
platform/core/src/classes/CommandsManager.js 90.69% <0.00%> (-1.14%) ⬇️
platform/core/src/classes/MetadataProvider.js 4.54% <0.00%> (-0.72%) ⬇️
platform/viewer/src/index.js 0.00% <0.00%> (ø)
platform/viewer/src/appInit.js 0.00% <0.00%> (ø)
platform/core/src/utils/index.js 100.00% <0.00%> (ø)
platform/core/src/utils/isImage.js 100.00% <0.00%> (ø)
platform/core/src/services/ServicesManager.js 100.00% <0.00%> (ø)
... and 50 more

Continue to review full report at Codecov.

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

@@ -31,6 +31,10 @@
{
"packageName": "@ohif/extension-cornerstone-dicom-seg",
"version": "3.0.0"
},
{
"packageName": "dicom-rtss",
Copy link
Member

Choose a reason for hiding this comment

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

can we name this extension dicom-rt?

Copy link
Member

Choose a reason for hiding this comment

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

@ohif/extension-dicom-rt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do, I will make this modification along with the colors in the next commit.

@@ -0,0 +1,73 @@
{
"name": "dicom-rtss",
Copy link
Member

Choose a reason for hiding this comment

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

please see other package.json and follow their namings

@sedghi
Copy link
Member

sedghi commented Jan 6, 2023

For some reason the exported RT looks like this
image

Do you have any idea why?

@dxlin
Copy link
Contributor Author

dxlin commented Jan 6, 2023

Would you be able to also show the original segmentation?

It looks like the contour is outside of the volume, but that should not be happening as the vtkjs routine to my knowledge is based on the segmentation volume, which should be identical to the image volume.

@sedghi sedghi changed the base branch from v3-stable to master June 19, 2023 13:38
@sedghi
Copy link
Member

sedghi commented Jun 19, 2023

Any chance you can look at my comment?

@dxlin
Copy link
Contributor Author

dxlin commented Jun 21, 2023

Any chance you can look at my comment?

Sorry I hadn't had a chance to look at this again for a while, I'll try to take a look later this week.

Would you be able to note the tool/steps you used to create the segmentation? When I did my testing I had only used the brush tool so I may need to test with a variety of tools.

@sedghi
Copy link
Member

sedghi commented Jul 20, 2023

We should move this logic inside @cornerstonejs/adapters

@sedghi
Copy link
Member

sedghi commented Oct 3, 2023

@dxlin as discussed we are going to close this and you will create a new 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