Add new 3D volume viewport #281

luccascorrea · 2022-11-04T15:22:53Z

This pull request was originated from this issue.

The existing VolumeViewport class was refactored and all common functionality was extracted to an abstract base class called BaseVolumeViewport. Then both the existing VolumeViewport class and the new VolumeViewport3D extend it.

Besides that, a new utility function was added to allow easily applying a preset to a viewport.

You can try it here https://deploy-preview-281--cornerstone-3d-docs.netlify.app/live-examples/volumeviewport3d

netlify · 2022-11-04T15:23:01Z

✅ Deploy Preview for cornerstone-3d-docs ready!

Name	Link
🔨 Latest commit
🔍 Latest deploy log	https://app.netlify.com/sites/cornerstone-3d-docs/deploys/639255de896d5e0b611c5194
😎 Deploy Preview	https://deploy-preview-281--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 · 2022-11-04T19:53:30Z

thanks, I will review, you need to run yarn build:update-api and include (commit) the generated files in your PR for CI to pass

luccascorrea · 2022-11-04T20:19:26Z

Gotcha. Thanks, that's exactly what I was trying to figure out. I just fixed everything so it should be good now.

sedghi · 2022-11-07T19:46:21Z

@NeilMacPhee Can you give a first review pass on this? Thanks

NeilMacPhee · 2022-11-28T18:10:58Z

@NeilMacPhee Can you give a first review pass on this? Thanks

Did a first pass review and everything looks good to me

sedghi · 2022-12-01T19:21:23Z

@luccascorrea Can you please rebase your branch? I have time to review today and tomorrow. Thanks

luccascorrea · 2022-12-01T22:23:28Z

I just finished rebasing it @sedghi.

sedghi · 2022-12-02T02:20:51Z

This is not a correct rebase as I see old code still there, can you make sure your rebase is on the most recent main commit

luccascorrea · 2022-12-02T02:38:31Z

Hi @sedghi. I actually did rebase it on the latest commit on the main branch. I had a look at the RenderingEngine.ts file that you screenshotted and the version on my branch looks exactly like the one on the main branch except for 6 line changes that I made. Can you please have another look?

luccascorrea · 2022-12-02T02:41:41Z

Here's what that portion of the code looks like on my branch:

which is the same as here: https://github.com/cornerstonejs/cornerstone3D-beta/blob/main/packages/core/src/RenderingEngine/RenderingEngine.ts#L211-L230

sedghi · 2022-12-02T03:04:06Z

You are right, so also seems like you have surpassed the husky commit hooks since that should have fixed all " " to ' '
can you run eslint fix? there are too many " " .

luccascorrea · 2022-12-02T03:17:50Z

Sure. I can do that tomorrow.

luccascorrea · 2022-12-02T11:49:08Z

It's fixed @sedghi. If there's anything else you'd like me to adjust, please let me know.

swederik · 2022-12-07T12:01:30Z

This is great! One issue is that I see it is missing from https://github.com/cornerstonejs/cornerstone3D-beta/blob/main/utils/ExampleRunner/example-info.json

luccascorrea · 2022-12-07T12:20:00Z

Thanks @swederik! You're right. I just pushed a fix.

packages/tools/src/utilities/segmentation/createLabelmapVolumeForViewport.ts

packages/tools/src/tools/WindowLevelTool.ts

sedghi

Looks great initial roudn. Please see my comments. Could you also please email me on [email protected], I want to discuss proper attribution with you since we usually put noticable contributions in our newsletter and this PR is certainly one.

Thanks

packages/tools/src/tools/base/BaseTool.ts

packages/tools/src/synchronizers/callbacks/voiSyncCallback.ts

sedghi · 2022-12-07T21:47:55Z

packages/core/src/enums/ViewportType.ts

@@ -15,6 +15,7 @@ enum ViewportType {
  ORTHOGRAPHIC = 'orthographic',
  /** Perspective Viewport: Not Implemented yet */
  PERSPECTIVE = 'perspective',
+  VOLUME_3D = 'volume3d',


I don't think VOLUME_3D is a good pick, but cannot think of a better one either. I think by default we have camera parallel projection ON

@swederik Do you think it should be perspective 3D? or orthogonal 3D makes sense, also any idea for a good name?

Yes, I couldn't think of a better name. I'll wait for your input on this.

sedghi · 2022-12-07T21:54:30Z

packages/core/src/RenderingEngine/BaseVolumeViewport.ts

@@ -0,0 +1,961 @@
+import { vec3 } from 'gl-matrix';


I beleive the following methods are VolumeViewport specific and don't belong here in the BaseVolumeViewport

setOrientation, _getOrientationVectors, _getAcquisitionPlaneOrientation, _setViewPlaneToAcquisitionPlane, getBlendModes, setBlendModes, getIntensityFromWorld, setSlabThickness, getSlabThickness, canvasToWorld, worldToCanvas, getCurrentImageIdIndex, getCurrentImageId

specially canvasToWorld and worldToCanvas requires correct camera matrices which probably doesn't work out of the box, but we don't need them right now since we don't have tools on 3DViewport. You can just console.log('needs implementation') for them

I refactored all the methods you mentioned but canvasToWorld and worldToCanvas are actually necessary otherwise the TrackballRotateTool can't work with the new 3D viewport. In my tests, the original implementation seems to be working fine.

sedghi

Looks great thanks

luccascorrea · 2022-12-09T21:24:55Z

That's fantastic @sedghi 😃

ranasrule · 2022-12-23T15:17:40Z

Any chance we will be seeing this in OHIF Viewer V3 anytime soon ?

* fix: mouse-up should not unhighlight annotations (#305) * fix: annotation highlighted and tooling for ellipticalROI * update build * fix tests * chore(release): publish [skip ci] - [email protected] - @cornerstonejs/[email protected] * fix: stack viewport flip scroll (#304) * fix: use focal point for pan cache for stack viewport * fix: pan dir with flip * fix pan values while flipped * update build * apply review comments * fix build * chore(release): publish [skip ci] - @cornerstonejs/[email protected] - [email protected] - @cornerstonejs/[email protected] - @cornerstonejs/[email protected] * feat: add referenceCursors tool (#275) * added basic cursorCrosshairSync tool with example, TODO: for now cursorSync is displayed regardless of distance, create configurable distance and also sync the position of all viewports over which the mouse is not to scroll to a slice that is close to the currentMousePosition in 3d space * addde stack syncing for StackViewport and syncing for volumeViewport on imageChange events, added configuration for max display distance * refactored tool functions * added comment to possible bug * added configuration options to example * changed look of crosshair to 4 lines with central space * undid local tsconfig change * undid yarn.lock changes * added tool to example-info.json * removed from example-runner because it broke build * readded example and fixed typo * readded example-info and changed example to trigger rebuild * added cleanup for mouseoverElement when tool is disabled * added cleanup when tool gets disabled, this does not get called when toolGroup gets destroyed, might cause remaining listeners * applied naming changes, reworked adding annotation logic * removed event listeners and moved logic to check for stack scrolling into rendering logic * added planeDistanceToPoint to planar utilities * added getClosestStackImageIndexForPoint * rewrote logic to use onCameraModified * updated example-info * fixed bug with 0 being falsey * added logic to remove cursor if wanted * modified toolGroup so that setting a tool active only changes the cursor to default if there is no primary mouse cursor * fixed bug not updating disable cursor * fixed missing parentheses from merge * readded scrollWheel scrolling and api changes * fixed typos * chore(release): publish [skip ci] - @cornerstonejs/[email protected] - [email protected] - @cornerstonejs/[email protected] - @cornerstonejs/[email protected] * fix: ZoomTool fix for polyData actors with no imageData (#308) * chore(release): publish [skip ci] - [email protected] - @cornerstonejs/[email protected] * fix: If planar annotation is not visible, filter it (#318) Co-authored-by: edward65 <[email protected]> * fix: filter planarFreeHandeROI based on parallel normals instead of equal normals. (#315) Co-authored-by: Ramon Emiliani <[email protected]> * fix: get correct imageData with targetId in BaseTool (#294) * limit disabled element not need to render * Update BaseTool.ts fix: get correct viewport when there are multiple viewport with same stack data Co-authored-by: chendingmiao <[email protected]> * chore(release): publish [skip ci] - [email protected] - @cornerstonejs/[email protected] * fix: htj2k and keymodifier (#313) * fix(htj2k):Support htj2k in the streaming volume loader * fix(decodeImage):Fix htj2k image decode and mouse key modifiers * Update for PR * update ci build * chore(release): publish [skip ci] - [email protected] - @cornerstonejs/[email protected] - @cornerstonejs/[email protected] * fix: coronal view should not be flipped (#321) * chore(release): publish [skip ci] - @cornerstonejs/[email protected] - [email protected] - @cornerstonejs/[email protected] - @cornerstonejs/[email protected] * fix: bidirectional tool when short and long axis changes (#309) * fix rotation for handles * fix: short axis movement * fix: bidirectional tool incorrect interaction * chore(release): publish [skip ci] - @cornerstonejs/[email protected] - [email protected] - @cornerstonejs/[email protected] - @cornerstonejs/[email protected] * fix(volumeViewport): Add optional scaling as the volume can be undefined (#323) While trying to get the volume from the cache, it can be undefined so getting the scaling attribute would throw an error in that case. This is a quick fix * chore(release): publish [skip ci] - @cornerstonejs/[email protected] - [email protected] - @cornerstonejs/[email protected] - @cornerstonejs/[email protected] * fix: Use queryselector instead of firstChild to get svg-layer (#268) * chore(release): publish [skip ci] - [email protected] - @cornerstonejs/[email protected] * [wip] initial dicom-loader typescript conversion * [wip] initial typescript conversion * [wip] update types for tests * feat: enable having multiple instances of the same tool and add more seg tools (#327) * feat: add floodFill and advanced brushTool * feat: enable adding tool instances from a parent tool class * fix threshold and brush size * chore(release): publish [skip ci] - [email protected] - @cornerstonejs/[email protected] * feat: Add new 3D volume viewport (#281) * Extract common volume viewport functionality to base class * Add viewport presets * Add utility function for applying preset to volume actor * Add new 3D volume viewport * Add example for VolumeViewport3D * feat: add presets dropdown to demo * update example info json Co-authored-by: Luccas Correa <[email protected]> Co-authored-by: Alireza <[email protected]> * chore(release): publish [skip ci] - @cornerstonejs/[email protected] - [email protected] - @cornerstonejs/[email protected] - @cornerstonejs/[email protected] * fix: Add coplanar check in stackImageSync callback (#335) * Add coplanar check in stackImageSync callback * Refactoring function * chore(release): publish [skip ci] - [email protected] - @cornerstonejs/[email protected] * fix: could not access 'index' before initialization (#337) * Avoid circular dependancy with vite build * Nicer import * chore(release): publish [skip ci] - [email protected] - @cornerstonejs/[email protected] * [dicom-loader] update CornerstoneWadoLoaderOptions to include optional params * [dicom-image-loader] types update * [dicom-image-loader] port wadoImageLoader tests to monorepo * [dicom-image-loader] port changes from cornerstoneWADOImageLoader commit id 9d71753

* more efficient buffer allocation I was able to speed up writing segmentations for large series (>1000 images) by a factor of 10 in Firefox with this fix The problem was too many buffer copies when the buffer was too small The same strategy is used in Rust for vector capacity allocation: https://github.com/rust-lang/rust/blob/0ca7f74dbd23a3e8ec491cd3438f490a3ac22741/src/liballoc/raw_vec.rs#L397 * fix edge case of too small new allocation * comment

luccascorrea force-pushed the feat/new-3d-viewport branch 2 times, most recently from 3211c63 to 2a6cf30 Compare November 4, 2022 19:38

luccascorrea force-pushed the feat/new-3d-viewport branch from 2a6cf30 to dd7102f Compare November 4, 2022 20:14

sedghi self-requested a review November 24, 2022 18:26

luccascorrea force-pushed the feat/new-3d-viewport branch 2 times, most recently from bbafa30 to a513081 Compare December 1, 2022 22:17

luccascorrea force-pushed the feat/new-3d-viewport branch from a513081 to 7b7960f Compare December 2, 2022 11:44

luccascorrea force-pushed the feat/new-3d-viewport branch from 7b7960f to b0e8ae0 Compare December 7, 2022 12:16

sedghi reviewed Dec 7, 2022

View reviewed changes

packages/tools/src/utilities/segmentation/createLabelmapVolumeForViewport.ts Outdated Show resolved Hide resolved

sedghi reviewed Dec 7, 2022

View reviewed changes

packages/tools/src/tools/WindowLevelTool.ts Outdated Show resolved Hide resolved

sedghi requested changes Dec 7, 2022

View reviewed changes

Luccas Correa added 4 commits December 8, 2022 00:55

Extract common volume viewport functionality to base class

433d8c1

Add viewport presets

6270d4b

Add utility function for applying preset to volume actor

9e0db31

Add new 3D volume viewport

cea9360

Add example for VolumeViewport3D

7259e3b

luccascorrea force-pushed the feat/new-3d-viewport branch from b0e8ae0 to 7259e3b Compare December 8, 2022 00:59

sedghi added 2 commits December 8, 2022 14:55

feat: add presets dropdown to demo

f039f34

update example info json

7abc9e4

sedghi approved these changes Dec 9, 2022

View reviewed changes

sedghi merged commit 57cf7ac into cornerstonejs:main Dec 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new 3D volume viewport #281

Add new 3D volume viewport #281

luccascorrea commented Nov 4, 2022 •

edited by sedghi

Loading

netlify bot commented Nov 4, 2022 •

edited

Loading

sedghi commented Nov 4, 2022

luccascorrea commented Nov 4, 2022

sedghi commented Nov 7, 2022

NeilMacPhee commented Nov 28, 2022

sedghi commented Dec 1, 2022

luccascorrea commented Dec 1, 2022

sedghi commented Dec 2, 2022

luccascorrea commented Dec 2, 2022

luccascorrea commented Dec 2, 2022 •

edited

Loading

sedghi commented Dec 2, 2022

luccascorrea commented Dec 2, 2022

luccascorrea commented Dec 2, 2022

swederik commented Dec 7, 2022

luccascorrea commented Dec 7, 2022

sedghi left a comment

sedghi Dec 7, 2022

luccascorrea Dec 8, 2022

sedghi Dec 7, 2022

luccascorrea Dec 8, 2022

sedghi left a comment

luccascorrea commented Dec 9, 2022

ranasrule commented Dec 23, 2022

Add new 3D volume viewport #281

Add new 3D volume viewport #281

Conversation

luccascorrea commented Nov 4, 2022 • edited by sedghi Loading

netlify bot commented Nov 4, 2022 • edited Loading

✅ Deploy Preview for cornerstone-3d-docs ready!

sedghi commented Nov 4, 2022

luccascorrea commented Nov 4, 2022

sedghi commented Nov 7, 2022

NeilMacPhee commented Nov 28, 2022

sedghi commented Dec 1, 2022

luccascorrea commented Dec 1, 2022

sedghi commented Dec 2, 2022

luccascorrea commented Dec 2, 2022

luccascorrea commented Dec 2, 2022 • edited Loading

sedghi commented Dec 2, 2022

luccascorrea commented Dec 2, 2022

luccascorrea commented Dec 2, 2022

swederik commented Dec 7, 2022

luccascorrea commented Dec 7, 2022

sedghi left a comment

Choose a reason for hiding this comment

sedghi Dec 7, 2022

Choose a reason for hiding this comment

luccascorrea Dec 8, 2022

Choose a reason for hiding this comment

sedghi Dec 7, 2022

Choose a reason for hiding this comment

luccascorrea Dec 8, 2022

Choose a reason for hiding this comment

sedghi left a comment

Choose a reason for hiding this comment

luccascorrea commented Dec 9, 2022

ranasrule commented Dec 23, 2022

luccascorrea commented Nov 4, 2022 •

edited by sedghi

Loading

netlify bot commented Nov 4, 2022 •

edited

Loading

luccascorrea commented Dec 2, 2022 •

edited

Loading