fix: Hanging protocol state fixes (#3242)

* fix: Some residual issues with hanging protocol after state sync

fix: Some issues introduced by the state syncing changes

* fix: PR fixes, mostly code cleanup

* Improve a race condition in an automated test

* Remove obsolete code

* PR fixes

extensions/cornerstone/src/Viewport/OHIFCornerstoneViewport.tsx

@@ -220,15 +220,27 @@ const OHIFCornerstoneViewport = React.memo(props => {
     const currentPresentation = cornerstoneViewportService.getPresentation(
       viewportIndex
     );
-    const { presentationSync } = stateSyncService.getState();
-    if (currentPresentation) {
-      stateSyncService.store({
-        presentationSync: {
-          ...presentationSync,
-          [currentPresentation.id]: currentPresentation,
-        },
-      });
+    if (!currentPresentation || !currentPresentation.presentationIds) return;
+    const {
+      lutPresentationStore,
+      positionPresentationStore,
+    } = stateSyncService.getState();
+    const { presentationIds } = currentPresentation;
+    const { lutPresentationId, positionPresentationId } = presentationIds || {};
+    const storeState = {};
+    if (lutPresentationId) {
+      storeState.lutPresentationStore = {
+        ...lutPresentationStore,
+        [lutPresentationId]: currentPresentation,
+      };
     }
+    if (positionPresentationId) {
+      storeState.positionPresentationStore = {
+        ...positionPresentationStore,
+        [positionPresentationId]: currentPresentation,
+      };
+    }
+    stateSyncService.store(storeState);
   };
 
   const cleanUpServices = useCallback(() => {
@@ -384,18 +396,25 @@ const OHIFCornerstoneViewport = React.memo(props => {
 
       storePresentation();
 
-      const { presentationSync } = stateSyncService.getState();
-      const { presentationId } = viewportOptions;
-      const presentation = presentationId
-        ? (presentationSync[presentationId] as Presentation)
-        : null;
+      const {
+        lutPresentationStore,
+        positionPresentationStore,
+      } = stateSyncService.getState();
+      const { presentationIds } = viewportOptions;
+      const presentations = {
+        positionPresentation:
+          positionPresentationStore[presentationIds?.positionPresentationId],
+        lutPresentation:
+          lutPresentationStore[presentationIds?.lutPresentationId],
+      };
+      console.log('Using presentations', presentations);
 
       cornerstoneViewportService.setViewportData(
         viewportIndex,
         viewportData,
         viewportOptions,
         displaySetOptions,
-        presentation
+        presentations
       );
     };
 

extensions/cornerstone/src/init.tsx

@@ -98,9 +98,15 @@ export default async function init({
     _showCPURenderingModal(uiModalService, hangingProtocolService);
   }
 
-  // Stores a map from `presentationId` to a Presentation object so that
-  // an OHIFCornerstoneViewport can be redisplayed with the same attributes
-  stateSyncService.register('presentationSync', { clearOnModeExit: true });
+  // Stores a map from `lutPresentationId` to a Presentation object so that
+  // an OHIFCornerstoneViewport can be redisplayed with the same LUT
+  stateSyncService.register('lutPresentationStore', { clearOnModeExit: true });
+
+  // Stores a map from `positionPresentationId` to a Presentation object so that
+  // an OHIFCornerstoneViewport can be redisplayed with the same position
+  stateSyncService.register('positionPresentationStore', {
+    clearOnModeExit: true,
+  });
 
   const labelmapRepresentation =
     cornerstoneTools.Enums.SegmentationRepresentations.Labelmap;

extensions/cornerstone/src/services/SegmentationService/SegmentationService.ts

@@ -1591,7 +1591,7 @@ class SegmentationService {
 
     segmentInfo.isVisible = isVisible;
 
-    cstSegmentation.config.visibility.setVisibilityForSegmentIndex(
+    cstSegmentation.config.visibility.setSegmentVisibility(
       toolGroupId,
       segmentationRepresentationUID,
       segmentIndex,

extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts

@@ -21,11 +21,7 @@ import {
   StackViewportData,
   VolumeViewportData,
 } from '../../types/CornerstoneCacheService';
-import {
-  Presentation,
-  StackPresentation,
-  VolumePresentation,
-} from '../../types/Presentation';
+import { Presentation, Presentations } from '../../types/Presentation';
 import {
   setColormap,
   setLowerUpperColorTransferFunction,
@@ -173,23 +169,30 @@ class CornerstoneViewportService extends PubSubService
     this.viewportsById.delete(viewportId);
   }
 
+  public setPresentations(viewport, presentations?: Presentations): void {
+    const properties = presentations.lutPresentation?.properties;
+    if (properties) viewport.setProperties(properties);
+    const camera = presentations.positionPresentation?.camera;
+    if (camera) viewport.setCamera(camera);
+  }
+
   public getPresentation(viewportIndex: number): Presentation {
     const viewportInfo = this.viewportsInfo.get(viewportIndex);
     if (!viewportInfo) return;
-    const {
-      presentationId: id,
-      viewportType,
-    } = viewportInfo.getViewportOptions();
-    if (!id) return;
+    const { viewportType, presentationIds } = viewportInfo.getViewportOptions();
 
     const csViewport = this.getCornerstoneViewportByIndex(viewportIndex);
     if (!csViewport) return;
 
     const properties = csViewport.getProperties();
+    if (properties.isComputedVOI) {
+      delete properties.voiRange;
+      delete properties.VOILUTFunction;
+    }
     const initialImageIndex = csViewport.getCurrentImageIdIndex();
     const camera = csViewport.getCamera();
     return {
-      id,
+      presentationIds,
       viewportType:
         !viewportType || viewportType === 'stack' ? 'stack' : 'volume',
       properties,
@@ -211,7 +214,7 @@ class CornerstoneViewportService extends PubSubService
     viewportData: StackViewportData | VolumeViewportData,
     publicViewportOptions: PublicViewportOptions,
     publicDisplaySetOptions: DisplaySetOptions[],
-    presentation?: Presentation
+    presentations?: Presentations
   ): void {
     const renderingEngine = this.getRenderingEngine();
     const viewportId =
@@ -276,7 +279,7 @@ class CornerstoneViewportService extends PubSubService
     renderingEngine.enableElement(viewportInput);
 
     const viewport = renderingEngine.getViewport(viewportId);
-    this._setDisplaySets(viewport, viewportData, viewportInfo, presentation);
+    this._setDisplaySets(viewport, viewportData, viewportInfo, presentations);
   }
 
   public getCornerstoneViewport(
@@ -340,7 +343,7 @@ class CornerstoneViewportService extends PubSubService
     viewport: Types.IStackViewport,
     viewportData: StackViewportData,
     viewportInfo: ViewportInfo,
-    presentation?: StackPresentation
+    presentations: Presentations
   ): void {
     const displaySetOptions = viewportInfo.getDisplaySetOptions();
 
@@ -353,7 +356,8 @@ class CornerstoneViewportService extends PubSubService
     this.viewportsDisplaySets.set(viewport.id, [displaySetInstanceUID]);
 
     let initialImageIndexToUse =
-      presentation?.initialImageIndex ?? initialImageIndex;
+      presentations?.positionPresentation?.initialImageIndex ??
+      initialImageIndex;
 
     if (
       initialImageIndexToUse === undefined ||
@@ -363,8 +367,8 @@ class CornerstoneViewportService extends PubSubService
         this._getInitialImageIndexForStackViewport(viewportInfo, imageIds) || 0;
     }
 
-    const properties = presentation?.properties || {};
-    if (!presentation?.properties) {
+    const properties = { ...presentations.lutPresentation?.properties };
+    if (!presentations.lutPresentation?.properties) {
       const { voi, voiInverted } = displaySetOptions[0];
       if (voi && (voi.windowWidth || voi.windowCenter)) {
         const { lower, upper } = csUtils.windowLevel.toLowHighRange(
@@ -385,7 +389,8 @@ class CornerstoneViewportService extends PubSubService
       // The scroll, however, works fine in CS3D
       viewport.scroll(initialImageIndexToUse);
       viewport.setProperties(properties);
-      if (presentation?.camera) viewport.setCamera(presentation.camera);
+      const camera = presentations.positionPresentation?.camera;
+      if (camera) viewport.setCamera(camera);
     });
   }
 
@@ -441,7 +446,7 @@ class CornerstoneViewportService extends PubSubService
     viewport: Types.IVolumeViewport,
     viewportData: VolumeViewportData,
     viewportInfo: ViewportInfo,
-    presentation: VolumePresentation
+    presentations: Presentations
   ): Promise<void> {
     // TODO: We need to overhaul the way data sources work so requests can be made
     // async. I think we should follow the image loader pattern which is async and
@@ -514,10 +519,18 @@ class CornerstoneViewportService extends PubSubService
     });
 
     // This returns the async continuation only
-    return this.setVolumesForViewport(viewport, volumeInputArray, presentation);
+    return this.setVolumesForViewport(
+      viewport,
+      volumeInputArray,
+      presentations
+    );
   }
 
-  public async setVolumesForViewport(viewport, volumeInputArray, presentation) {
+  public async setVolumesForViewport(
+    viewport,
+    volumeInputArray,
+    presentations
+  ) {
     const {
       displaySetService,
       segmentationService,
@@ -525,9 +538,7 @@ class CornerstoneViewportService extends PubSubService
     } = this.servicesManager.services;
 
     await viewport.setVolumes(volumeInputArray);
-    const { properties, camera } = presentation || {};
-    if (properties) viewport.setProperties(properties);
-    if (camera) viewport.setCamera(camera);
+    this.setPresentations(viewport, presentations);
 
     // load any secondary displaySets
     const displaySetInstanceUIDs = this.viewportsDisplaySets.get(viewport.id);
@@ -703,21 +714,21 @@ class CornerstoneViewportService extends PubSubService
     viewport: StackViewport | VolumeViewport,
     viewportData: StackViewportData | VolumeViewportData,
     viewportInfo: ViewportInfo,
-    presentation?: Presentation
+    presentations: Presentations = {}
   ): void {
     if (viewport instanceof StackViewport) {
       this._setStackViewport(
         viewport,
         viewportData as StackViewportData,
         viewportInfo,
-        presentation as StackPresentation
+        presentations
       );
     } else if (viewport instanceof VolumeViewport) {
       this._setVolumeViewport(
         viewport,
         viewportData as VolumeViewportData,
         viewportInfo,
-        presentation as VolumePresentation
+        presentations
       );
     } else {
       throw new Error('Unknown viewport type');

extensions/cornerstone/src/services/ViewportService/Viewport.ts

@@ -1,4 +1,5 @@
 import { Types, Enums } from '@cornerstonejs/core';
+import { Types as UITypes } from '@ohif/ui';
 import getCornerstoneBlendMode from '../../utils/getCornerstoneBlendMode';
 import getCornerstoneOrientation from '../../utils/getCornerstoneOrientation';
 import getCornerstoneViewportType from '../../utils/getCornerstoneViewportType';
@@ -15,12 +16,13 @@ export type InitialImageOptions = {
 };
 
 export type ViewportOptions = {
+  id?: string;
   viewportType: Enums.ViewportType;
   toolGroupId: string;
   viewportId: string;
   // Presentation ID to store/load presentation state from
-  presentationId?: string;
-  orientation?: Types.Orientation;
+  presentationIds?: UITypes.PresentationIds;
+  orientation?: Enums.OrientationAxis;
   background?: Types.Point3;
   syncGroups?: SyncGroup[];
   initialImageOptions?: InitialImageOptions;
@@ -33,11 +35,12 @@ export type ViewportOptions = {
 };
 
 export type PublicViewportOptions = {
+  id?: string;
   viewportType?: string;
   toolGroupId?: string;
-  presentationId?: string;
+  presentationIds?: UITypes.PresentationIds;
   viewportId?: string;
-  orientation?: string;
+  orientation?: Enums.OrientationAxis;
   background?: Types.Point3;
   syncGroups?: SyncGroup[];
   initialImageOptions?: InitialImageOptions;
@@ -51,6 +54,10 @@ export type DisplaySetSelector = {
 };
 
 export type PublicDisplaySetOptions = {
+  /** The display set options can have an id in order to distinguish
+   * it from other similar items.
+   */
+  id?: string;
   voi?: VOI;
   voiInverted?: boolean;
   blendMode?: string;
@@ -59,6 +66,7 @@ export type PublicDisplaySetOptions = {
 };
 
 export type DisplaySetOptions = {
+  id?: string;
   voi?: VOI;
   voiInverted: boolean;
   blendMode?: Enums.BlendModes;
@@ -177,7 +185,7 @@ class ViewportInfo {
     let viewportType = viewportOptionsEntry.viewportType;
     const {
       toolGroupId = DEFAULT_TOOLGROUP_ID,
-      presentationId,
+      presentationIds,
     } = viewportOptionsEntry;
     let orientation;
 
@@ -202,7 +210,7 @@ class ViewportInfo {
       viewportType: viewportType as Enums.ViewportType,
       orientation,
       toolGroupId,
-      presentationId,
+      presentationIds,
     });
   }
 

extensions/cornerstone/src/tools/CalibrationLineTool.ts

@@ -1,9 +1,10 @@
 import { metaData } from '@cornerstonejs/core';
-import { LengthTool } from '@cornerstonejs/tools';
-import { calibrateImageSpacing } from '@cornerstonejs/tools/dist/esm/utilities';
+import { LengthTool, utilities } from '@cornerstonejs/tools';
 import callInputDialog from '../utils/callInputDialog';
 import getActiveViewportEnabledElement from '../utils/getActiveViewportEnabledElement';
 
+const { calibrateImageSpacing } = utilities;
+
 /**
  * Calibration Line tool works almost the same as the
  */

extensions/cornerstone/src/types/Presentation.ts

@@ -1,23 +1,23 @@
 /** Store presentation data for either stack viewports or volume viewports */
 import { Types } from '@cornerstonejs/core';
+import { Types as UITypes } from '@ohif/ui';
 
-export interface BasePresentation {
-  id: string;
-  properties: Record<string, unknown>;
-  initialImageIndex?: number;
+/**
+ * Has information on the presentation of the viewport.
+ */
+export interface Presentation extends Types.StackViewportProperties {
+  presentationIds: UITypes.PresentationIds;
+  viewportType: string;
+  initialImageIndex: number;
   camera: Types.ICamera;
+  properties: Types.StackViewportProperties | Types.VolumeViewportProperties;
+  zoom?: number;
+  pan?: [number, number];
 }
 
-export interface StackPresentation extends BasePresentation {
-  viewportType: 'stack';
-}
-
-export interface VolumePresentation extends BasePresentation {
-  viewportType: 'volume';
-}
-
-// Currently it seems like the entire presentation state can be shared between
-// Stack and Volume, but is setup to allow differences
-export type Presentation = StackPresentation | VolumePresentation;
+export type Presentations = {
+  positionPresentation?: Presentation;
+  lutPresentation?: Presentation;
+};
 
 export default Presentation;