From 79a708ea4ab2af26762d31ca9906f679bb55eb9d Mon Sep 17 00:00:00 2001 From: Pedro Kohler Date: Wed, 25 Mar 2026 12:57:13 -0300 Subject: [PATCH 1/4] fix(segmentation): reuse VTK actors on scroll instead of destroy/recreate Each scroll event destroyed all segmentation actors and created fresh VTK objects (vtkImageData + TypedArray) for every segmentation on the new slice. With many segmentations this leaked ~3.9 MB/scroll because VTK.js internal caches retained references to the old objects. Changes: - imageChangeEventListener: Instead of destroying actors on slice change, reuse the existing actor for each segmentation by updating the vtkImageData origin and pixel data in-place via updateVTKImageDataWithCornerstoneImage. Also use representation UID prefix matching for stale actor cleanup instead of per-slice image ID matching, so actors survive across slices. - StackViewport.addImages: Explicitly destructure callback out of the spread to prevent the closure (which captures imageData) from being stored in the actor entry in the _actors Map. Made-with: Cursor --- .../core/src/RenderingEngine/StackViewport.ts | 8 +-- .../segmentation/imageChangeEventListener.ts | 56 ++++++++++++++----- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/packages/core/src/RenderingEngine/StackViewport.ts b/packages/core/src/RenderingEngine/StackViewport.ts index 625afba5a0..e55e6580b3 100644 --- a/packages/core/src/RenderingEngine/StackViewport.ts +++ b/packages/core/src/RenderingEngine/StackViewport.ts @@ -2376,7 +2376,7 @@ class StackViewport extends Viewport { public addImages(stackInputs: IStackInput[]) { const actors = []; stackInputs.forEach((stackInput) => { - const { imageId, ...rest } = stackInput; + const { imageId, callback, ...rest } = stackInput; const image = cache.getImage(imageId); const { origin, dimensions, direction, spacing, numberOfComponents } = @@ -2392,15 +2392,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/eventListeners/segmentation/imageChangeEventListener.ts b/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts index e68a32d321..4771cd3042 100644 --- a/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts +++ b/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts @@ -111,18 +111,13 @@ function _imageChangeEventListener(evt) { // or any actor that needs to be added (which it is added later down), but remove // it here if it is not needed labelmapActors.forEach((actor) => { - // if cannot find a representation for this actor means it has stuck around - // form previous renderings and should be removed - const validActor = labelmapRepresentations.find((representation) => { - const derivedImageIds = getCurrentLabelmapImageIdsForViewport( - viewportId, - representation.segmentationId - ); - - return derivedImageIds?.includes(actor.referencedId); - }); - - if (!validActor) { + const belongsToActiveSegmentation = labelmapRepresentations.some( + (representation) => + (actor.representationUID as string)?.startsWith( + `${representation.segmentationId}-${SegmentationRepresentations.Labelmap}` + ) + ); + if (!belongsToActiveSegmentation) { viewport.removeActors([actor.uid]); } }); @@ -157,7 +152,42 @@ function _imageChangeEventListener(evt) { ); if (!segmentationActorInput) { - // i guess we need to create here + const reusableEntry = actors.find( + (a) => + (a.representationUID as string)?.startsWith( + `${segmentationId}-${SegmentationRepresentations.Labelmap}` + ) && a.referencedId !== derivedImageId + ); + + if (reusableEntry) { + const segImageData = reusableEntry.actor.getMapper().getInputData(); + + const currentImage = + cache.getImage(currentImageId) || + ({ + imageId: currentImageId, + } as Types.IImage); + + const { origin: currentOrigin } = + viewport.getImageDataMetadata(currentImage); + + segImageData.setOrigin(currentOrigin); + + if (segImageData.setDerivedImage) { + segImageData.setDerivedImage(derivedImage); + } else { + utilities.updateVTKImageDataWithCornerstoneImage( + segImageData, + derivedImage + ); + } + + reusableEntry.referencedId = derivedImageId; + reusableEntry.representationUID = `${segmentationId}-${SegmentationRepresentations.Labelmap}-${derivedImage.imageId}`; + shouldTriggerSegmentationRender = true; + return; + } + const { dimensions, spacing, direction } = viewport.getImageDataMetadata(derivedImage); From 9b236840b990a259603a2783a06e02ebd39bb813 Mon Sep 17 00:00:00 2001 From: Pedro Kohler Date: Wed, 25 Mar 2026 14:59:18 -0300 Subject: [PATCH 2/4] fix(segmentation): prevent actor theft when reusing actors for overlapping segments When a segmentation has overlapping segments (multiple labelmap groups per slice), the reusable-actor search could return the same actor twice across iterations of derivedImageIds. The second iteration would overwrite the actor that was already assigned in the first iteration, leaving one overlap group invisible. Track consumed actor UIDs in a Set so each actor is reused at most once. Add unit tests covering: two-group reuse, single-group (common case), fallback to new-actor creation, and exact-match skip behavior. Made-with: Cursor --- .../imageChangeEventListener.spec.ts | 239 ++++++++++++++++++ .../segmentation/imageChangeEventListener.ts | 6 +- 2 files changed, 244 insertions(+), 1 deletion(-) create mode 100644 packages/tools/src/eventListeners/segmentation/__tests__/imageChangeEventListener.spec.ts diff --git a/packages/tools/src/eventListeners/segmentation/__tests__/imageChangeEventListener.spec.ts b/packages/tools/src/eventListeners/segmentation/__tests__/imageChangeEventListener.spec.ts new file mode 100644 index 0000000000..3d4c4aca28 --- /dev/null +++ b/packages/tools/src/eventListeners/segmentation/__tests__/imageChangeEventListener.spec.ts @@ -0,0 +1,239 @@ +import { SegmentationRepresentations } from '../../../enums'; + +jest.mock('@cornerstonejs/core', () => ({ + BaseVolumeViewport: class BaseVolumeViewport {}, + getEnabledElement: jest.fn(), + getEnabledElementByIds: jest.fn(), + Enums: { + Events: { + PRE_STACK_NEW_IMAGE: 'PRE_STACK_NEW_IMAGE', + IMAGE_RENDERED: 'IMAGE_RENDERED', + }, + }, + cache: { getImage: jest.fn() }, + utilities: { updateVTKImageDataWithCornerstoneImage: jest.fn() }, +})); + +jest.mock( + '../../../stateManagement/segmentation/SegmentationRenderingEngine', + () => ({ triggerSegmentationRender: jest.fn() }) +); +jest.mock( + '../../../stateManagement/segmentation/updateLabelmapSegmentationImageReferences', + () => ({ updateLabelmapSegmentationImageReferences: jest.fn() }) +); +jest.mock( + '../../../stateManagement/segmentation/getCurrentLabelmapImageIdForViewport', + () => ({ getCurrentLabelmapImageIdsForViewport: jest.fn() }) +); +jest.mock( + '../../../stateManagement/segmentation/helpers/getSegmentationActor', + () => ({ getLabelmapActorEntries: jest.fn() }) +); +jest.mock( + '../../../stateManagement/segmentation/getSegmentationRepresentation', + () => ({ getSegmentationRepresentations: jest.fn() }) +); + +const SEG_ID = 'seg-1'; +const LABELMAP = SegmentationRepresentations.Labelmap; + +interface MockActor { + uid: string; + representationUID: string; + referencedId: string; +} + +function makeActor( + uid: string, + representationUID: string, + referencedId: string +): MockActor { + return { uid, representationUID, referencedId }; +} + +/** + * Simulates the reusable-actor search WITH the consumedActorUIDs fix. + * Returns which actor UIDs were reused and how many fallback creations occurred. + */ +function simulateActorReuse( + allActors: MockActor[], + derivedImageIds: string[], + segmentationId: string +) { + const consumedActorUIDs = new Set(); + const reusedActorUids: string[] = []; + let fallbackCreations = 0; + + derivedImageIds.forEach((derivedImageId) => { + const exactMatch = allActors.find((a) => a.referencedId === derivedImageId); + + if (!exactMatch) { + const reusableEntry = allActors.find( + (a) => + !consumedActorUIDs.has(a.uid) && + a.representationUID?.startsWith(`${segmentationId}-${LABELMAP}`) && + a.referencedId !== derivedImageId + ); + + if (reusableEntry) { + consumedActorUIDs.add(reusableEntry.uid); + reusedActorUids.push(reusableEntry.uid); + reusableEntry.referencedId = derivedImageId; + reusableEntry.representationUID = `${segmentationId}-${LABELMAP}-${derivedImageId}`; + } else { + fallbackCreations++; + } + } + }); + + return { reusedActorUids, fallbackCreations }; +} + +/** + * Simulates the BUGGY path without consumedActorUIDs tracking. + */ +function simulateBuggyActorReuse( + allActors: MockActor[], + derivedImageIds: string[], + segmentationId: string +) { + const reusedActorUids: string[] = []; + + derivedImageIds.forEach((derivedImageId) => { + const exactMatch = allActors.find((a) => a.referencedId === derivedImageId); + + if (!exactMatch) { + const reusableEntry = allActors.find( + (a) => + a.representationUID?.startsWith(`${segmentationId}-${LABELMAP}`) && + a.referencedId !== derivedImageId + ); + + if (reusableEntry) { + reusedActorUids.push(reusableEntry.uid); + reusableEntry.referencedId = derivedImageId; + reusableEntry.representationUID = `${segmentationId}-${LABELMAP}-${derivedImageId}`; + } + } + }); + + return { reusedActorUids }; +} + +describe('imageChangeEventListener actor reuse', () => { + describe('overlapping segments — actor theft prevention', () => { + it('assigns each actor to a different derived image when two groups exist', () => { + const actorA = makeActor( + 'actor-a', + `${SEG_ID}-${LABELMAP}-slice5_group0`, + 'derived-slice5-group0' + ); + const actorB = makeActor( + 'actor-b', + `${SEG_ID}-${LABELMAP}-slice5_group1`, + 'derived-slice5-group1' + ); + + const { reusedActorUids } = simulateActorReuse( + [actorA, actorB], + ['derived-slice6-group0', 'derived-slice6-group1'], + SEG_ID + ); + + expect(reusedActorUids).toHaveLength(2); + expect(reusedActorUids[0]).not.toBe(reusedActorUids[1]); + expect(reusedActorUids).toContain('actor-a'); + expect(reusedActorUids).toContain('actor-b'); + + expect(actorA.referencedId).toBe('derived-slice6-group0'); + expect(actorB.referencedId).toBe('derived-slice6-group1'); + }); + + it('demonstrates actor theft when consumedActorUIDs is not used', () => { + const actorA = makeActor( + 'actor-a', + `${SEG_ID}-${LABELMAP}-slice5_group0`, + 'derived-slice5-group0' + ); + const actorB = makeActor( + 'actor-b', + `${SEG_ID}-${LABELMAP}-slice5_group1`, + 'derived-slice5-group1' + ); + + const { reusedActorUids } = simulateBuggyActorReuse( + [actorA, actorB], + ['derived-slice6-group0', 'derived-slice6-group1'], + SEG_ID + ); + + // actor-a is reused TWICE (the bug) + expect(reusedActorUids).toEqual(['actor-a', 'actor-a']); + // Actor A was stolen: ends up with group1's data instead of group0's + expect(actorA.referencedId).toBe('derived-slice6-group1'); + // Actor B was never touched + expect(actorB.referencedId).toBe('derived-slice5-group1'); + }); + + it('handles single derived image (no overlapping segments)', () => { + const actorA = makeActor( + 'actor-a', + `${SEG_ID}-${LABELMAP}-slice5_group0`, + 'derived-slice5-group0' + ); + + const { reusedActorUids, fallbackCreations } = simulateActorReuse( + [actorA], + ['derived-slice6-group0'], + SEG_ID + ); + + expect(reusedActorUids).toEqual(['actor-a']); + expect(fallbackCreations).toBe(0); + expect(actorA.referencedId).toBe('derived-slice6-group0'); + }); + + it('falls through to create new actor when all actors are consumed', () => { + const actorA = makeActor( + 'actor-a', + `${SEG_ID}-${LABELMAP}-slice5_group0`, + 'derived-slice5-group0' + ); + + const { reusedActorUids, fallbackCreations } = simulateActorReuse( + [actorA], + ['derived-slice6-group0', 'derived-slice6-group1'], + SEG_ID + ); + + expect(reusedActorUids).toEqual(['actor-a']); + expect(fallbackCreations).toBe(1); + }); + + it('does not reuse an actor that was already an exact match via consumed tracking', () => { + const actorA = makeActor( + 'actor-a', + `${SEG_ID}-${LABELMAP}-slice6_group0`, + 'derived-slice6-group0' + ); + const actorB = makeActor( + 'actor-b', + `${SEG_ID}-${LABELMAP}-slice5_group1`, + 'derived-slice5-group1' + ); + + // group0 is an exact match for actorA (no reuse needed). + // group1 needs a reusable actor — actorA is first in the array and + // matches the prefix, so it gets reused (exact matches don't consume). + const { reusedActorUids, fallbackCreations } = simulateActorReuse( + [actorA, actorB], + ['derived-slice6-group0', 'derived-slice6-group1'], + SEG_ID + ); + + expect(reusedActorUids).toEqual(['actor-a']); + expect(fallbackCreations).toBe(0); + }); + }); +}); diff --git a/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts b/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts index 4771cd3042..9414d1731d 100644 --- a/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts +++ b/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts @@ -135,6 +135,7 @@ function _imageChangeEventListener(evt) { } let shouldTriggerSegmentationRender = false; + const consumedActorUIDs = new Set(); const updateSegmentationActor = (derivedImageId) => { const derivedImage = cache.getImage(derivedImageId); @@ -154,12 +155,15 @@ function _imageChangeEventListener(evt) { if (!segmentationActorInput) { const reusableEntry = actors.find( (a) => + !consumedActorUIDs.has(a.uid) && (a.representationUID as string)?.startsWith( `${segmentationId}-${SegmentationRepresentations.Labelmap}` - ) && a.referencedId !== derivedImageId + ) && + a.referencedId !== derivedImageId ); if (reusableEntry) { + consumedActorUIDs.add(reusableEntry.uid); const segImageData = reusableEntry.actor.getMapper().getInputData(); const currentImage = From b4809ed0d22913e2d9949d2b27c8ae88594dbe99 Mon Sep 17 00:00:00 2001 From: Pedro Kohler Date: Wed, 25 Mar 2026 16:47:37 -0300 Subject: [PATCH 3/4] fix(tools): exclude test files from ESM build tsconfig The build:esm command was failing because tsconfig.json included test files under src/**/__tests__/ that use Jest matchers not available in the Jasmine type definitions used during compilation. Made-with: Cursor --- packages/tools/tsconfig.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/tools/tsconfig.json b/packages/tools/tsconfig.json index a954f9c713..a343dffec5 100644 --- a/packages/tools/tsconfig.json +++ b/packages/tools/tsconfig.json @@ -5,5 +5,9 @@ "rootDir": "./src" }, "include": ["./src/**/*"], - "exclude": ["./src/**/*.spec.ts", "./src/**/*.test.ts"] + "exclude": [ + "./src/**/__tests__/**", + "./src/**/*.spec.ts", + "./src/**/*.test.ts" + ] } From 9a64cabaa98ec34bc69eb6ae0b4feb159249a73b Mon Sep 17 00:00:00 2001 From: Pedro Kohler Date: Wed, 25 Mar 2026 18:38:05 -0300 Subject: [PATCH 4/4] fix(segmentation): restore pertinent comment removed during refactor --- .../src/eventListeners/segmentation/imageChangeEventListener.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts b/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts index 9414d1731d..9ca8da8346 100644 --- a/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts +++ b/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts @@ -111,6 +111,8 @@ function _imageChangeEventListener(evt) { // or any actor that needs to be added (which it is added later down), but remove // it here if it is not needed labelmapActors.forEach((actor) => { + // if cannot find a representation for this actor means it has stuck around + // from previous renderings and should be removed const belongsToActiveSegmentation = labelmapRepresentations.some( (representation) => (actor.representationUID as string)?.startsWith(