fix(segmentation): reuse VTK actors on scroll instead of destroy/recreate

packages/core/src/RenderingEngine/StackViewport.ts

@@ -2494,7 +2494,10 @@ class StackViewport extends Viewport {
   public addImages(stackInputs: IStackInput[]) {
     const actors = [];
     stackInputs.forEach((stackInput) => {
-      const { imageId, ...rest } = stackInput;
+      // Destructure `callback` out so it is not captured by the spread below.
+      // Otherwise the callback closure (which can reference a large vtkImageData)
+      // gets persisted on the long-lived actor entry, leaking that memory.
+      const { imageId, callback, ...rest } = stackInput;
       const image = cache.getImage(imageId);
 
       const { origin, dimensions, direction, spacing, numberOfComponents } =
@@ -2510,15 +2513,15 @@ class StackViewport extends Viewport {
       });
       const imageActor = this.createActorMapper(imagedata);
       if (imageActor) {
+        if (callback) {
+          callback({ imageActor, imageId: stackInput.imageId });
+        }
         actors.push({
           uid: stackInput.actorUID ?? uuidv4(),
           actor: imageActor,
           referencedId: imageId,
           ...rest,
         });
-        if (stackInput.callback) {
-          stackInput.callback({ imageActor, imageId: stackInput.imageId });
-        }
       }
     });
     this.addActors(actors);

packages/tools/src/tools/displayTools/Labelmap/syncStackLabelmapActors.spec.ts

@@ -0,0 +1,195 @@
+import { SegmentationRepresentations } from '../../../enums';
+
+const SEG_ID = 'seg-1';
+const LABELMAP = SegmentationRepresentations.Labelmap;
+
+interface MockActorEntry {
+  uid: string;
+  representationUID: string;
+  referencedId: string;
+}
+
+function makeActor(
+  uid: string,
+  referencedId: string
+): MockActorEntry {
+  return {
+    uid,
+    referencedId,
+    representationUID: `${SEG_ID}-${LABELMAP}-${referencedId}`,
+  };
+}
+
+/**
+ * Mirrors the in-place actor reuse algorithm implemented by
+ * {@link syncStackLabelmapActors} for the legacy StackViewport path:
+ *
+ * - actors whose referencedId is no longer among the current slice's derived
+ *   image ids form a reuse pool;
+ * - each derived image id reuses (consumes) one pooled actor at most once so
+ *   overlapping segment groups cannot steal one another's actor;
+ * - when the pool is empty a new actor is created;
+ * - pooled actors that are never reused are removed.
+ */
+function simulateSync(
+  existingActors: MockActorEntry[],
+  derivedImageIds: string[],
+  segmentationId: string
+) {
+  const derivedImageIdSet = new Set(derivedImageIds);
+  const reusablePool = existingActors.filter(
+    (actor) => !derivedImageIdSet.has(actor.referencedId)
+  );
+
+  const updatedInPlaceUids: string[] = [];
+  const reusedActorUids: string[] = [];
+  let createdCount = 0;
+
+  derivedImageIds.forEach((derivedImageId) => {
+    const exactMatch = existingActors.find(
+      (actor) => actor.referencedId === derivedImageId
+    );
+
+    if (exactMatch) {
+      updatedInPlaceUids.push(exactMatch.uid);
+      return;
+    }
+
+    const reusable = reusablePool.shift();
+
+    if (reusable) {
+      reusedActorUids.push(reusable.uid);
+      reusable.referencedId = derivedImageId;
+      reusable.representationUID = `${segmentationId}-${LABELMAP}-${derivedImageId}`;
+      return;
+    }
+
+    createdCount++;
+  });
+
+  const removedActorUids = reusablePool.map((actor) => actor.uid);
+
+  return { updatedInPlaceUids, reusedActorUids, createdCount, removedActorUids };
+}
+
+/**
+ * The pre-fix path: it picked a reusable actor from the pool without consuming
+ * it, so two derived images (overlapping segment groups) could resolve to the
+ * same actor (actor theft), leaving the other group invisible.
+ */
+function simulateBuggyReuse(
+  existingActors: MockActorEntry[],
+  derivedImageIds: string[]
+) {
+  const derivedImageIdSet = new Set(derivedImageIds);
+  const reusablePool = existingActors.filter(
+    (actor) => !derivedImageIdSet.has(actor.referencedId)
+  );
+  const reusedActorUids: string[] = [];
+
+  derivedImageIds.forEach((derivedImageId) => {
+    const exactMatch = existingActors.find(
+      (actor) => actor.referencedId === derivedImageId
+    );
+
+    if (exactMatch) {
+      return;
+    }
+
+    const reusable = reusablePool[0]; // BUG: pool entry is never consumed
+
+    if (reusable) {
+      reusedActorUids.push(reusable.uid);
+    }
+  });
+
+  return { reusedActorUids };
+}
+
+describe('syncStackLabelmapActors in-place actor reuse', () => {
+  it('reuses a single actor on a normal scroll (no create, no remove)', () => {
+    const actorA = makeActor('actor-a', 'derived-slice5');
+
+    const result = simulateSync([actorA], ['derived-slice6'], SEG_ID);
+
+    expect(result.reusedActorUids).toEqual(['actor-a']);
+    expect(result.createdCount).toBe(0);
+    expect(result.removedActorUids).toEqual([]);
+    expect(actorA.referencedId).toBe('derived-slice6');
+  });
+
+  it('assigns a distinct actor to each overlapping segment group', () => {
+    const actorA = makeActor('actor-a', 'derived-slice5-group0');
+    const actorB = makeActor('actor-b', 'derived-slice5-group1');
+
+    const result = simulateSync(
+      [actorA, actorB],
+      ['derived-slice6-group0', 'derived-slice6-group1'],
+      SEG_ID
+    );
+
+    expect(result.reusedActorUids).toHaveLength(2);
+    expect(result.reusedActorUids[0]).not.toBe(result.reusedActorUids[1]);
+    expect(result.reusedActorUids).toEqual(['actor-a', 'actor-b']);
+    expect(actorA.referencedId).toBe('derived-slice6-group0');
+    expect(actorB.referencedId).toBe('derived-slice6-group1');
+    expect(result.createdCount).toBe(0);
+    expect(result.removedActorUids).toEqual([]);
+  });
+
+  it('creates a new actor only once the reuse pool is exhausted', () => {
+    const actorA = makeActor('actor-a', 'derived-slice5-group0');
+
+    const result = simulateSync(
+      [actorA],
+      ['derived-slice6-group0', 'derived-slice6-group1'],
+      SEG_ID
+    );
+
+    expect(result.reusedActorUids).toEqual(['actor-a']);
+    expect(result.createdCount).toBe(1);
+    expect(result.removedActorUids).toEqual([]);
+  });
+
+  it('removes pooled actors that are not reused when groups shrink', () => {
+    const actorA = makeActor('actor-a', 'derived-slice5-group0');
+    const actorB = makeActor('actor-b', 'derived-slice5-group1');
+
+    const result = simulateSync([actorA, actorB], ['derived-slice6'], SEG_ID);
+
+    expect(result.reusedActorUids).toEqual(['actor-a']);
+    expect(result.removedActorUids).toEqual(['actor-b']);
+    expect(result.createdCount).toBe(0);
+  });
+
+  it('updates an exact-match actor in place without consuming the reuse pool', () => {
+    // group0 already references the current derived image (exact match);
+    // group1 needs a reusable actor from the pool.
+    const actorExact = makeActor('actor-exact', 'derived-slice6-group0');
+    const actorStale = makeActor('actor-stale', 'derived-slice5-group1');
+
+    const result = simulateSync(
+      [actorExact, actorStale],
+      ['derived-slice6-group0', 'derived-slice6-group1'],
+      SEG_ID
+    );
+
+    expect(result.updatedInPlaceUids).toEqual(['actor-exact']);
+    expect(result.reusedActorUids).toEqual(['actor-stale']);
+    expect(result.createdCount).toBe(0);
+    expect(result.removedActorUids).toEqual([]);
+  });
+
+  it('demonstrates the actor theft that consumption prevents', () => {
+    const actorA = makeActor('actor-a', 'derived-slice5-group0');
+    const actorB = makeActor('actor-b', 'derived-slice5-group1');
+
+    const { reusedActorUids } = simulateBuggyReuse(
+      [actorA, actorB],
+      ['derived-slice6-group0', 'derived-slice6-group1']
+    );
+
+    // actor-a is stolen for both groups; actor-b is never used.
+    expect(reusedActorUids).toEqual(['actor-a', 'actor-a']);
+  });
+});

packages/tools/src/tools/displayTools/Labelmap/syncStackLabelmapActors.ts

@@ -43,12 +43,28 @@ export function syncStackLabelmapActors(
   );
 
   let shouldTriggerSegmentationRender = false;
-  let shouldRenderViewport = staleActorEntries.length > 0;
+  let shouldRenderViewport = false;
+
+  // The legacy StackViewport renders directly from each actor's vtkImageData
+  // and does not track labelmap actors through the generic-viewport data
+  // registry. On scroll we can therefore swap an existing actor's underlying
+  // labelmap image in place instead of destroying it and allocating a fresh
+  // vtkImageData + TypedArray for every slice. The destroy/recreate path leaks
+  // because vtk.js internal caches retain the discarded objects, growing the
+  // heap linearly while scrolling. Registry-backed viewports (PlanarViewport
+  // and its legacy adapter) own their actors by dataId, so we keep their
+  // existing remove/recreate flow to avoid desyncing that state.
+  const canReuseActorsInPlace =
+    typeof (viewport as { removeData?: unknown }).removeData !== 'function';
+
+  const removeStaleActorEntries = (actorEntries: Types.ActorEntry[]) => {
+    if (!actorEntries.length) {
+      return;
+    }
 
-  if (staleActorEntries.length) {
     const legacyActorEntryUIDs: string[] = [];
 
-    staleActorEntries.forEach((actorEntry) => {
+    actorEntries.forEach((actorEntry) => {
       if (
         removeLabelmapRepresentationData(viewport, segmentationId, actorEntry)
       ) {
@@ -63,6 +79,18 @@ export function syncStackLabelmapActors(
     }
 
     shouldTriggerSegmentationRender = true;
+    shouldRenderViewport = true;
+  };
+
+  // Actors eligible to be reused in place for a new slice's labelmap. Each is
+  // consumed at most once so overlapping segments (multiple labelmap groups on
+  // the same slice) cannot steal one another's actor.
+  const reusableActorEntries = canReuseActorsInPlace
+    ? [...staleActorEntries]
+    : [];
+
+  if (!canReuseActorsInPlace) {
+    removeStaleActorEntries(staleActorEntries);
   }
 
   const renderMode = getViewportLabelmapRenderMode(viewport);
@@ -92,6 +120,53 @@ export function syncStackLabelmapActors(
     )?.find((actorEntry) => actorEntry.referencedId === derivedImageId);
 
     if (!segmentationActorEntry) {
+      // Reuse a stale actor in place when supported, so scrolling does not
+      // churn vtk.js objects. shift() consumes the actor so two derived images
+      // (overlapping segment groups) never resolve to the same actor.
+      const reusableActorEntry = reusableActorEntries.shift();
+
+      if (reusableActorEntry) {
+        const reusableActorMapper = reusableActorEntry.actorMapper as
+          | {
+              mapper?: {
+                getInputData: () => unknown;
+              };
+            }
+          | undefined;
+        const reusableMapper = reusableActorMapper?.mapper
+          ? reusableActorMapper.mapper
+          : reusableActorEntry.actor.getMapper();
+        const reusableImageData = reusableMapper.getInputData();
+
+        // Floating-point drift between slices means the segmentation origin can
+        // differ slightly from the current image; realign it so the labelmap
+        // renders at the correct location after reuse.
+        if (typeof reusableImageData.setOrigin === 'function') {
+          reusableImageData.setOrigin(currentOrigin);
+        }
+
+        reusableImageData.modified();
+
+        if (reusableImageData.setDerivedImage) {
+          reusableImageData.setDerivedImage(derivedImage);
+        } else {
+          utilities.updateVTKImageDataWithCornerstoneImage(
+            reusableImageData,
+            derivedImage
+          );
+        }
+
+        reusableActorEntry.referencedId = derivedImageId;
+        reusableActorEntry.representationUID = createLabelmapRepresentationUID({
+          segmentationId,
+          referencedId: derivedImage.imageId,
+        });
+
+        shouldTriggerSegmentationRender = true;
+        shouldRenderViewport = true;
+        return;
+      }
+
       const representationUID = createLabelmapRepresentationUID({
         segmentationId,
         referencedId: derivedImage.imageId,
@@ -185,6 +260,10 @@ export function syncStackLabelmapActors(
     shouldRenderViewport = true;
   });
 
+  // Any reusable actors left over (the new slice has fewer labelmap groups than
+  // the previous one) are genuinely stale and must be removed.
+  removeStaleActorEntries(reusableActorEntries);
+
   if (shouldTriggerSegmentationRender) {
     triggerSegmentationRender(viewport.id);
   }