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(rendering): 16 bit texture support with flag #420

Merged

Conversation

Ouwen
Copy link
Contributor

@Ouwen Ouwen commented Feb 4, 2023

Adds 16 bit texture support to cornerstone3D

  • CSWIL will decide on array type to match native storage format. In the case native format is unsigned int, if the scaling params are negative, prescale = true will return back a signed int.
  • Streaming Volume does require the array buffer type to be predetermined (targetBuffer must be specified). This is because all incoming slices need to adhere to one data type.
  • Block Height is set to 1 in the case norm16 is used due to this chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1408247. In the case preferSizeOverAccuracy is used block height for texture transfers can be calculated as usual.

External PR (cornerstonejs/cornerstoneWADOImageLoader#509):

  • CSWIL needs to implement scaling logic. When a buffer type is not provided native is used (using signed versions of array buffers in the case scale is negative). When a buffer type is provided scaling will occur with possible over/underflows.

@netlify
Copy link

netlify bot commented Feb 4, 2023

Deploy Preview for cornerstone-3d-docs ready!

Name Link
🔨 Latest commit 52c9fca
🔍 Latest deploy log https://app.netlify.com/sites/cornerstone-3d-docs/deploys/640e75f5e60bb00008ac993c
😎 Deploy Preview https://deploy-preview-420--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.

@Ouwen
Copy link
Contributor Author

Ouwen commented Feb 4, 2023

Credit goes to @sedghi for initial commits here: #336, this commit should be coauthored by him.

@Ouwen Ouwen force-pushed the gradienthealth/16-bit-texture-support branch 3 times, most recently from f550c36 to 357970b Compare February 4, 2023 03:49
@Ouwen
Copy link
Contributor Author

Ouwen commented Feb 4, 2023

pending vtk 26.4.0

@Ouwen Ouwen force-pushed the gradienthealth/16-bit-texture-support branch from 357970b to 4c86d35 Compare February 7, 2023 05:45
@Ouwen
Copy link
Contributor Author

Ouwen commented Feb 7, 2023

@sedghi ready for review

@Ouwen Ouwen force-pushed the gradienthealth/16-bit-texture-support branch from 4c86d35 to c9df864 Compare February 13, 2023 05:40
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.

Great work! Thanks, it is getting there

packages/core/src/init.ts Outdated Show resolved Hide resolved
Comment on lines 65 to 72
// init
init,
hasNorm16TextureSupport,
hasActiveWebGLContext,
isCornerstoneInitialized,
// configs
getConfiguration,
setPreferSizeOverAccuracy,
Copy link
Member

Choose a reason for hiding this comment

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

These are getting out of hand, can we put them under the configUtils?

packages/core/src/RenderingEngine/StackViewport.ts Outdated Show resolved Hide resolved
@Ouwen Ouwen force-pushed the gradienthealth/16-bit-texture-support branch from c9df864 to 03d8490 Compare February 13, 2023 16:39
Adds 16 bit texture support to cornerstone3D
  - CSWIL will decide on array type to match native storage format. In the case native format is unsigned int, if the scaling params are negative, prescale = true will return back a signed int.
  - Streaming Volume does require the array buffer type to be predetermined (targetBuffer must be specified). This is because all incoming slices need to adhere to one data type.
  - Block Height is set to 1 in the case norm16 is used due to this chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1408247. In the case preferSizeOverAccuracy is used block height for texture transfers can be calculated as usual.

TODO:
  - CSWIL needs to implement scaling logic. When a buffer type is not provided native is used (using signed versions of array buffers in the case scale is negative). When a buffer type is provided scaling will occur with possible over/underflows.

Co-authored-by: sedghi <ar.sedghi@gmail.com>
@Ouwen Ouwen force-pushed the gradienthealth/16-bit-texture-support branch from c0fe82d to 0833bf6 Compare March 13, 2023 00:54
@Ouwen
Copy link
Contributor Author

Ouwen commented Mar 13, 2023

@sedghi the above is ready for review with previous comments addressed. Let me know what you think!

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 great! Thanks as always

@sedghi sedghi changed the title 16 bit texture support feat(rendering): 16 bit texture support under a flag Mar 13, 2023
@sedghi sedghi changed the title feat(rendering): 16 bit texture support under a flag feat(rendering): 16 bit texture support with flag Mar 13, 2023
@sedghi sedghi merged commit f14073e into cornerstonejs:main Mar 13, 2023
@Ouwen Ouwen mentioned this pull request Mar 16, 2023
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