diff --git a/extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts b/extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts index ee1b49387..82d53805f 100644 --- a/extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts +++ b/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(); diff --git a/modes/basic-dev-mode/src/index.js b/modes/basic-dev-mode/src/index.js index 54ce9ab68..e8b602442 100644 --- a/modes/basic-dev-mode/src/index.js +++ b/modes/basic-dev-mode/src/index.js @@ -140,7 +140,6 @@ function modeFactory({ modeConfiguration }) { } = servicesManager.services; ToolBarService.reset(); - MeasurementService.clearMeasurements(); ToolGroupService.destroy(); }, validationTags: { diff --git a/modes/longitudinal/src/index.js b/modes/longitudinal/src/index.js index 28051889c..28c7b5814 100644 --- a/modes/longitudinal/src/index.js +++ b/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: [], diff --git a/modes/tmtv/src/index.js b/modes/tmtv/src/index.js index fdbf697ed..cb927d6e2 100644 --- a/modes/tmtv/src/index.js +++ b/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: [], diff --git a/platform/core/src/extensions/ExtensionManager.js b/platform/core/src/extensions/ExtensionManager.js index aca983790..e72c72233 100644 --- a/platform/core/src/extensions/ExtensionManager.js +++ b/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); + } + } } /** diff --git a/platform/core/src/services/CustomizationService/CustomizationService.ts b/platform/core/src/services/CustomizationService/CustomizationService.ts index 376cab1b3..ee8a55ef4 100644 --- a/platform/core/src/services/CustomizationService/CustomizationService.ts +++ b/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); diff --git a/platform/core/src/services/DicomMetadataStore/createStudyMetadata.js b/platform/core/src/services/DicomMetadataStore/createStudyMetadata.js index ce59aff66..4822b616c 100644 --- a/platform/core/src/services/DicomMetadataStore/createStudyMetadata.js +++ b/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); diff --git a/platform/core/src/services/DisplaySetService/DisplaySetService.js b/platform/core/src/services/DisplaySetService/DisplaySetService.js index 2f4357bd9..da3813a3b 100644 --- a/platform/core/src/services/DisplaySetService/DisplaySetService.js +++ b/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]; diff --git a/platform/core/src/services/HangingProtocolService/HangingProtocolService.ts b/platform/core/src/services/HangingProtocolService/HangingProtocolService.ts index 216bf80bd..2a1a7230f 100644 --- a/platform/core/src/services/HangingProtocolService/HangingProtocolService.ts +++ b/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; diff --git a/platform/core/src/services/MeasurementService/MeasurementService.js b/platform/core/src/services/MeasurementService/MeasurementService.js index f7258941d..a0955e86a 100644 --- a/platform/core/src/services/MeasurementService/MeasurementService.js +++ b/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]; diff --git a/platform/core/src/services/ViewportGridService/ViewportGridService.ts b/platform/core/src/services/ViewportGridService/ViewportGridService.ts index bf8d0c039..0700afa00 100644 --- a/platform/core/src/services/ViewportGridService/ViewportGridService.ts +++ b/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 }); } diff --git a/platform/docs/docs/platform/extensions/index.md b/platform/docs/docs/platform/extensions/index.md index 97a7e7647..d4637fde1 100644 --- a/platform/docs/docs/platform/extensions/index.md +++ b/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 diff --git a/platform/docs/docs/platform/managers/service.md b/platform/docs/docs/platform/managers/service.md index 849a79d40..8307d1c68 100644 --- a/platform/docs/docs/platform/managers/service.md +++ b/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. diff --git a/platform/docs/docs/platform/modes/lifecycle.md b/platform/docs/docs/platform/modes/lifecycle.md index 4993c30f5..92edeed5d 100644 --- a/platform/docs/docs/platform/modes/lifecycle.md +++ b/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. diff --git a/platform/ui/src/components/Button/Button.tsx b/platform/ui/src/components/Button/Button.tsx index 39888c01c..104db9368 100644 --- a/platform/ui/src/components/Button/Button.tsx +++ b/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', diff --git a/platform/viewer/src/components/ViewportGrid.tsx b/platform/viewer/src/components/ViewportGrid.tsx index cabbb06e3..66d7e8e18 100644 --- a/platform/viewer/src/components/ViewportGrid.tsx +++ b/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`, diff --git a/platform/viewer/src/routes/Mode/Mode.tsx b/platform/viewer/src/routes/Mode/Mode.tsx index f7159fc53..347114bd8 100644 --- a/platform/viewer/src/routes/Mode/Mode.tsx +++ b/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,