Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/core/src/RenderingEngine/StackViewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } =
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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<string>();
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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,14 @@ function _imageChangeEventListener(evt) {
// 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) {
// from previous renderings and should be removed
const belongsToActiveSegmentation = labelmapRepresentations.some(
(representation) =>
(actor.representationUID as string)?.startsWith(
`${representation.segmentationId}-${SegmentationRepresentations.Labelmap}`
)
);
if (!belongsToActiveSegmentation) {
viewport.removeActors([actor.uid]);
}
});
Expand All @@ -140,6 +137,7 @@ function _imageChangeEventListener(evt) {
}

let shouldTriggerSegmentationRender = false;
const consumedActorUIDs = new Set<string>();
const updateSegmentationActor = (derivedImageId) => {
const derivedImage = cache.getImage(derivedImageId);

Expand All @@ -157,7 +155,45 @@ function _imageChangeEventListener(evt) {
);

if (!segmentationActorInput) {
// i guess we need to create here
const reusableEntry = actors.find(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember there was an actual reason for this, and i don't remember why. I let QA test it and maybe you fixed it or got fixed

(a) =>
!consumedActorUIDs.has(a.uid) &&
(a.representationUID as string)?.startsWith(
`${segmentationId}-${SegmentationRepresentations.Labelmap}`
) &&
a.referencedId !== derivedImageId
);

if (reusableEntry) {
consumedActorUIDs.add(reusableEntry.uid);
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);

Expand Down
6 changes: 5 additions & 1 deletion packages/tools/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,9 @@
"rootDir": "./src"
},
"include": ["./src/**/*"],
"exclude": ["./src/**/*.spec.ts", "./src/**/*.test.ts"]
"exclude": [
"./src/**/__tests__/**",
"./src/**/*.spec.ts",
"./src/**/*.test.ts"
]
}
Loading