Progressive loading of vtkjs viewport #1055

JamesAPetts · 2019-10-16T16:44:48Z

Progressively load volumes in vtkjs. The volume and UI is still interactable whilst waiting on frames to come from the data source. The volume is re-loaded into the GPU every 20% of frames that are loaded.
WWWC on viewport overlay correctly updates on change.
Auto window level based on leveling of cornerstone viewport on mode switch.
PT leveled from 0-5 SUV.

Adds progressive volume loading for vtkjs viewport whilst the frames are streamed from PACs and rebuilt in the volume. Closes: closes OHIF#1051

codecov · 2019-10-16T16:47:44Z

Codecov Report

Merging #1055 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #1055   +/-   ##
======================================
  Coverage    9.62%   9.62%           
======================================
  Files         251     251           
  Lines        6257    6257           
  Branches     1162    1162           
======================================
  Hits          602     602           
  Misses       4600    4600           
  Partials     1055    1055

Flag	Coverage Δ
#core	`13.59% <ø> (ø)`	⬆️
#viewer	`0.42% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d49b31...db927ce. Read the comment docs.

dannyrb · 2019-10-16T20:01:29Z

@JamesAPetts is this ready for review?

dannyrb · 2019-10-16T20:05:49Z

extensions/vtk/src/OHIFVTKViewport.js

+            this.setState({
+              volumes: [volumeActor],
+            });
+          }, 200);


☝️ 👉 👇 👈

I know, this was Erik and I trying to make something render to the user ASAP. Hence the massive disclaimer up top. Feel free to fix it if you find a way!

…meStreaming

swederik · 2019-10-18T08:10:31Z

extensions/vtk/src/LoadingIndicator.js

@@ -35,7 +35,8 @@ class LoadingIndicator extends PureComponent {
          <div className="imageViewerLoadingIndicator loadingIndicator">
            <div className="indicatorContents">
              <p>
-                Loading... <i className="fa fa-spin fa-circle-o-notch fa-fw" />{' '}
+                Reformatting...{' '}


Use the translation function for i18n, and remove that {' '} thing

swederik · 2019-10-18T08:11:20Z

extensions/vtk/src/OHIFVTKViewport.js

@@ -172,47 +172,53 @@ class OHIFVTKViewport extends Component {
      default:
        imageDataObject = getImageData(stack.imageIds, displaySetInstanceUid);

-        const loadImageDataPromise = loadImageData(imageDataObject);
+        //loadImageData(imageDataObject);


I guess you can remove this line

swederik · 2019-10-18T08:13:21Z

extensions/vtk/src/OHIFVTKViewport.js

+        paintFilterLabelMapImageData: null, // TODO
+        percentComplete: 0,
+      },
+      () => {}


useless callback

swederik · 2019-10-18T08:15:37Z

extensions/vtk/src/OHIFVTKViewport.js

@@ -285,6 +321,56 @@ class OHIFVTKViewport extends Component {
    }
  }

+  loadProgressively() {


Maybe pass imageDataObject into the function instead of expecting it in this? Seems cleaner.

swederik · 2019-10-18T08:16:51Z

extensions/vtk/src/OHIFVTKViewport.js

+          // TODO -> Just do this higher up, call when loadImageData is first
+          // called and then do this once after all apis are made.
+          if (
+            this.props.viewportIndex === 0 &&


Why does viewportIndex matter?

So it only happens once, I'll move this down the viewport 👍 .

Moved down here: OHIF/react-vtkjs-viewport#56

swederik · 2019-10-18T08:19:08Z

extensions/vtk/src/utils/setMPRLayout.js

    const numViewports = numRows * numColumns;
+
+    if (viewportPropsArray && viewportPropsArray.length !== numViewports) {
+      reject(


Can we reject with new Error('viewportProps is supplied but its length is not equal to numViewports') instead of just a string? I thought we had an ESLint rule for that (https://eslint.org/docs/rules/prefer-promise-reject-errors)

swederik · 2019-10-18T08:20:25Z

extensions/vtk/src/utils/setMPRLayout.js

@@ -42,3 +53,65 @@ export default function setMPRLayout(displaySet) {
    );
  });
 }
+
+/*


Just remove this instead of leaving a massive comment block

swederik

LGTM pending minor changes and a double check of whether or not we still need RAF

JamesAPetts · 2019-10-18T09:13:21Z

Addressed issues and removed RAF successfully.

Just need to wait for react-vtkjs-viewport to publish and I'll merge this.

JamesAPetts added 10 commits October 14, 2019 14:19

Streaming WIP.

7402492

Functional default and manual wwwc

ad81c51

Merge remote-tracking branch 'upstream/master' into vtkVolumeStreaming

0a6689b

WIP use PR #OHIF#1226 of vtk-js.

6a8dd3a

Merge remote-tracking branch 'upstream/master' into vtkVolumeStreaming

c2d1325

Update vtk.js

3c0dcca

WIP orientation API.

27377de

feat: 🎸 Progressive volume loading for vtk viewport

e7da2e7

Adds progressive volume loading for vtkjs viewport whilst the frames are streamed from PACs and rebuilt in the volume. Closes: closes OHIF#1051

pull in master.

4841d7a

Remove cst4 dependency of vtk viewport.

28ec524

Merge branch 'master' into vtkVolumeStreaming

327d98a

dannyrb reviewed Oct 16, 2019

View reviewed changes

JamesAPetts added 2 commits October 17, 2019 10:11

Merge remote-tracking branch 'upstream/master' into vtkVolumeStreaming

3cdb8a1

Merge remote-tracking branch 'origin/vtkVolumeStreaming' into vtkVolu…

c265ca3

…meStreaming

swederik reviewed Oct 18, 2019

View reviewed changes

swederik approved these changes Oct 18, 2019

View reviewed changes

Address review comments.

e4aae63

Update to react-vtkjs-viewport 0.3.0

db927ce

JamesAPetts merged commit 9cdf8b6 into OHIF:master Oct 18, 2019

@@ @@ -42,3 +53,65 @@ export default function setMPRLayout(displaySet) { @@
                   );
                 });
               }
+              /*

Uh oh!

Progressive loading of vtkjs viewport #1055

Progressive loading of vtkjs viewport #1055

Conversation

JamesAPetts commented Oct 16, 2019

Uh oh!

codecov bot commented Oct 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dannyrb commented Oct 16, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JamesAPetts Oct 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swederik Oct 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swederik left a comment

Choose a reason for hiding this comment

Uh oh!

JamesAPetts commented Oct 18, 2019

Uh oh!

Uh oh!

codecov bot commented Oct 16, 2019 •

edited

Loading

JamesAPetts Oct 16, 2019 •

edited

Loading

swederik Oct 18, 2019 •

edited

Loading