feat: Add more tests to Synchronizer and ToolGroup managers (#217)

* feat: Add more tests to Synchronizer and ToolGroup managers * feat: Add more documentation to usages * fix: review comments
cornerstonejs · Mar 21, 2022 · f22ae0f · f22ae0f
1 parent 8499c6f
commit f22ae0f
Show file tree

Hide file tree

Showing 13 changed files with 1,004 additions and 459 deletions.
diff --git a/packages/cornerstone-render/src/utilities/testUtils.js b/packages/cornerstone-render/src/utilities/testUtils.js
@@ -285,7 +285,7 @@ function compareImages(imageDataURL, baseline, outputName) {
   })
 }
 
-function _canvasPointsToPagePoints(DomCanvasElement, canvasPoint) {
+function canvasPointsToPagePoints(DomCanvasElement, canvasPoint) {
   const rect = DomCanvasElement.getBoundingClientRect()
   return [
     canvasPoint[0] + rect.left + window.pageXOffset,
@@ -310,7 +310,7 @@ function createNormalizedMouseEvent(imageData, index, canvas, viewport) {
   const tempWorld1 = imageData.indexToWorldVec3(index)
   const tempCanvasPoint1 = viewport.worldToCanvas(tempWorld1)
   const canvasPoint1 = tempCanvasPoint1.map((p) => Math.round(p))
-  const [pageX, pageY] = _canvasPointsToPagePoints(canvas, canvasPoint1)
+  const [pageX, pageY] = canvasPointsToPagePoints(canvas, canvasPoint1)
   const worldCoord = viewport.canvasToWorld(canvasPoint1)
 
   return {
@@ -330,6 +330,7 @@ const testUtils = {
   compareImages,
   downloadURI,
   createNormalizedMouseEvent,
+  canvasPointsToPagePoints,
 }
 
 export default testUtils
diff --git a/packages/cornerstone-tools/src/store/SynchronizerManager/createSynchronizer.ts b/packages/cornerstone-tools/src/store/SynchronizerManager/createSynchronizer.ts
@@ -6,11 +6,11 @@ function createSynchronizer(
   eventName: string,
   eventHandler: ISynchronizerEventHandler
 ): Synchronizer {
-  const toolGroupWithIdExists = state.synchronizers.some(
-    (tg) => tg.id === synchronizerId
+  const synchronizerWithSameIdExists = state.synchronizers.some(
+    (sync) => sync.id === synchronizerId
   )
 
-  if (toolGroupWithIdExists) {
+  if (synchronizerWithSameIdExists) {
     throw new Error(`Synchronizer with id '${synchronizerId}' already exists.`)
   }
 

diff --git a/packages/cornerstone-tools/src/synchronizers/callbacks/voiSyncCallback.ts b/packages/cornerstone-tools/src/synchronizers/callbacks/voiSyncCallback.ts
@@ -1,4 +1,9 @@
-import { getRenderingEngine, StackViewport, Types, VolumeViewport } from '@ohif/cornerstone-render'
+import {
+  getRenderingEngine,
+  StackViewport,
+  Types,
+  VolumeViewport,
+} from '@ohif/cornerstone-render'
 
 /**
  * @function cameraSyncCallback - Synchronizer callback to synchronize the voi of volumeActors of identical volumes
@@ -28,33 +33,33 @@ export default function voiSyncCallback(
   const tScene = renderingEngine.getScene(targetViewport.sceneUID)
 
   if (tScene && tScene.uid === sceneUID) {
-    // Same scene, no need to update.
+    // Same scene, no need to update since RGB transfer function gets updated.
     return
   }
 
   const tViewport = renderingEngine.getViewport(targetViewport.viewportUID)
 
   if (tViewport instanceof VolumeViewport) {
     const scene = renderingEngine.getScene(tViewport.sceneUID)
-    let volumeActor = scene.getVolumeActor(volumeUID)
+    const volumeActor = scene.getVolumeActor(volumeUID)
 
     // TODO: This may not be what we want. It is a fallback
     // for cases when we are syncing from a stack to a volume viewport
     //if (!volumeActor) {
-      // TODO: this is a bit confusing that this returns something different
+    // TODO: this is a bit confusing that this returns something different
     // than getVolumeActor(). We should change getVolumeActor() I think
     //  volumeActor = scene.getVolumeActors()[0].volumeActor
     //}
 
     if (volumeActor) {
       volumeActor
-      .getProperty()
-      .getRGBTransferFunction(0)
-      .setRange(range.lower, range.upper)
+        .getProperty()
+        .getRGBTransferFunction(0)
+        .setRange(range.lower, range.upper)
     }
   } else if (tViewport instanceof StackViewport) {
     tViewport.setProperties({
-      voiRange: range
+      voiRange: range,
     })
   } else {
     throw new Error('Viewport type not supported.')

diff --git a/packages/cornerstone-tools/src/tools/WindowLevelTool.ts b/packages/cornerstone-tools/src/tools/WindowLevelTool.ts
@@ -41,12 +41,12 @@ export default class WindowLevelTool extends BaseTool {
     const enabledElement = getEnabledElement(canvas)
     const { scene, sceneUID, viewportUID, viewport } = enabledElement
 
-    let volumeUID;
+    let volumeUID
     if (this.configuration && this.configuration.volumeUID) {
       volumeUID = this.configuration.volumeUID
     } else {
-      const defaultActor = viewport.getDefaultActor();
-      volumeUID = defaultActor.uid;
+      const defaultActor = viewport.getDefaultActor()
+      volumeUID = defaultActor.uid
     }
 
     let volumeActor
@@ -117,7 +117,7 @@ export default class WindowLevelTool extends BaseTool {
 
     // store the new range for viewport to preserve it during scrolling
     viewport.setProperties({
-      voiRange: newRange
+      voiRange: newRange,
     })
 
     viewport.render()
@@ -128,15 +128,16 @@ export default class WindowLevelTool extends BaseTool {
     const { dimensions, scalarData } = imageVolume
     const middleSliceIndex = Math.floor(dimensions[2] / 2)
 
-    if (!(imageVolume instanceof StreamingImageVolume)) {
-      return
-    }
+    // Todo: volume shouldn't only be streaming image volume, it can be imageVolume
+    // if (!(imageVolume instanceof StreamingImageVolume)) {
+    //   return
+    // }
 
-    const streamingVolume = <StreamingImageVolume>imageVolume
+    // const streamingVolume = <StreamingImageVolume>imageVolume
 
-    if (!streamingVolume.loadStatus.cachedFrames[middleSliceIndex]) {
-      return DEFAULT_IMAGE_DYNAMIC_RANGE
-    }
+    // if (!streamingVolume.loadStatus.cachedFrames[middleSliceIndex]) {
+    //   return DEFAULT_IMAGE_DYNAMIC_RANGE
+    // }
 
     const frameLength = dimensions[0] * dimensions[1]
     let bytesPerVoxel