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(nifti): NIFTI data type enhancement #1219

Merged
merged 11 commits into from
May 28, 2024

Conversation

yanqzsu
Copy link
Contributor

@yanqzsu yanqzsu commented Apr 22, 2024

Context

Changes & Results

Testing

The test data is provided by #791(pull request #1033) and #964
https://raw.githubusercontent.com/alfnie/conn/master/utils/surf/referenceT1.nii
https://drive.google.com/file/d/1sLOBm3PGT2XOAWdPRdcyMLZ7tOB9fCcT/view?usp=drive_link

Following is the compare results with NiiVue(RAS) and ITK-SNAP(LPS)
INT16
nifti-compare-int16 (2)

UINT8
nifti-compare-uint8 (2)

UINT8 with float scaler
nifti-compare

Checklist

PR

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

Code

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: 18.20.1
  • "Browser: Chrome 83.0.4103.116

yanqzsu added 4 commits April 22, 2024 15:31
…32 (cornerstonejs#964)

* feat(Nifti): support uint8 nifti files (cornerstonejs#791)

* fix(Nifti): scale negetive and float  result is not considered. Skip scale if scale_slope is 1 and scale_inter is 0
Copy link

netlify bot commented Apr 22, 2024

Deploy Preview for cornerstone-3d-docs ready!

Name Link
🔨 Latest commit 3c1638b
🔍 Latest deploy log https://app.netlify.com/sites/cornerstone-3d-docs/deploys/6655b88dcfca2c000813fd6a
😎 Deploy Preview https://deploy-preview-1219--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.

@yanqzsu
Copy link
Contributor Author

yanqzsu commented Apr 29, 2024

This PR is very important to my project.
Do I need some modifications?

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.

Hi, this is amazing contribution thanks for this.

I tried the branch, but was not able to load the int8_t

CleanShot 2024-05-15 at 15 29 17@2x

I think this is the data problem as it renders the same in 3D slicer too, so i'm fine with most of the changes. Please see my comments

CleanShot 2024-05-15 at 15 31 07@2x

@yanqzsu yanqzsu requested a review from sedghi May 18, 2024 07:13
@yanqzsu yanqzsu marked this pull request as draft May 28, 2024 09:43
@yanqzsu
Copy link
Contributor Author

yanqzsu commented May 28, 2024

Hi, @sedghi

I tried the branch, but was not able to load the int8_t. I think this is the data problem as it renders the same in 3D slicer too

I found more int8_t images for testing, CT_Abdo, CT_Electrodes, etc. from niivue-images.
We can rendered them same as ITK-SNAP and NiiVue.
So I think it is a data problem too.
Screenshot 2024-05-28 192235

For other images, most of them are OK except visiblehuman.nii.gz which is a RGB image and chris_MRA.nii.gz which need update nifti-reader-js to 0.6.8.
I create a new PR #1286 to do this because there are too many conflicts.

Also I found a bug of _getImageScalarDataFromImageVolume() in setDefaultVolumeVOI.ts
const { scalarData } = imageVolume;
const { volumeBuffer } = scalarData;
volumeBuffer always be undefined so I fixed it meantime.

Other codes are adjusted to make the code structure clearer

Please review it again.

@yanqzsu yanqzsu marked this pull request as ready for review May 28, 2024 11:31
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.

Looks awesome now, thanks

@sedghi sedghi merged commit 03a2335 into cornerstonejs:main May 28, 2024
10 checks passed
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