cornerstone3D
cornerstone3D copied to clipboard
feat: added support for sigmoid voiLUTFunction for StackViewport and VolumeViewport
- CPU fallback is unimplemented
- Volume streaming auto fallbacks to
LINEAR
mode.
Deploy Preview for cornerstone-3d-docs ready!
Name | Link |
---|---|
Latest commit | 348409a58bf48d4ee9d80fe0f8a68ffb4f06e96e |
Latest deploy log | https://app.netlify.com/sites/cornerstone-3d-docs/deploys/63e992456260f70008faeb5e |
Deploy Preview | https://deploy-preview-224--cornerstone-3d-docs.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
@sedghi
@sedghi added VOI type as an enum and added documentation
All looks good, two last things
- This only adds it for StackViewport right? maybe we rename the PR to include such information?
- Can you please post here a before/after image for LINEAR vs SIGMOID voi for reference (since we are not adding new docs)
Thanks
Sure thing, I think I can also try implementing this in the volume viewer
Linear
Sigmoid
@sedghi I've added the changes you suggested. Also scaffolding for the VolumeViewport. One thing that came to mind while looking at the volume viewport is if the voiLUTfunction should exist at the viewportInput level vs setVOI in the stackviewport as well as volume viewport
Thoughts?
I noticed that we have the following:
voiLUTFunction
: seems to refer to cornerstoneWADO image object/metadata properties
voiLutFunction
: property of viewports
VOILUTFunction
: dcmjs naming convention
Is this intentional?
I hate this a lot and didn't notice it, good catch
voiLUTFunction: seems to refer to cornerstoneWADO image object/metadata properties
voiLutFunction: property of viewports
VOILUTFunction: dcmjs naming convention
Can we remove voiLutFunction
and use either of those? I think we can live with the dcmjs one only
Were you able to run the example i added? It seems to be not working as I expected
cd packages/core
yarn run example voiSigmoid
@sedghi this PR requires making this change to vtkjs (https://github.com/gradienthealth/vtk-js/commit/623f5ec9841b207d5d91b754f259872e81b4d249)
VOILUTFunction should be supported in volumes and stack viewport now.
There is some setProperties logic that I believe is still TODO from previous commits, but I think it might be beyond scope of this PR.
pending https://github.com/Kitware/vtk-js/pull/2603
@sedghi seems like the vtk.js version would need to be bumped to latest v25.10.1.
Would this have consequences for other related libraries (OHIF/Viewer)?
That will break a lot of tests, I have fixed them in my other branch, can we wait for my branch (not create PR yet) to be merged before this?
Sure, no rush
https://github.com/Kitware/vtk-js/pull/2610, heads up the full fix requires vtk.js 25.11.1
@sedghi hmmm unsure why these doc issues are popping up. As far as I know, I ran yarn run build:update-api
nvm it is from the examples, I know the fix...
@sedghi ready for review
@sedghi just another ping since there are a lot of branches flying around...
@sedghi this is ready for review. Instead of moving the triggerEvent function outside of the createVolumeActor
I decide to remove the need for this.voiRange
instead this is something that can be derived from the volume actor's look up table. The logic for this is in the getProperties
section.