From 89a96f02a01315f946de6833f4530dc391cdd91b Mon Sep 17 00:00:00 2001
From: Bruno Alves de Faria <brunoalvesdefaria@gmail.com>
Date: Tue, 31 Oct 2017 10:15:07 -0200
Subject: [PATCH] Lazy loading measurements from lesions table

---
 .../measurementTableTimepointCell.html        |   3 +-
 .../measurementTableTimepointCell.js          |  17 +-
 .../measurementTableTimepointCell.styl        |  25 ++-
 .../client/lib/activateMeasurements.js        |  30 +--
 .../client/lib/jumpToRowItem.js               | 178 ++++++++----------
 .../imports/client/components/browser/item.js |   2 +-
 .../imports/client/lib/loadStudy.js           |  27 ++-
 7 files changed, 140 insertions(+), 142 deletions(-)

diff --git a/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.html b/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.html
index e1d6e063e..d740dff5c 100644
--- a/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.html
+++ b/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.html
@@ -1,6 +1,7 @@
 <template name="measurementTableTimepointCell">
     {{#if hasDataAtThisTimepoint}}
-        <div class="measurementTableTimepointCell noselect" tabindex="1">
+        <div class="measurementTableTimepointCell noselect {{#if isLoading}}loading{{/if}}" tabindex="1">
+            <i class="fa fa-spin fa-circle-o-notch fa-fw loading-spinner"></i>
             {{displayData}}
         </div>
     {{else}}
diff --git a/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.js b/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.js
index 03d44481c..84c805910 100644
--- a/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.js
+++ b/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.js
@@ -7,8 +7,7 @@ Template.measurementTableTimepointCell.helpers({
         // This simple function just checks whether or not timepoint data
         // exists for this Measurement at this Timepoint
         const instance = Template.instance();
-        const rowItem = instance.data.rowItem;
-        const timepointId = instance.data.timepointId;
+        const { rowItem, timepointId } = instance.data;
 
         if (timepointId) {
             const dataAtThisTimepoint = _.where(rowItem.entries, { timepointId });
@@ -20,8 +19,7 @@ Template.measurementTableTimepointCell.helpers({
 
     displayData() {
         const instance = Template.instance();
-        const rowItem = instance.data.rowItem;
-        const timepointId = instance.data.timepointId;
+        const { rowItem, timepointId } = instance.data;
 
         let data;
         if (timepointId) {
@@ -45,8 +43,17 @@ Template.measurementTableTimepointCell.helpers({
             OHIF.log.warn('Something went wrong?');
         }
 
-        const displayFunction = tool.options.measurementTable.displayFunction;
+        const { displayFunction } = tool.options.measurementTable;
         return displayFunction(data);
+    },
+
+    isLoading() {
+        const instance = Template.instance();
+        const { rowItem, timepointId } = instance.data;
+        const { entries } = rowItem;
+        const measurementData = timepointId ? _.findWhere(entries, { timepointId }) : entries[0];
+        const { studyInstanceUid } = measurementData;
+        return OHIF.studies.loadingDict.get(studyInstanceUid) === 'loading';
     }
 });
 
diff --git a/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.styl b/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.styl
index 2c2631b76..b4da3b3e1 100644
--- a/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.styl
+++ b/Packages/ohif-measurements/client/components/measurementTable/measurementTableTimepointCell/measurementTableTimepointCell.styl
@@ -1,10 +1,21 @@
-@import "{ohif:design}/app"
+@require '{ohif:design}/app'
 
 .measurementTableTimepointCell
-	theme('border-left', '%s solid $uiBorderColor' % $uiBorderThickness)
-	theme('color', '$textPrimaryColor')
-	padding: 0 10px
-	cursor: pointer
+  theme('border-left', '%s solid $uiBorderColor' % $uiBorderThickness)
+  theme('color', '$textPrimaryColor')
+  cursor: pointer
+  padding: 0 10px
+  position: relative
 
-	&:hover
-		theme('color', '$activeColor')
+  .loading-spinner
+    display: none
+    font-size: 16px
+    position: absolute
+    right: 5px
+    top: 3px
+
+  &.loading .loading-spinner
+    display: block
+
+  &:hover
+    theme('color', '$activeColor')
diff --git a/Packages/ohif-measurements/client/lib/activateMeasurements.js b/Packages/ohif-measurements/client/lib/activateMeasurements.js
index e5e0a6624..b23330a42 100644
--- a/Packages/ohif-measurements/client/lib/activateMeasurements.js
+++ b/Packages/ohif-measurements/client/lib/activateMeasurements.js
@@ -77,31 +77,9 @@ OHIF.measurements.activateMeasurements = (element, measurementData) => {
         cornerstoneTools.scrollToIndex(element, imageIdIndex);
     }
 
-    // TODO: Find another way to do this?
-    // This might update one element twice, but at least it makes sure all viewports are
-    // updated and the highlight is removed from inactive tools in all visible viewports
-    const $viewports = $('.imageViewerViewport');
-    $viewports.each((index, element) => {
-        const $element = $(element);
-        if (!$element.find('canvas')) {
-            return;
-        }
+    const $element = $(element);
+    if (!$element.find('canvas').length) return;
 
-        // TODO: Implement isEnabledElement in Cornerstone
-        // or maybe just remove the 'error' this throws?
-        let ee;
-        try {
-            ee = cornerstone.getEnabledElement(element);
-        } catch(error) {
-            OHIF.log.warn(error);
-            return;
-        }
-
-        if (!ee.image) {
-            return;
-        }
-
-        $element.trigger('ViewerMeasurementsActivated');
-        cornerstone.updateImage(element);
-    });
+    $element.trigger('ViewerMeasurementsActivated');
+    cornerstone.updateImage(element);
 };
diff --git a/Packages/ohif-measurements/client/lib/jumpToRowItem.js b/Packages/ohif-measurements/client/lib/jumpToRowItem.js
index b5a187412..619f83d78 100644
--- a/Packages/ohif-measurements/client/lib/jumpToRowItem.js
+++ b/Packages/ohif-measurements/client/lib/jumpToRowItem.js
@@ -1,43 +1,42 @@
 import { $ } from 'meteor/jquery';
 import { _ } from 'meteor/underscore';
 import { OHIF } from 'meteor/ohif:core';
+import { cornerstone } from 'meteor/ohif:cornerstone';
 
-function renderIntoViewport(viewportIndex, studyInstanceUid, seriesInstanceUid, sopInstanceUid, renderedCallback) {
-    // @TypeSafeStudies
-    // First, check if we already have this study loaded
-    const alreadyLoadedStudy = OHIF.viewer.Studies.findBy({ studyInstanceUid });
+function renderIntoViewport(measurementData, enabledElement, viewportIndex) {
+    const { activateMeasurements, findAndRenderDisplaySet } = OHIF.measurements;
+    const { studyInstanceUid, seriesInstanceUid, sopInstanceUid } = measurementData;
+    const { element } = enabledElement;
 
-    const { findAndRenderDisplaySet } = OHIF.measurements;
-    if (alreadyLoadedStudy) {
-        // If the Study is already loaded, find the display set and render it
-        findAndRenderDisplaySet(alreadyLoadedStudy.displaySets, viewportIndex, studyInstanceUid, seriesInstanceUid, sopInstanceUid, renderedCallback);
-    } else {
-        // If not, retrieve the study metadata and then find the relevant display set and
-        // render it.
-        const $viewports = $('.imageViewerViewport');
-        const element = $viewports.get(viewportIndex);
-        const startLoadingHandler = cornerstoneTools.loadHandlerManager.getStartLoadHandler();
-        startLoadingHandler(element);
-        OHIF.studies.retrieveStudyMetadata(studyInstanceUid).then(study => {
-            OHIF.log.warn('renderIntoViewport');
+    return new Promise((resolve, reject) => {
+        const renderedCallback = element => {
+            activateMeasurements(element, measurementData);
+            $(element).one('CornerstoneImageRendered', () => resolve());
+        };
 
-            // Double check to make sure this study wasn't already inserted
-            // into OHIF.viewer.Studies, so we don't cause duplicate entry errors
-            const loaded = OHIF.viewer.Studies.findBy({
-                studyInstanceUid: study.studyInstanceUid
-            });
-            if (!loaded) {
-                OHIF.viewer.Studies.insert(study);
+        // Check if the study / series we need is already the one in the viewport
+        if (enabledElement && enabledElement.image) {
+            const imageId = enabledElement.image.imageId;
+            const series = cornerstone.metaData.get('series', imageId);
+            const study = cornerstone.metaData.get('study', imageId);
+
+            const isSameStudy = study.studyInstanceUid === measurementData.studyInstanceUid;
+            const isSameSeries = series.seriesInstanceUid === measurementData.seriesInstanceUid;
+            if (isSameStudy && isSameSeries) {
+                // If it is, activate the measurements in this viewport and stop here
+                renderedCallback(element);
             }
+        }
 
-            findAndRenderDisplaySet(study.displaySets, viewportIndex, studyInstanceUid, seriesInstanceUid, sopInstanceUid, renderedCallback);
-        }).catch(error => {
-            OHIF.log.error(`There was an error trying to retrieve the study\'s metadata for studyInstanceUid: ${studyInstanceUid}`);
-            OHIF.log.error(error.stack);
+        // Otherwise, re-render the viewport with the required study/series, then add an rendered
+        // callback to activate the measurements
 
-            OHIF.log.trace();
-        });
-    }
+        // @TypeSafeStudies
+        const study = OHIF.viewer.Studies.findBy({ studyInstanceUid });
+
+        // TODO: Support frames? e.g. for measurements on multi-frame instances
+        findAndRenderDisplaySet(study.displaySets, viewportIndex, studyInstanceUid, seriesInstanceUid, sopInstanceUid, renderedCallback);
+    });
 }
 
 function syncViewports(viewportsIndexes) {
@@ -52,98 +51,85 @@ function syncViewports(viewportsIndexes) {
     }
 }
 
+// Store the lastActivatedRowItem to cancel jumping if another rowItem was triggered during loading
+let lastActivatedRowItem;
+
 /**
  * Activates a set of lesions when lesion table row is clicked
  *
  * @param measurementId The unique key for a specific Measurement
  */
 OHIF.measurements.jumpToRowItem = (rowItem, timepoints) => {
-    OHIF.measurements.deactivateAllToolData();
-
-    const activateMeasurements = OHIF.measurements.activateMeasurements;
-    const activatedViewportIndexes = [];
-    let syncViewportsCaller = syncViewports;
-    let renderCount = 0;
-
-    // Deactivate stack synchronizer because it will be re-activated later
-    OHIF.viewer.stackImagePositionOffsetSynchronizer.deactivate();
-
-    // Retrieve the timepoints that are currently being displayed in the
-    // Measurement Table
-    const numTimepoints = Math.max(timepoints.length, 1);
+    lastActivatedRowItem = rowItem;
 
     // Retrieve the list of available viewports
     const $viewports = $('.imageViewerViewport');
     const numViewports = Math.max($viewports.length, 0);
 
+    // Retrieve the timepoints that are currently being displayed in the  Measurement Table
+    const numTimepoints = Math.max(timepoints.length, 1);
+
     const numViewportsToUpdate = Math.min(numTimepoints, numViewports);
 
-    //  Display timepoints in the order of viewports which is set by the hanging protocol
-    //      Reverse the timepoint array if the first timepoint does not match with the first viewport data
-    const layoutManager = OHIF.viewerbase.layoutManager;
-    if (timepoints[0].studyInstanceUids.indexOf(layoutManager.viewportData[0].studyInstanceUid) < 0) {
-        timepoints.reverse();
-    }
-
+    // Retrieve the measurements data
+    const measurementsData = [];
+    const promises = new Set();
     for (let i = 0; i < numViewportsToUpdate; i++) {
-        const timepoint = timepoints[i];
-        const timepointId = timepoint.timepointId;
+        const { timepointId } = timepoints[i];
 
         const dataAtThisTimepoint = _.where(rowItem.entries, { timepointId });
-        if (!dataAtThisTimepoint || !dataAtThisTimepoint.length) {
-            continue;
-        }
+        if (!dataAtThisTimepoint || !dataAtThisTimepoint.length) continue;
 
         const measurementData = dataAtThisTimepoint[0];
+        measurementsData.push(measurementData);
+        const promise = OHIF.studies.loadStudy(measurementData.studyInstanceUid);
+        promise.then(() => OHIF.measurements.syncMeasurementAndToolData(measurementData));
+        promises.add(promise);
+    }
 
-        // Check if the study / series we need is already the one in the viewport
-        const element = $viewports.get(i);
+    // Wait for studies metadata to be retrieved before jumpint to the given row item
+    Promise.all(promises).then(() => {
+        // Stop here if another rowItem was activated during loading process
+        if (rowItem !== lastActivatedRowItem) return;
 
-        activatedViewportIndexes.push(i);
+        OHIF.measurements.deactivateAllToolData();
 
-        // TODO: Implement isEnabledElement in Cornerstone
-        // or maybe just remove the 'error' this throws?
-        let enabledElement;
-        try {
-            enabledElement = cornerstone.getEnabledElement(element);
-        } catch(error) {
-            continue;
+        const activatedViewportIndexes = [];
+
+        // Deactivate stack synchronizer because it will be re-activated later
+        OHIF.viewer.stackImagePositionOffsetSynchronizer.deactivate();
+
+        //  Display timepoints in the order of viewports which is set by the hanging protocol
+        //  Reverse the array if the first timepoint does not match with the first viewport data
+        const layoutManager = OHIF.viewerbase.layoutManager;
+        const viewportData = layoutManager.viewportData[0];
+        if (timepoints[0].studyInstanceUids.indexOf(viewportData.studyInstanceUid) < 0) {
+            timepoints.reverse();
         }
 
-        if (enabledElement && enabledElement.image) {
-            const imageId = enabledElement.image.imageId;
-            const series = cornerstone.metaData.get('series', imageId);
-            const study = cornerstone.metaData.get('study', imageId);
+        const renderPromises = [];
+        for (let viewportIndex = 0; viewportIndex < numViewportsToUpdate; viewportIndex++) {
+            const measurementData = measurementsData[viewportIndex];
+            if (!measurementData) continue;
 
-            if (series.seriesInstanceUid === measurementData.seriesInstanceUid &&
-                study.studyInstanceUid === measurementData.studyInstanceUid) {
+            activatedViewportIndexes.push(viewportIndex);
 
-                // If it is, activate the measurements in this viewport and stop here
-                activateMeasurements(element, measurementData);
+            const element = $viewports.get(viewportIndex);
+
+            // TODO: Implement isEnabledElement in Cornerstone
+            // or maybe just remove the 'error' this throws?
+            let enabledElement;
+            try {
+                enabledElement = cornerstone.getEnabledElement(element);
+            } catch(error) {
                 continue;
             }
+
+            const promise = renderIntoViewport(measurementData, enabledElement, viewportIndex);
+            renderPromises.push(promise);
         }
 
-        // The sync will be called only after loading all series on viewports
-        syncViewportsCaller = _.after(++renderCount, syncViewports);
-
-        // Otherwise, re-render the viewport with the required study/series, then
-        // add an onRendered callback to activate the measurements
-        const renderedCallback = element => {
-            activateMeasurements(element, measurementData);
-            syncViewportsCaller(activatedViewportIndexes);
-        };
-
-        // TODO: Support frames? e.g. for measurements on multi-frame instances
-        renderIntoViewport(i,
-                           measurementData.studyInstanceUid,
-                           measurementData.seriesInstanceUid,
-                           measurementData.sopInstanceUid,
-                           renderedCallback);
-    }
-
-    // If all viewports are already rendered then sync them
-    if (!renderCount) {
-        syncViewportsCaller(activatedViewportIndexes);
-    }
+        // Wait for all viewports to be rendered then sync them
+        Promise.all(renderPromises).then(() => syncViewports(activatedViewportIndexes));
+    });
 };
diff --git a/Packages/ohif-studies/imports/client/components/browser/item.js b/Packages/ohif-studies/imports/client/components/browser/item.js
index 8075cf20a..8003f6c08 100644
--- a/Packages/ohif-studies/imports/client/components/browser/item.js
+++ b/Packages/ohif-studies/imports/client/components/browser/item.js
@@ -34,7 +34,7 @@ Template.studyBrowserItem.onCreated(() => {
     instance.autorun(() => {
         const instance = Template.instance();
 
-        OHIF.studies.loadedDict.get(studyInstanceUid);
+        OHIF.studies.loadingDict.get(studyInstanceUid);
         const studyMetadata = instance.getStudyMetadata();
         if (studyMetadata) {
             const firstInstance = studyMetadata.getFirstInstance();
diff --git a/Packages/ohif-studies/imports/client/lib/loadStudy.js b/Packages/ohif-studies/imports/client/lib/loadStudy.js
index db0cb6f51..f0e3bfe02 100644
--- a/Packages/ohif-studies/imports/client/lib/loadStudy.js
+++ b/Packages/ohif-studies/imports/client/lib/loadStudy.js
@@ -1,8 +1,9 @@
 import { ReactiveDict } from 'meteor/reactive-dict';
+import { Tracker } from 'meteor/tracker';
 import { OHIF } from 'meteor/ohif:core';
 
-// Create a studies loaded state dictionary to enable reactivity
-OHIF.studies.loadedDict = new ReactiveDict();
+// Create a studies loaded state dictionary to enable reactivity. Values: loading|loaded|failed
+OHIF.studies.loadingDict = new ReactiveDict();
 
 /**
  * Load the study metadata and store its information locally
@@ -11,7 +12,18 @@ OHIF.studies.loadedDict = new ReactiveDict();
  * @returns {Promise} that will be resolved with the study metadata or rejected with an error
  */
 OHIF.studies.loadStudy = studyInstanceUid => new Promise((resolve, reject) => {
-    OHIF.studies.retrieveStudyMetadata(studyInstanceUid).then(study => {
+    // Disable reactivity to get the current loading state
+    let currentLoadingState;
+    Tracker.nonreactive(() => {
+        currentLoadingState = OHIF.studies.loadingDict.get(studyInstanceUid) || '';
+    });
+
+    // Set the loading state as the study is not yet loaded
+    if (currentLoadingState !== 'loading') {
+        OHIF.studies.loadingDict.set(studyInstanceUid, 'loading');
+    }
+
+    return OHIF.studies.retrieveStudyMetadata(studyInstanceUid).then(study => {
         // Add the display sets to the study if not present
         if (!study.displaySets) {
             const displaySets = OHIF.viewerbase.sortingManager.getDisplaySets(study);
@@ -30,13 +42,16 @@ OHIF.studies.loadStudy = studyInstanceUid => new Promise((resolve, reject) => {
             OHIF.viewer.StudyMetadataList.insert(study);
         }
 
-        // Add the study to the loading listener to display loading progress on series thumbnails
+        // Add the study to the loading listener to allow loading progress handling
         const studyLoadingListener = OHIF.viewerbase.StudyLoadingListener.getInstance();
         studyLoadingListener.addStudy(study);
 
         // Add the studyInstanceUid to the loaded state dictionary
-        OHIF.studies.loadedDict.set(study.studyInstanceUid, true);
+        OHIF.studies.loadingDict.set(studyInstanceUid, 'loaded');
 
         resolve(study);
-    }).catch(reject);
+    }).catch((...args) => {
+        OHIF.studies.loadingDict.set(studyInstanceUid, 'failed');
+        reject(args);
+    });
 });