Skip to content

Commit 6c6cead

Browse files
committed
fix(dicom): preserve slices across incremental imports
Adding chunks to an existing DICOM image re-sorts the list and reallocates the volume buffer. Slices already decoded were zeroed by the reallocation and never rewritten while their status still said Loaded. A decode in flight across that reallocation wrote through the slot it captured before the sort, so its pixels or its error could land on another chunk's slice. Two overlapping additions each awaited a sort of the shared list, and the sort that settled last won, dropping the newer call's chunks. The image cache also starts a load only when it first registers an image, so chunks added on a re-import were allocated for but never asked to load. Redecode every chunk that holds data after a reallocation and report those slices as Loading until the rewrite lands. Resolve a chunk's slot after its decode settles, and discard any result from a previous allocation generation, so completion order cannot overwrite current state. Address failures by chunk rather than by captured index. Serialize additions through a promise queue. Ask an already-registered image to start loading after a re-import adds to it.
1 parent 0dd3577 commit 6c6cead

4 files changed

Lines changed: 550 additions & 36 deletions

File tree

‎src/core/streaming/__tests__/dicomChunkImage.spec.ts‎

Lines changed: 359 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ function decodeTo(dataFor: (value: number) => ArrayLike<number>) {
8484
return read;
8585
}
8686

87+
const readDicomImage = decodeTo((value) =>
88+
new Uint16Array(PIXELS_PER_SLICE).fill(value)
89+
);
90+
8791
function sliceOf(image: DicomChunkImage, index: number) {
8892
const scalars = image.getVtkImageData().getPointData().getScalars();
8993
const data = scalars.getData();
@@ -93,11 +97,11 @@ function sliceOf(image: DicomChunkImage, index: number) {
9397
}
9498

9599
async function loadRejectingSeries(
96-
readDicomImage: DicomChunkImageInit['readDicomImage']
100+
read: DicomChunkImageInit['readDicomImage']
97101
) {
98102
const image = new DicomChunkImage({
99103
splitAndSort: splitAndSortByPosition,
100-
readDicomImage,
104+
readDicomImage: read,
101105
});
102106
const errors: unknown[] = [];
103107
image.addEventListener('chunkError', ({ error }) => {
@@ -180,4 +184,357 @@ describe('DicomChunkImage', () => {
180184
expect(message).toContain('fractional');
181185
expect(message).toContain('Uint8Array');
182186
});
187+
188+
it('serializes concurrent additions so an older sort cannot drop newer chunks', async () => {
189+
const pendingSorts: Array<{
190+
chunks: Chunk[];
191+
resolve: (volumes: Record<string, Chunk[]>) => void;
192+
}> = [];
193+
const deferredSplitAndSort: DicomChunkImageInit['splitAndSort'] = (
194+
chunks
195+
) =>
196+
new Promise((resolve) => {
197+
pendingSorts.push({ chunks: [...chunks], resolve });
198+
});
199+
const finishSort = (index: number) => {
200+
pendingSorts[index].resolve({
201+
volume: [...pendingSorts[index].chunks].sort((a, b) => zOf(a) - zOf(b)),
202+
});
203+
};
204+
const image = new DicomChunkImage({
205+
splitAndSort: deferredSplitAndSort,
206+
readDicomImage,
207+
});
208+
const [first, second] = await Promise.all([
209+
makeLoadedChunk(1),
210+
makeLoadedChunk(2),
211+
]);
212+
213+
const olderAddition = image.addChunks([second]);
214+
await vi.waitFor(() => expect(pendingSorts).toHaveLength(1));
215+
216+
const newerAddition = image.addChunks([first]);
217+
await Promise.resolve();
218+
expect(pendingSorts).toHaveLength(1);
219+
220+
finishSort(0);
221+
await olderAddition;
222+
await vi.waitFor(() => expect(pendingSorts).toHaveLength(2));
223+
expect(pendingSorts[1].chunks.map(zOf)).toEqual([2, 1]);
224+
225+
finishSort(1);
226+
await newerAddition;
227+
await vi.waitFor(() =>
228+
expect(image.getChunkStatuses()).toEqual([
229+
ChunkStatus.Loaded,
230+
ChunkStatus.Loaded,
231+
])
232+
);
233+
234+
expect(image.getChunks().map(zOf)).toEqual([1, 2]);
235+
expect(sliceOf(image, 0)).toEqual(Array(PIXELS_PER_SLICE).fill(1));
236+
expect(sliceOf(image, 1)).toEqual(Array(PIXELS_PER_SLICE).fill(2));
237+
238+
image.dispose();
239+
});
240+
241+
it('decodes every loaded chunk into its sorted slice position', async () => {
242+
const image = new DicomChunkImage({
243+
splitAndSort: splitAndSortByPosition,
244+
readDicomImage,
245+
});
246+
247+
const loads: Array<{ z: number; zRange: number[] }> = [];
248+
image.addEventListener('chunkLoad', ({ chunk, updatedExtent }) => {
249+
loads.push({ z: zOf(chunk), zRange: updatedExtent.slice(4) });
250+
});
251+
252+
const [first, second, third] = await Promise.all([
253+
makeLoadedChunk(1),
254+
makeLoadedChunk(2),
255+
makeLoadedChunk(3),
256+
]);
257+
258+
await image.addChunks([first]);
259+
await vi.waitFor(() => expect(loads.length).toBe(1));
260+
261+
// Arrival order deliberately differs from slice order.
262+
await image.addChunks([third, second]);
263+
await vi.waitFor(() => expect(loads.length).toBe(4));
264+
265+
expect(image.getChunks().map(zOf)).toEqual([1, 2, 3]);
266+
// The first slice is redecoded because reallocation cleared its pixels.
267+
expect(loads).toEqual([
268+
{ z: 1, zRange: [0, 0] },
269+
{ z: 1, zRange: [0, 0] },
270+
{ z: 2, zRange: [1, 1] },
271+
{ z: 3, zRange: [2, 2] },
272+
]);
273+
expect(sliceOf(image, 0)).toEqual(Array(PIXELS_PER_SLICE).fill(1));
274+
expect(sliceOf(image, 1)).toEqual(Array(PIXELS_PER_SLICE).fill(2));
275+
expect(sliceOf(image, 2)).toEqual(Array(PIXELS_PER_SLICE).fill(3));
276+
277+
image.dispose();
278+
});
279+
280+
it('keeps a stale in-flight decode from clobbering the re-sorted volume', async () => {
281+
// Hold each decode independently by its pixel value.
282+
const pending = new Map<number, Array<() => void>>();
283+
const deferredRead: DicomChunkImageInit['readDicomImage'] = async (
284+
file
285+
) => {
286+
const value = Number(await file.text());
287+
return new Promise((resolve) => {
288+
const resolvers = pending.get(value) ?? [];
289+
resolvers.push(() =>
290+
resolve({
291+
image: {
292+
size: [COLUMNS, ROWS, 1],
293+
data: new Uint16Array(PIXELS_PER_SLICE).fill(value),
294+
imageType: { components: 1 },
295+
},
296+
})
297+
);
298+
pending.set(value, resolvers);
299+
});
300+
};
301+
302+
const image = new DicomChunkImage({
303+
splitAndSort: splitAndSortByPosition,
304+
readDicomImage: deferredRead,
305+
});
306+
307+
let loads = 0;
308+
image.addEventListener('chunkLoad', () => {
309+
loads += 1;
310+
});
311+
312+
const [first, second, third] = await Promise.all([
313+
makeLoadedChunk(1),
314+
makeLoadedChunk(2),
315+
makeLoadedChunk(3),
316+
]);
317+
318+
// Start chunk 3 in slot 0, then move it to slot 2 while decoding.
319+
await image.addChunks([third]);
320+
await vi.waitFor(() => expect(pending.get(3)).toHaveLength(1));
321+
322+
await image.addChunks([first, second]);
323+
await vi.waitFor(() => {
324+
expect(pending.get(1)).toHaveLength(1);
325+
expect(pending.get(2)).toHaveLength(1);
326+
expect(pending.get(3)).toHaveLength(2);
327+
});
328+
329+
// Complete the current decodes before the stale attempt.
330+
pending.get(1)![0]();
331+
pending.get(2)![0]();
332+
pending.get(3)![1]();
333+
await vi.waitFor(() => expect(loads).toBe(3));
334+
335+
pending.get(3)![0]();
336+
await Promise.resolve();
337+
await Promise.resolve();
338+
expect(loads).toBe(3);
339+
340+
expect(sliceOf(image, 0)).toEqual(Array(PIXELS_PER_SLICE).fill(1));
341+
expect(sliceOf(image, 1)).toEqual(Array(PIXELS_PER_SLICE).fill(2));
342+
expect(sliceOf(image, 2)).toEqual(Array(PIXELS_PER_SLICE).fill(3));
343+
344+
image.dispose();
345+
});
346+
347+
it('does not let a stale success overwrite a replacement failure', async () => {
348+
// Hold each decode independently by its pixel value.
349+
const pending = new Map<number, Array<(err?: Error) => void>>();
350+
const deferredRead: DicomChunkImageInit['readDicomImage'] = async (
351+
file
352+
) => {
353+
const value = Number(await file.text());
354+
return new Promise((resolve, reject) => {
355+
const settlers = pending.get(value) ?? [];
356+
settlers.push((err) => {
357+
if (err) reject(err);
358+
else
359+
resolve({
360+
image: {
361+
size: [COLUMNS, ROWS, 1],
362+
data: new Uint16Array(PIXELS_PER_SLICE).fill(value),
363+
imageType: { components: 1 },
364+
},
365+
});
366+
});
367+
pending.set(value, settlers);
368+
});
369+
};
370+
371+
const image = new DicomChunkImage({
372+
splitAndSort: splitAndSortByPosition,
373+
readDicomImage: deferredRead,
374+
});
375+
376+
const errors: number[] = [];
377+
image.addEventListener('chunkError', ({ chunk }) => {
378+
errors.push(zOf(chunk));
379+
});
380+
381+
const [first, second, third] = await Promise.all([
382+
makeLoadedChunk(1),
383+
makeLoadedChunk(2),
384+
makeLoadedChunk(3),
385+
]);
386+
387+
// Start chunk 3 in slot 0, then move it to slot 2 while decoding.
388+
await image.addChunks([third]);
389+
await vi.waitFor(() => expect(pending.get(3)).toHaveLength(1));
390+
391+
await image.addChunks([first, second]);
392+
await vi.waitFor(() => expect(pending.get(3)).toHaveLength(2));
393+
394+
pending.get(1)![0]();
395+
pending.get(2)![0]();
396+
await vi.waitFor(() =>
397+
expect(image.getChunkStatuses()[1]).toBe(ChunkStatus.Loaded)
398+
);
399+
400+
// Fail the attempt for the current allocation.
401+
pending.get(3)![1](new Error('replacement decode failed'));
402+
await vi.waitFor(() => expect(errors).toEqual([3]));
403+
404+
expect(image.getChunkStatuses()).toEqual([
405+
ChunkStatus.Loaded,
406+
ChunkStatus.Loaded,
407+
ChunkStatus.Errored,
408+
]);
409+
expect(sliceOf(image, 0)).toEqual(Array(PIXELS_PER_SLICE).fill(1));
410+
411+
// A late success from the previous allocation must be ignored.
412+
pending.get(3)![0]();
413+
await Promise.resolve();
414+
await Promise.resolve();
415+
expect(image.getChunkStatuses()[2]).toBe(ChunkStatus.Errored);
416+
expect(sliceOf(image, 2)).toEqual(Array(PIXELS_PER_SLICE).fill(0));
417+
418+
image.dispose();
419+
});
420+
421+
it('does not let a stale failure overwrite a replacement success', async () => {
422+
const pending = new Map<number, Array<(err?: Error) => void>>();
423+
const deferredRead: DicomChunkImageInit['readDicomImage'] = async (
424+
file
425+
) => {
426+
const value = Number(await file.text());
427+
return new Promise((resolve, reject) => {
428+
const settlers = pending.get(value) ?? [];
429+
settlers.push((err) => {
430+
if (err) reject(err);
431+
else
432+
resolve({
433+
image: {
434+
size: [COLUMNS, ROWS, 1],
435+
data: new Uint16Array(PIXELS_PER_SLICE).fill(value),
436+
imageType: { components: 1 },
437+
},
438+
});
439+
});
440+
pending.set(value, settlers);
441+
});
442+
};
443+
444+
const image = new DicomChunkImage({
445+
splitAndSort: splitAndSortByPosition,
446+
readDicomImage: deferredRead,
447+
});
448+
449+
const errors: number[] = [];
450+
image.addEventListener('chunkError', ({ chunk }) => {
451+
errors.push(zOf(chunk));
452+
});
453+
454+
const [first, second, third] = await Promise.all([
455+
makeLoadedChunk(1),
456+
makeLoadedChunk(2),
457+
makeLoadedChunk(3),
458+
]);
459+
460+
await image.addChunks([third]);
461+
await vi.waitFor(() => expect(pending.get(3)).toHaveLength(1));
462+
463+
await image.addChunks([first, second]);
464+
await vi.waitFor(() => expect(pending.get(3)).toHaveLength(2));
465+
466+
pending.get(1)![0]();
467+
pending.get(2)![0]();
468+
pending.get(3)![1]();
469+
await vi.waitFor(() =>
470+
expect(image.getChunkStatuses()).toEqual([
471+
ChunkStatus.Loaded,
472+
ChunkStatus.Loaded,
473+
ChunkStatus.Loaded,
474+
])
475+
);
476+
477+
// A late failure from the previous allocation must be ignored.
478+
pending.get(3)![0](new Error('stale decode failed'));
479+
await Promise.resolve();
480+
await Promise.resolve();
481+
expect(errors).toEqual([]);
482+
expect(image.getChunkStatuses()[2]).toBe(ChunkStatus.Loaded);
483+
expect(sliceOf(image, 2)).toEqual(Array(PIXELS_PER_SLICE).fill(3));
484+
485+
image.dispose();
486+
});
487+
488+
it('reports a reallocated chunk as loading until its slice is rewritten', async () => {
489+
const pending: Array<() => void> = [];
490+
const deferredRead: DicomChunkImageInit['readDicomImage'] = async (
491+
file
492+
) => {
493+
const value = Number(await file.text());
494+
return new Promise((resolve) => {
495+
pending.push(() =>
496+
resolve({
497+
image: {
498+
size: [COLUMNS, ROWS, 1],
499+
data: new Uint16Array(PIXELS_PER_SLICE).fill(value),
500+
imageType: { components: 1 },
501+
},
502+
})
503+
);
504+
});
505+
};
506+
507+
const image = new DicomChunkImage({
508+
splitAndSort: splitAndSortByPosition,
509+
readDicomImage: deferredRead,
510+
});
511+
512+
const [first, second] = await Promise.all([
513+
makeLoadedChunk(1),
514+
makeLoadedChunk(2),
515+
]);
516+
517+
await image.addChunks([first]);
518+
await vi.waitFor(() => expect(pending).toHaveLength(1));
519+
pending[0]();
520+
await vi.waitFor(() => expect(image.status.value).toBe('complete'));
521+
522+
// Reallocation cleared chunk 1, and neither replacement decode has run.
523+
await image.addChunks([second]);
524+
525+
expect(image.getChunkStatuses()).toEqual([
526+
ChunkStatus.Loading,
527+
ChunkStatus.Loading,
528+
]);
529+
expect(image.status.value).toBe('incomplete');
530+
expect(sliceOf(image, 0)).toEqual(Array(PIXELS_PER_SLICE).fill(0));
531+
532+
await vi.waitFor(() => expect(pending).toHaveLength(3));
533+
pending.slice(1).forEach((settle) => settle());
534+
await vi.waitFor(() => expect(image.status.value).toBe('complete'));
535+
expect(sliceOf(image, 0)).toEqual(Array(PIXELS_PER_SLICE).fill(1));
536+
expect(sliceOf(image, 1)).toEqual(Array(PIXELS_PER_SLICE).fill(2));
537+
538+
image.dispose();
539+
});
183540
});

0 commit comments

Comments
 (0)