fix(grid):Grid service wasn't being reset (#3068)

* fix(grid):Grid service wasn't being reset

* fix(service):Fix the initial service state

Services with mode specific state differed in internal state between
initial and subsequent load.  This fix address that structurally by
allows the mode to store/manage service state, but makes the
responsibility of service state central to the service.

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

@@ -136,7 +136,7 @@ class CornerstoneViewportService implements IViewportService {
   public destroy() {
     this._removeResizeObserver();
     this.viewportGridResizeObserver = null;
-    this.renderingEngine.destroy();
+    this.renderingEngine?.destroy?.();
     this.viewportsDisplaySets.clear();
     this.renderingEngine = null;
     cache.purgeCache();

modes/basic-dev-mode/src/index.js

@@ -140,7 +140,6 @@ function modeFactory({ modeConfiguration }) {
       } = servicesManager.services;
 
       ToolBarService.reset();
-      MeasurementService.clearMeasurements();
       ToolGroupService.destroy();
     },
     validationTags: {

modes/longitudinal/src/index.js

@@ -126,20 +126,16 @@ function modeFactory() {
       const {
         ToolGroupService,
         SyncGroupService,
-        MeasurementService,
         ToolBarService,
         SegmentationService,
         CornerstoneViewportService,
-        HangingProtocolService,
       } = servicesManager.services;
 
       ToolBarService.reset();
-      MeasurementService.clearMeasurements();
       ToolGroupService.destroy();
       SyncGroupService.destroy();
       SegmentationService.destroy();
       CornerstoneViewportService.destroy();
-      HangingProtocolService.reset();
     },
     validationTags: {
       study: [],

modes/tmtv/src/index.js

@@ -136,21 +136,17 @@ function modeFactory({ modeConfiguration }) {
       const {
         ToolGroupService,
         SyncGroupService,
-        MeasurementService,
         ToolBarService,
         SegmentationService,
         CornerstoneViewportService,
-        HangingProtocolService,
       } = servicesManager.services;
 
       unsubscriptions.forEach(unsubscribe => unsubscribe());
       ToolBarService.reset();
-      MeasurementService.clearMeasurements();
       ToolGroupService.destroy();
       SyncGroupService.destroy();
       SegmentationService.destroy();
       CornerstoneViewportService.destroy();
-      HangingProtocolService.reset();
     },
     validationTags: {
       study: [],

platform/core/src/extensions/ExtensionManager.js

@@ -40,6 +40,9 @@ export default class ExtensionManager {
       _extensionLifeCycleHooks,
     } = this;
 
+    // The onModeEnter of the service must occur BEFORE the extension
+    // onModeEnter in order to reset the state to a standard state
+    // before the extension restores and cached data.
     for (const service of Object.values(_servicesManager.services)) {
       service?.onModeEnter?.();
     }
@@ -65,10 +68,6 @@ export default class ExtensionManager {
       _extensionLifeCycleHooks,
     } = this;
 
-    for (const service of Object.values(_servicesManager.services)) {
-      service?.onModeExit?.();
-    }
-
     registeredExtensionIds.forEach(extensionId => {
       const onModeExit = _extensionLifeCycleHooks.onModeExit[extensionId];
 
@@ -79,6 +78,16 @@ export default class ExtensionManager {
         });
       }
     });
+
+    // The service onModeExit calls must occur after the extension ones
+    // so that extension ones can store/restore data.
+    for (const service of Object.values(_servicesManager.services)) {
+      try {
+        service?.onModeExit?.();
+      } catch (e) {
+        console.warn('onModeExit caught', e);
+      }
+    }
   }
 
   /**

platform/core/src/services/CustomizationService/CustomizationService.ts

@@ -104,7 +104,6 @@ export default class CustomizationService extends PubSubService {
 
     commands.forEach(({ commandName, commandOptions, context }) => {
       if (commandName) {
-        console.log('Running command', commandName);
         commandsManager.runCommand(
           commandName,
           {
@@ -128,7 +127,6 @@ export default class CustomizationService extends PubSubService {
     customizationId: string,
     customization: Customization
   ): void {
-    console.log('** Set mode customization', customizationId, customization);
     this.modeCustomizations[customizationId] = merge(
       this.modeCustomizations[customizationId] || {},
       customization
@@ -194,7 +192,6 @@ export default class CustomizationService extends PubSubService {
   }
 
   setGlobalCustomization(id: string, value: Customization): void {
-    console.log('*** Set global', id, value);
     this.globalCustomizations[id] = value;
     this._broadcastGlobalCustomizationModified();
   }
@@ -234,7 +231,6 @@ export default class CustomizationService extends PubSubService {
     if (!value) return;
     if (typeof value === 'string') {
       const extensionValue = this.findExtensionValue(value);
-      console.log('Adding extension values', value, extensionValue);
       this.addReferences(extensionValue);
     } else if (Array.isArray(value)) {
       this.addReferences(value, isGlobal);

platform/core/src/services/DicomMetadataStore/createStudyMetadata.js

@@ -49,7 +49,15 @@ function createStudyMetadata(StudyInstanceUID) {
       );
 
       if (existingSeries) {
-        existingSeries.instances.push(...instances);
+        // Only add instances not already present, so generate a map
+        // of existing instances and filter the to add by things
+        // already present.
+        const sopMap = {};
+        existingSeries.instances.forEach(
+          it => (sopMap[it.SOPInstanceUID] = it)
+        );
+        const newInstances = instances.filter(it => !sopMap[it.SOPInstanceUID]);
+        existingSeries.instances.push(...newInstances);
       } else {
         const series = createSeriesMetadata(instances);
         this.series.push(series);

platform/core/src/services/DisplaySetService/DisplaySetService.js

@@ -53,7 +53,12 @@ export default class DisplaySetService {
     const activeDisplaySets = this.activeDisplaySets;
 
     displaySets.forEach(displaySet => {
-      activeDisplaySets.push(displaySet);
+      // This test makes adding display sets an N^2 operation, so it might
+      // become important to do this in an efficient manner for large
+      // numbers of display sets.
+      if (!activeDisplaySets.includes(displaySet)) {
+        activeDisplaySets.push(displaySet);
+      }
     });
   }
 
@@ -194,6 +199,17 @@ export default class DisplaySetService {
     }
   };
 
+  /**
+   * The onModeExit returns the display set service to the initial state,
+   * that is without any display sets.  To avoid recreating display sets,
+   * the mode specific onModeExit is called before this method and should
+   * store the active display sets and the cached data.
+   */
+  onModeExit() {
+    this.getDisplaySetCache().length = 0;
+    this.activeDisplaySets.length = 0;
+  }
+
   makeDisplaySetForInstances(instancesSrc, settings) {
     let instances = instancesSrc;
     const instance = instances[0];

platform/core/src/services/HangingProtocolService/HangingProtocolService.ts

@@ -122,6 +122,11 @@ class HangingProtocolService {
     this.displaySetMatchDetails = new Map();
   }
 
+  /** Leave the hanging protocol in the initialized state */
+  public onModeExit() {
+    this.reset();
+  }
+
   public getActiveProtocol(): {
     protocol: HangingProtocol.Protocol;
     stage: number;

platform/core/src/services/MeasurementService/MeasurementService.js

@@ -570,6 +570,15 @@ class MeasurementService {
     this._broadcastEvent(this.EVENTS.MEASUREMENTS_CLEARED, { measurements });
   }
 
+  /**
+   * Called after the mode.onModeExit is called to reset the state.
+   * To store measurements for later use, store them in the mode.onModeExit
+   * and restore them in the mode onModeEnter.
+   */
+  onModeExit() {
+    this.clearMeasurements();
+  }
+
   jumpToMeasurement(viewportIndex, measurementUID) {
     const measurement = this.measurements[measurementUID];
 

platform/core/src/services/ViewportGridService/ViewportGridService.ts

@@ -96,6 +96,16 @@ class ViewportGridService {
     this.serviceImplementation._reset();
   }
 
+  /**
+   * The onModeExit must set the state of the viewport grid to a standard/clean
+   * state.  To implement store/recover of the viewport grid, perform
+   * a state store in the mode or extension onModeExit, and recover that
+   * data if appropriate in the onModeEnter of the mode or extension.
+   */
+  public onModeExit(): void {
+    this.serviceImplementation._onModeExit();
+  }
+
   public setCachedLayout({ cacheId, cachedLayout }) {
     this.serviceImplementation._setCachedLayout({ cacheId, cachedLayout });
   }

platform/docs/docs/platform/extensions/index.md

@@ -206,8 +206,10 @@ used to initialize data.
 
 [`onModeExit`](./lifecycle#onModeExit): Similarly to onModeEnter, this hook is
 called when navigating away from a mode, or before a mode’s data or datasource
-is changed. This can be used to clean up data (e.g. remove annotations that do
-not need to be persisted)
+is changed. This can be used to cache data for re-use later, but since it
+isn't known which mode will be entered next, the state after exiting should be
+clean, that is, the same as the state on a clean start.  This is called BEFORE
+service clean up, and after mode specific onModeExit handling.
 
 ## Modules
 

platform/docs/docs/platform/managers/service.md

@@ -178,3 +178,27 @@ import BackEndService from "../services/BackEndService/BackEndService";
 
 export { BackEndService };
 ```
+
+# Service Mode Lifecycle
+Services may implement initialization and cleanup for mode specific data.
+In order to prevent defects where there are differences between initial
+and subsequent displays of a study, the contract of the service is that the
+state the service is in on mode entry shall be the same whether the mode was
+entered or was exited and entered again.
+
+To implement storage/recovery of state, the mode must store the data on
+exiting the mode, and restore the data in it's onModeEnter.  For example,
+the mode may decide to preserve measurement data in the onModeExit, and
+to restore it in the onModeEnter.  This does not violate the contract since
+it is the mode's decision to apply the stored state, and to cache it.
+
+## onModeEnter
+A service may implement an onModeEnter call to initialize the service to
+be ready for entering a mode.
+This is called before the mode `onModeEnter` is called.
+
+## onModeExit
+When entering a mode, the service contract states that the service needs to
+be in the same state whether it is a fresh load or has previously entered the mode.
+The onModeExit allows a service to clean itself up after the mode 'onModeExit'
+has stored any persistent data.

platform/docs/docs/platform/modes/lifecycle.md

@@ -16,7 +16,11 @@ Currently, there are two hooks that are called for modes:
 
 This hook gets run after the defined route has been entered by the mode. This
 hook can be used to initialize the data, services and appearance of the viewer
-upon the first render.
+upon the first render, in any way that is custom to the mode.
+
+This is called after service `onModeEnter` calls so that the entry into a mode
+is done in a predefined/fixed state.  That allows any restoring of existing state
+to be performed.
 
 For instance, in `longitudinal` mode we are using this hook to initialize the
 `ToolBarService` and set the window level/width tool to be active and add
@@ -64,9 +68,13 @@ function modeFactory() {
 
 ## onModeExit
 
-This hook is called when the viewer navigate away from the route in the url.
-This is the place for cleaning up data, and services by unsubscribing to the
-events.
+This hook is called when the viewer navigates away from the route in the url.
+It is called BEFORE the service specific onModeExit calls are performed, and
+thus still has access to stateful data which can be cached or stored before
+the services clean themselves up.
+This is the place for cleaning up NON-service specific data, and services
+by unsubscribing to the events.  The cleanup of the service itself is intended
+to occur in the service `onModeEnter`.
 
 For instance, it can be used to reset the `ToolBarService` which reset the
 toggled buttons.

platform/ui/src/components/Button/Button.tsx

@@ -230,11 +230,13 @@ Button.propTypes = {
   color: PropTypes.oneOf([
     'default',
     'primary',
+    'primaryActive',
     'secondary',
     'white',
     'black',
     'inherit',
     'light',
+    'translucent',
   ]),
   border: PropTypes.oneOf([
     'none',

platform/viewer/src/components/ViewportGrid.tsx

@@ -452,6 +452,7 @@ function _getViewportComponent(
     }
   }
 
+  console.log("Can't show displaySet", SOPClassHandlerId, displaySets[0]);
   UINotificationService.show({
     title: 'Viewport Not Supported Yet',
     message: `Cannot display SOPClassId of ${displaySets[0].SOPClassUID} yet`,

platform/viewer/src/routes/Mode/Mode.tsx

@@ -312,11 +312,22 @@ export default function ModeRoute({
     });
 
     return () => {
-      extensionManager.onModeExit();
-      mode?.onModeExit({ servicesManager, extensionManager });
+      // The mode.onModeExit must be done first to allow it to store
+      // information, and must be in a try/catch to ensure subscriptions
+      // are unsubscribed.
+      try {
+        mode?.onModeExit?.({ servicesManager, extensionManager });
+      } catch (e) {
+        console.warn('mode exit failure', e);
+      }
+      // The unsubscriptions must occur before the extension onModeExit
+      // in order to prevent exceptions during cleanup caused by spurious events
       unsubscriptions.forEach(unsub => {
         unsub();
       });
+      // The extension manager must be called after the mode, this is
+      // expected to cleanup the state to a standard setup.
+      extensionManager.onModeExit();
     };
   }, [
     mode,