fix(segmentation overlay): update viewport ds list upon seg delete - OHIF-2425 (#5729)

- add segmentationExists check to getSopClassHandlerModule
- now firing SEGMENTATION_REMOVED and SEGMENTATION_REPRESENTATION_REMOVED events
- centralized segmentation removal in a listener
- when a segmentation is deleted (completely), remove it from all viewports it overlays
- when a segmentation is removed from a viewport, remove it as overlay from the viewport

---------

Co-authored-by: Joe Boccanfuso <joe.boccanfuso@radicalimaging.com>
Co-authored-by: Bill Wallace <wayfarer3130@gmail.com>

extensions/cornerstone-dicom-rt/src/getSopClassHandlerModule.ts

@@ -1,5 +1,6 @@
 import { utils, Types as OhifTypes } from '@ohif/core';
 import i18n from '@ohif/i18n';
+import { segmentation as cstSegmentation } from '@cornerstonejs/tools';
 
 import { SOPClassHandlerId } from './id';
 import loadRTStruct from './loadRTStruct';
@@ -8,8 +9,6 @@ const { sopClassDictionary } = utils;
 
 const sopClassUids = [sopClassDictionary.RTStructureSetStorage];
 
-const cachedRTStructsSEG = new Set<string>();
-
 const loadPromises = {};
 
 function _getDisplaySetsFromSeries(
@@ -143,23 +142,13 @@ function _load(
   if (
     (rtDisplaySet.loading || rtDisplaySet.isLoaded) &&
     loadPromises[SOPInstanceUID] &&
-    cachedRTStructsSEG.has(rtDisplaySet.displaySetInstanceUID)
+    _segmentationExists(rtDisplaySet)
   ) {
     return loadPromises[SOPInstanceUID];
   }
 
   rtDisplaySet.loading = true;
 
-  const { unsubscribe } = segmentationService.subscribe(
-    segmentationService.EVENTS.SEGMENTATION_LOADING_COMPLETE,
-    (evt: { rtDisplaySet: { displaySetInstanceUID: string } }) => {
-      if (evt.rtDisplaySet?.displaySetInstanceUID === rtDisplaySet.displaySetInstanceUID) {
-        cachedRTStructsSEG.add(rtDisplaySet.displaySetInstanceUID);
-        unsubscribe();
-      }
-    }
-  );
-
   // We don't want to fire multiple loads, so we'll wait for the first to finish
   // and also return the same promise to any other callers.
   loadPromises[SOPInstanceUID] = new Promise<void>(async (resolve, reject) => {
@@ -219,6 +208,10 @@ function _deriveReferencedSeriesSequenceFromFrameOfReferenceSequence(
   return ReferencedSeriesSequence;
 }
 
+function _segmentationExists(segDisplaySet) {
+  return !!cstSegmentation.state.getSegmentation(segDisplaySet.displaySetInstanceUID);
+}
+
 function getSopClassHandlerModule(params: OhifTypes.Extensions.ExtensionParams) {
   const { servicesManager, extensionManager } = params;
 

extensions/cornerstone-dicom-rt/src/viewports/OHIFCornerstoneRTViewport.tsx

@@ -168,7 +168,7 @@ function OHIFCornerstoneRTViewport(props: withAppTypes) {
 
     return () => {
       // remove the segmentation representations if seg displayset changed
-      segmentationService.removeSegmentationRepresentations(viewportId);
+      segmentationService.removeRepresentationsFromViewport(viewportId);
       referencedDisplaySetRef.current = null;
       toolGroupService.destroyToolGroup(toolGroupId);
     };

extensions/cornerstone/src/commandsModule.ts

@@ -1735,12 +1735,14 @@ function commandsModule({
      * Removes a segmentation from the viewport
      * @param props.segmentationId - The ID of the segmentation to remove
      */
-    removeSegmentationFromViewportCommand: ({ segmentationId }) => {
+    removeSegmentationFromViewportCommand: ({ segmentationId: displaySetInstanceUID }) => {
-      const { segmentationService, viewportGridService } = servicesManager.services;
+      const { viewportGridService } = servicesManager.services;
-      segmentationService.removeSegmentationRepresentations(
+      const viewportId = viewportGridService.getActiveViewportId();
-        viewportGridService.getActiveViewportId(),
+
-        { segmentationId }
+      commandsManager.runCommand('removeDisplaySetLayer', {
-      );
+        viewportId,
+        displaySetInstanceUID,
+      });
     },
 
     /**

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

@@ -2567,12 +2567,12 @@ describe('SegmentationService', () => {
   describe('clearSegmentationRepresentations', () => {
     it('should clear the segmentation representations', () => {
       const viewportId = 'viewportId';
-      jest.spyOn(service, 'removeSegmentationRepresentations').mockReturnValue(undefined);
+      jest.spyOn(service, 'removeRepresentationsFromViewport').mockReturnValue(undefined);
 
       service.clearSegmentationRepresentations(viewportId);
 
-      expect(service.removeSegmentationRepresentations).toHaveBeenCalledTimes(1);
+      expect(service.removeRepresentationsFromViewport).toHaveBeenCalledTimes(1);
-      expect(service.removeSegmentationRepresentations).toHaveBeenCalledWith(viewportId);
+      expect(service.removeRepresentationsFromViewport).toHaveBeenCalledWith(viewportId);
     });
   });
 
@@ -2600,7 +2600,7 @@ describe('SegmentationService', () => {
     });
   });
 
-  describe('removeSegmentationRepresentations', () => {
+  describe('removeRepresentationsFromViewport', () => {
     it('should remove the segmentation representations', () => {
       const viewportId = 'viewportId';
       const specifier = {
@@ -2609,7 +2609,7 @@ describe('SegmentationService', () => {
       };
       jest.spyOn(cstSegmentation, 'removeSegmentationRepresentations').mockReturnValue(undefined);
 
-      service.removeSegmentationRepresentations(viewportId, specifier);
+      service.removeRepresentationsFromViewport(viewportId, specifier);
 
       expect(cstSegmentation.removeSegmentationRepresentations).toHaveBeenCalledTimes(1);
       expect(cstSegmentation.removeSegmentationRepresentations).toHaveBeenCalledWith(

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

@@ -247,7 +247,7 @@ class SegmentationService extends PubSubService {
 
     eventTarget.removeEventListener(
       csToolsEnums.Events.SEGMENTATION_REMOVED,
-      this._onSegmentationModifiedFromSource
+      this._onSegmentationRemovedFromSource
     );
 
     eventTarget.removeEventListener(
@@ -267,7 +267,7 @@ class SegmentationService extends PubSubService {
 
     eventTarget.removeEventListener(
       csToolsEnums.Events.SEGMENTATION_REPRESENTATION_REMOVED,
-      this._onSegmentationRepresentationModifiedFromSource
+      this._onSegmentationRepresentationRemovedFromSource
     );
 
     eventTarget.removeEventListener(
@@ -1284,14 +1284,14 @@ class SegmentationService extends PubSubService {
 
   /**
    * Clears segmentation representations from the viewport.
-   * Unlike removeSegmentationRepresentations, this doesn't update
+   * Unlike removeRepresentationsFromViewport, this doesn't update
    * removed display set and representation maps.
    * We track removed segmentations manually to avoid re-adding them
    * when the display set is added again.
    * @param viewportId - The viewport ID to clear segmentation representations from.
    */
   public clearSegmentationRepresentations(viewportId: string): void {
-    this.removeSegmentationRepresentations(viewportId);
+    this.removeRepresentationsFromViewport(viewportId);
   }
 
   /**
@@ -1307,7 +1307,7 @@ class SegmentationService extends PubSubService {
   }
 
   /**
-   * It removes the segmentation representations from the viewport.
+   * Removes segmentation representations from the viewport.
    * @param viewportId - The viewport id to remove the segmentation representations from.
    * @param specifier - The specifier to remove the segmentation representations.
    *
@@ -1317,7 +1317,7 @@ class SegmentationService extends PubSubService {
    * If a type specifier is provided, only the segmentation representation with the specified type are removed.
    * If both a segmentationId and type specifier are provided, only the segmentation representation with the specified segmentationId and type are removed.
    */
-  public removeSegmentationRepresentations(
+  public removeRepresentationsFromViewport(
     viewportId: string,
     specifier: {
       segmentationId?: string;
@@ -1861,7 +1861,7 @@ class SegmentationService extends PubSubService {
 
     eventTarget.addEventListener(
       csToolsEnums.Events.SEGMENTATION_REMOVED,
-      this._onSegmentationModifiedFromSource
+      this._onSegmentationRemovedFromSource
     );
 
     eventTarget.addEventListener(
@@ -1881,7 +1881,7 @@ class SegmentationService extends PubSubService {
 
     eventTarget.addEventListener(
       csToolsEnums.Events.SEGMENTATION_REPRESENTATION_REMOVED,
-      this._onSegmentationRepresentationModifiedFromSource
+      this._onSegmentationRepresentationRemovedFromSource
     );
 
     eventTarget.addEventListener(
@@ -2120,6 +2120,14 @@ class SegmentationService extends PubSubService {
     });
   };
 
+  private _onSegmentationRepresentationRemovedFromSource = evt => {
+    const { segmentationId, viewportId } = evt.detail;
+    this._broadcastEvent(this.EVENTS.SEGMENTATION_REPRESENTATION_REMOVED, {
+      segmentationId,
+      viewportId,
+    });
+  };
+
   private _onSegmentationModifiedFromSource = (
     evt: cstTypes.EventTypes.SegmentationModifiedEventType
   ) => {
@@ -2140,6 +2148,16 @@ class SegmentationService extends PubSubService {
     });
   };
 
+  private _onSegmentationRemovedFromSource = (
+    evt: cstTypes.EventTypes.SegmentationRemovedEventType
+  ) => {
+    const { segmentationId } = evt.detail;
+
+    this._broadcastEvent(this.EVENTS.SEGMENTATION_REMOVED, {
+      segmentationId,
+    });
+  };
+
   private _onAnnotationCutMergeProcessCompletedFromSource = evt => {
     const { segmentationId } = evt.detail;
     this._broadcastEvent(this.EVENTS.SEGMENTATION_ANNOTATION_CUT_MERGE_PROCESS_COMPLETED, {

extensions/cornerstone/src/utils/setUpSegmentationEventHandlers.test.ts

@@ -15,6 +15,7 @@ describe('setUpSegmentationEventHandlers', () => {
   const mockSegmentationService = {
     EVENTS: {
       SEGMENTATION_ADDED: 'SEGMENTATION_ADDED',
+      SEGMENTATION_REMOVED: 'SEGMENTATION_REMOVED',
     },
     subscribe: jest.fn(),
     getSegmentation: jest.fn(),
@@ -40,6 +41,7 @@ describe('setUpSegmentationEventHandlers', () => {
   const mockUnsubscribeDataModified = jest.fn();
   const mockUnsubscribeModified = jest.fn();
   const mockUnsubscribeCreated = jest.fn();
+  const mockUnsubscribeRemoved = jest.fn();
   const mockUnsubscribeSelectedSegmentationsForViewportEvents = [jest.fn(), jest.fn()];
 
   const defaultParameters = {
@@ -59,8 +61,14 @@ describe('setUpSegmentationEventHandlers', () => {
       unsubscribeSelectedSegmentationsForViewportEvents:
         mockUnsubscribeSelectedSegmentationsForViewportEvents,
     });
-    mockSegmentationService.subscribe.mockReturnValue({
+    mockSegmentationService.subscribe.mockImplementation((eventName: string) => {
-      unsubscribe: mockUnsubscribeCreated,
+      if (eventName === mockSegmentationService.EVENTS.SEGMENTATION_ADDED) {
+        return { unsubscribe: mockUnsubscribeCreated };
+      }
+      if (eventName === mockSegmentationService.EVENTS.SEGMENTATION_REMOVED) {
+        return { unsubscribe: mockUnsubscribeRemoved };
+      }
+      return { unsubscribe: jest.fn() };
     });
   });
 
@@ -99,6 +107,7 @@ describe('setUpSegmentationEventHandlers', () => {
         mockUnsubscribeDataModified,
         mockUnsubscribeModified,
         mockUnsubscribeCreated,
+        mockUnsubscribeRemoved,
         ...mockUnsubscribeSelectedSegmentationsForViewportEvents,
       ],
     });
@@ -358,5 +367,6 @@ describe('setUpSegmentationEventHandlers', () => {
     expect(mockUnsubscribeDataModified).toHaveBeenCalled();
     expect(mockUnsubscribeModified).toHaveBeenCalled();
     expect(mockUnsubscribeCreated).toHaveBeenCalled();
+    expect(mockUnsubscribeRemoved).toHaveBeenCalled();
   });
 });

extensions/cornerstone/src/utils/setUpSegmentationEventHandlers.ts

@@ -7,7 +7,8 @@ import {
 } from './segmentationHandlers';
 
 export const setUpSegmentationEventHandlers = ({ servicesManager, commandsManager }) => {
-  const { segmentationService, customizationService, displaySetService } = servicesManager.services;
+  const { segmentationService, customizationService, displaySetService, viewportGridService } =
+    servicesManager.services;
 
   const { unsubscribe: unsubscribeSegmentationDataModifiedHandler } =
     setupSegmentationDataModifiedHandler({
@@ -57,6 +58,36 @@ export const setUpSegmentationEventHandlers = ({ servicesManager, commandsManage
     }
   );
 
+  const { unsubscribe: unsubscribeSegmentationRemoved } = segmentationService.subscribe(
+    segmentationService.EVENTS.SEGMENTATION_REMOVED,
+    ({ segmentationId }) => {
+      const displaySet = displaySetService.getDisplaySetByUID(segmentationId);
+
+      // Remove the display set layer from all viewports that have it
+      if (displaySet) {
+        const state = viewportGridService.getState();
+        const viewports = state.viewports;
+
+        // Find all viewports that contain this segmentation's display set as a layer
+        for (const [viewportId, viewport] of viewports.entries()) {
+          const displaySetInstanceUIDs = viewport.displaySetInstanceUIDs || [];
+          if (displaySetInstanceUIDs.includes(segmentationId)) {
+            // Remove the display set layer from this viewport
+            commandsManager.runCommand('removeDisplaySetLayer', {
+              viewportId,
+              displaySetInstanceUID: segmentationId,
+            });
+          }
+        }
+
+        // Delete the display set from the service if it was made in client
+        if (displaySet.madeInClient) {
+          displaySetService.deleteDisplaySet(segmentationId);
+        }
+      }
+    }
+  );
+
   const { unsubscribeSelectedSegmentationsForViewportEvents } =
     setUpSelectedSegmentationsForViewportHandler({
       segmentationService,
@@ -66,6 +97,7 @@ export const setUpSegmentationEventHandlers = ({ servicesManager, commandsManage
     unsubscribeSegmentationDataModifiedHandler,
     unsubscribeSegmentationModifiedHandler,
     unsubscribeSegmentationCreated,
+    unsubscribeSegmentationRemoved,
     ...unsubscribeSelectedSegmentationsForViewportEvents,
   ];
 

extensions/default/src/commandsModule.ts

@@ -66,8 +66,8 @@ const commandsModule = ({
      */
     addDisplaySetAsLayer: ({ viewportId, displaySetInstanceUID, removeFirst = false }) => {
       if (!viewportId) {
-          const { activeViewportId } = servicesManager.services.viewportGridService.getState();
+        const { activeViewportId } = servicesManager.services.viewportGridService.getState();
-          viewportId = activeViewportId;
+        viewportId = activeViewportId;
       }
 
       if (!viewportId || !displaySetInstanceUID) {
@@ -151,6 +151,19 @@ const commandsModule = ({
         return;
       }
 
+      // Check if it's a segmentation and handle accordingly.
+      // Note that for the sake of hydrated segmentations, we remove the
+      // segmentation before checking if the display set is indeed in the viewport.
+      // This is because hydrated segmentations are not in the viewport per se
+      // {i.e. they are not layered) but are simply referenced by the display
+      // set in the viewport.
+      const isSegmentation = DERIVED_OVERLAY_MODALITIES.includes(displaySet.Modality);
+      if (isSegmentation) {
+        segmentationService.removeRepresentationsFromViewport(viewportId, {
+          segmentationId: displaySetInstanceUID,
+        });
+      }
+
       // Get current display sets for the viewport
       const currentDisplaySetUIDs = viewportGridService.getDisplaySetsUIDsForViewport(viewportId);
 
@@ -159,14 +172,6 @@ const commandsModule = ({
         return;
       }
 
-      // Check if it's a segmentation and handle accordingly
-      const isSegmentation = DERIVED_OVERLAY_MODALITIES.includes(displaySet.Modality);
-      if (isSegmentation) {
-        segmentationService.removeSegmentationRepresentations(viewportId, {
-          segmentationId: displaySetInstanceUID,
-        });
-      }
-
       const updatedViewports = hangingProtocolService.getViewportsRequireUpdate(
         viewportId,
         displaySetInstanceUID

platform/docs/docs/migration-guide/3p11-to-3p12/index.md

@@ -1,11 +1,11 @@
 ---
 sidebar_position: 1
-sidebar_label: 3.11 -> 3.12 beta
+sidebar_label: 3.11 -> 3.12
 ---
 
 # Migration Guide
 
-This guide provides information about migrating from OHIF version 3.11 to version 3.12 beta
+This guide provides information about migrating from OHIF version 3.11 to version 3.12
 
 ## Optional: Migrate modes to extend `modes/basic`
 

platform/docs/docs/migration-guide/3p12-to-3p13/index.md

@@ -0,0 +1,15 @@
+---
+id: index
+sidebar_position: 1
+sidebar_label: 3.12 -> 3.13
+title: 3.12 to 3.13 Migration Guide
+---
+
+import DocCardList from '@theme/DocCardList';
+import { useCurrentSidebarCategory } from '@docusaurus/theme-common';
+
+# 3.12 to 3.13 Migration Guide
+
+This guide covers changes when upgrading from OHIF version 3.12 to version 3.13.
+
+<DocCardList items={useCurrentSidebarCategory().items.filter(item => item.docId !== 'migration-guide/3p12-to-3p13/index')} />

platform/docs/docs/migration-guide/3p12-to-3p13/segmentation-service.md

@@ -0,0 +1,51 @@
+---
+sidebar_position: 2
+sidebar_label: SegmentationService
+title: SegmentationService API change
+---
+
+# SegmentationService – method rename
+
+The `SegmentationService` method for removing segmentation representations from a viewport has been renamed to better reflect its behavior.
+
+## Change
+
+| 3.12 (old)                         | 3.13 (new)                        |
+|------------------------------------|-----------------------------------|
+| `removeSegmentationRepresentations` | `removeRepresentationsFromViewport` |
+
+The signature is unchanged:
+
+```ts
+removeRepresentationsFromViewport(
+  viewportId: string,
+  specifier?: {
+    segmentationId?: string;
+    type?: SegmentationRepresentations;
+  }
+): void
+```
+
+## Migration
+
+Replace calls to `removeSegmentationRepresentations` with `removeRepresentationsFromViewport`.
+
+**Before (3.12):**
+
+```ts
+segmentationService.removeSegmentationRepresentations(viewportId);
+// or with a specifier:
+segmentationService.removeSegmentationRepresentations(viewportId, { segmentationId });
+```
+
+**After (3.13):**
+
+```ts
+segmentationService.removeRepresentationsFromViewport(viewportId);
+// or with a specifier:
+segmentationService.removeRepresentationsFromViewport(viewportId, { segmentationId });
+```
+
+## Reason
+
+The new name makes it clear that the method removes representations from a specific viewport, rather than removing segmentations globally.