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(suv display): fix scaling of non-SUV PT images #536

Merged
merged 7 commits into from
Apr 11, 2023

Conversation

pwespi
Copy link
Contributor

@pwespi pwespi commented Mar 31, 2023

Currently PT images that are not in SUV are not rescaled at all. This is problematic because PT images can be in units of e.g. counts or Bq/ml, so it's important that the rescaling is applied in those cases.

@netlify
Copy link

netlify bot commented Mar 31, 2023

Deploy Preview for cornerstone-3d-docs ready!

Name Link
🔨 Latest commit 86f946a
🔍 Latest deploy log https://app.netlify.com/sites/cornerstone-3d-docs/deploys/6435681924c9e90008204b4d
😎 Deploy Preview https://deploy-preview-536--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 settings.

@@ -1,8 +1,6 @@
function getModalityUnit(modality: string, isPreScaled: boolean): string {
if (modality === 'CT') {
return 'HU';
} else if (modality === 'PT' && isPreScaled === true) {
Copy link
Member

Choose a reason for hiding this comment

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

Why we are removing this unit here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because a PT image can be in many different units, e.g. counts or Bq/ml (see the full list here: https://dicom.nema.org/medical/dicom/current/output/chtml/part03/sect_C.8.9.html#sect_C.8.9.1.1.3).

If we want to display the correct unit, we would have to know the (0054,1001) tag in addition to knowing whether SUV rescaling has been applied. I thought it's better to not show a unit than the wrong one.

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 get the metadata and check if suvbw is there in scalingModule, and return 'SUV' to be able to show SUV like before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Adapted.

@@ -1021,8 +1023,15 @@ class CircleROITool extends AnnotationTool {
stdDev /= count;
stdDev = Math.sqrt(stdDev);

const { suvbw } =
Copy link
Member

Choose a reason for hiding this comment

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

we don't need to do this metadata query on each cachedStats calculation since it doesn't change with annotation data change, it can be done outside, in the render loop, just before _getTextLines calls

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved the metadata query

@pwespi
Copy link
Contributor Author

pwespi commented Apr 6, 2023

Some tests are currently failing. I assume it's because of the metadata query, but I'm not sure how to fix it.

@sedghi
Copy link
Member

sedghi commented Apr 6, 2023

can you follow this please https://www.cornerstonejs.org/docs/contribute/update-api

@sedghi
Copy link
Member

sedghi commented Apr 6, 2023

seems like tools' tests are failing, maybe look here to how to run tests locally https://www.cornerstonejs.org/docs/contribute/tests

@pwespi
Copy link
Contributor Author

pwespi commented Apr 6, 2023

seems like tools' tests are failing, maybe look here to how to run tests locally https://www.cornerstonejs.org/docs/contribute/tests

It seems like in some tests annotation.metadata.referencedImageId is undefined, which causes fakeMetaDataProvider to throw an error (calling .split of undefined).

Do you think calling const { suvbw } = metaData.get('scalingModule', annotation.metadata.referencedImageId) || {}; is problematic?

If I catch the undefined in fakeMetaDataProvider, all tests pass locally, but I'm not sure if this would hide another problem.

@pwespi
Copy link
Contributor Author

pwespi commented Apr 11, 2023

Added a check for referencedImageId being defined and this fixes the tests.

@sedghi
Copy link
Member

sedghi commented Apr 11, 2023

Hmmm, this will break our volume viewports I guess, since they don't necessarily have referenceImageId, right now they can show suv

image

I guess the correct logic can be created inside AnnotationTool base class to check if it is a volume or stack. If volume it checks if scaling is presented in the volume (https://github.com/cornerstonejs/cornerstone3D-beta/blob/578830013cb7b9474166dfe1f469f7908b39aabe/packages/core/src/cache/classes/ImageVolume.ts#L37-L46), and for stack it looks at scaling module or seems like we have StackViewport.scaling as well that you can check.

@pwespi
Copy link
Contributor Author

pwespi commented Apr 11, 2023

Added isSuvScaled to AnnotationTool base class that checks for viewport type.

Copy link
Member

@sedghi sedghi left a comment

Choose a reason for hiding this comment

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

thanks, this looks great now

@sedghi sedghi changed the title fix: fix scaling of non-SUV PT images fix(suv display): fix scaling of non-SUV PT images Apr 11, 2023
@sedghi sedghi merged commit f9182f0 into cornerstonejs:main Apr 11, 2023
@pwespi
Copy link
Contributor Author

pwespi commented Apr 11, 2023

Thank you for your help and your great work on this project in general! 🎉

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