Jump to click and voisync for volume3d #678
Conversation
✅ Deploy Preview for cornerstone-3d-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -274,6 +274,10 @@ | |||
| "generateImageFromTimeData": { | |||
| "name": "Generate 3D Volume From 4D Data", | |||
| "description": "Demostrates generating a 3D volume from 4D data using subtract, average or sum." | |||
| }, | |||
| "mprVolume3d": { | |||
There was a problem hiding this comment.
what is the difference between this example and the ptct example we have here? https://www.cornerstonejs.org/live-examples/petct
There was a problem hiding this comment.
Hi @sedghi, it's exactly the same example but Volume3D instead of Volume in the MIP viewport, now I've just modified the PTCT example by adding a button to switch to Volume3D.
|
can you please update this now |
| @@ -32,7 +33,10 @@ export default function voiSyncCallback( | |||
|
|
|||
| const tViewport = renderingEngine.getViewport(targetViewport.viewportId); | |||
|
|
|||
| if (tViewport instanceof VolumeViewport) { | |||
| if ( | |||
| tViewport instanceof VolumeViewport || | |||
There was a problem hiding this comment.
You should say instance of BaseVolumeViewport instead of both
| @@ -621,7 +621,7 @@ class Viewport implements IViewport { | |||
| * be detected for pan/zoom values) | |||
| * @returns boolean | |||
| */ | |||
| protected resetCamera( | |||
| public resetCamera( | |||
There was a problem hiding this comment.
Why are you making this public, protected is good enough
There was a problem hiding this comment.
Hi @sedghi, resetCamera should be public in Viewport.ts because in the others viewports classes it convert it to public
VolumeViewport.ts:
resetCamera( resetPan?: boolean, resetZoom?: boolean, resetToCenter?: boolean ): boolean { return super.resetCamera(resetPan, resetZoom, resetToCenter); }
VolumeViewport3D.ts:
public resetCamera( resetPan = true, resetZoom = true, resetToCenter = true ): boolean { super.resetCamera(resetPan, resetZoom, resetToCenter); this.resetVolumeViewportClippingRange(); return; }
| const volume = await volumeLoader.createAndCacheVolume(ptVolumeId, { | ||
| imageIds: ptImageIds, | ||
| }); | ||
| volume.load(); |
There was a problem hiding this comment.
you don't need to create another volume for this, the same volume can be used in a different viewport type
| viewportIds.FUSION.AXIAL, | ||
| viewportIds.FUSION.SAGITTAL, | ||
| viewportIds.FUSION.CORONAL, | ||
| viewportIds.PETMIP.CORONAL, |
There was a problem hiding this comment.
Also I don't think you need to redefine the toolgroups, just grab the existing toolGroup for the MIP and modify its viewport (I don't think that is even needed, maybe just re-enable it?)
|
seems there are some conflicts for merging @Sofien-Sellami can you please resolve them? thanks |
# Conflicts: # packages/core/src/RenderingEngine/BaseVolumeViewport.ts # packages/tools/src/synchronizers/callbacks/voiSyncCallback.ts
Context
In this pull request i wanted to add jump to click and voiSync for the volume 3D viewport, with these last changes i refactored the viewport classes, i find it easier to put all the methods in BaseVolumeViewport and then reuse them in any desired class.
Changes & Results
Testing
Checklist
PR
semantic-release format and guidelines.
Code
My code has been well-documented (function documentation, inline comments,
etc.)
I have run the
yarn build:update-apito update the API documentation, and havecommitted the changes to this PR. (Read more here https://www.cornerstonejs.org/docs/contribute/update-api)
Public Documentation Updates
additions or removals.
Tested Environment