diff --git a/packages/core/src/RenderingEngine/StackViewport.ts b/packages/core/src/RenderingEngine/StackViewport.ts index f78c129f27..584a063622 100644 --- a/packages/core/src/RenderingEngine/StackViewport.ts +++ b/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); diff --git a/packages/tools/src/tools/displayTools/Labelmap/syncStackLabelmapActors.spec.ts b/packages/tools/src/tools/displayTools/Labelmap/syncStackLabelmapActors.spec.ts new file mode 100644 index 0000000000..2774ff75ea --- /dev/null +++ b/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']); + }); +}); diff --git a/packages/tools/src/tools/displayTools/Labelmap/syncStackLabelmapActors.ts b/packages/tools/src/tools/displayTools/Labelmap/syncStackLabelmapActors.ts index fb4abea319..185b010afa 100644 --- a/packages/tools/src/tools/displayTools/Labelmap/syncStackLabelmapActors.ts +++ b/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); }