Skip to content

Commit bb65efa

Browse files
committed
fix(dicom): reject chunks that do not fit the volume buffer
A series whose instances disagree on Rows, Columns, or SamplesPerPixel is still grouped into one volume, whose buffer is sized from the first instance. An instance that decoded larger than its slot threw "RangeError: offset is out of bounds" from TypedArray.set, naming neither the file nor what was wrong with it. One that decoded smaller wrote inside the buffer and raised nothing at all, leaving a frame smeared across a larger slot. Validate each decoded chunk against its slot before writing, and name the file and both sizes in the error. Rejecting a chunk also has to leave the image usable. An errored chunk is now terminal: an image settles, and stops reporting itself as loading, once every chunk is either loaded or errored, so the views drop their progress indicator and auto window/level is computed from the chunks that did load. An image whose every chunk errored settles but stays incomplete, since it holds no pixel data. An outlier at the first instance sizes the buffer, so every other chunk in its series is rejected against it. Report only the first error per image so one bad instance cannot bury the series under identical notifications.
1 parent 5db8412 commit bb65efa

3 files changed

Lines changed: 284 additions & 9 deletions

File tree

‎src/core/streaming/dicomChunkImage.ts‎

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -407,8 +407,36 @@ export default class DicomChunkImage
407407
const pixelData = scalars.getData() as TypedArray;
408408

409409
const dims = this.vtkImageData.value.getDimensions();
410-
const offset =
411-
dims[0] * dims[1] * scalars.getNumberOfComponents() * chunkIndex;
410+
const components = scalars.getNumberOfComponents();
411+
412+
// The volume buffer is sized from the first chunk's metadata, so each
413+
// chunk gets a fixed slot: one frame per chunk in a multi-file volume,
414+
// the whole volume when a single multi-frame chunk fills it.
415+
const framesPerChunk = this.chunks.length > 1 ? 1 : dims[2];
416+
const [chunkWidth, chunkHeight] = result.image.size;
417+
const chunkFrames = result.image.size[2] ?? 1;
418+
const chunkComponents = result.image.imageType.components;
419+
if (
420+
chunkWidth !== dims[0] ||
421+
chunkHeight !== dims[1] ||
422+
chunkFrames !== framesPerChunk ||
423+
chunkComponents !== components
424+
) {
425+
// A lone chunk defines the volume it fails to fit, so advice about
426+
// agreeing with the other files only makes sense for a multi-file volume.
427+
const advice =
428+
this.chunks.length > 1
429+
? ' Every file in a volume must have the same Rows, Columns, and SamplesPerPixel.'
430+
: '';
431+
throw new Error(
432+
`File ${chunkId} (chunk ${chunkIndex}) does not fit the volume it belongs to. ` +
433+
`It decoded to ${chunkWidth}x${chunkHeight}x${chunkFrames} with ${chunkComponents} component(s), ` +
434+
`but the volume has room for ${dims[0]}x${dims[1]}x${framesPerChunk} with ${components} component(s).` +
435+
advice
436+
);
437+
}
438+
439+
const offset = dims[0] * dims[1] * components * chunkIndex;
412440
pixelData.set(result.image.data as TypedArray, offset);
413441

414442
const rangeAlreadyInitialized = this.chunkStatus.some(
@@ -455,17 +483,27 @@ export default class DicomChunkImage
455483
this.onChunksUpdated();
456484
}
457485

486+
// Errored is terminal: a rejected chunk never loads, so waiting on it would
487+
// keep the image loading forever.
488+
private isSettled() {
489+
return this.chunkStatus.every(
490+
(status) =>
491+
status === ChunkStatus.Loaded || status === ChunkStatus.Errored
492+
);
493+
}
494+
458495
private computeStatus(): ProgressiveImageStatus {
459-
for (let i = 0; i < this.chunkStatus.length; i++) {
460-
if (this.chunkStatus[i] !== ChunkStatus.Loaded) return 'incomplete';
461-
}
462-
return 'complete';
496+
const anyLoaded = this.chunkStatus.some(
497+
(status) => status === ChunkStatus.Loaded
498+
);
499+
// An image where every chunk errored holds no pixel data, so it is settled
500+
// but never complete.
501+
return this.isSettled() && anyLoaded ? 'complete' : 'incomplete';
463502
}
464503

465504
private onChunksUpdated() {
466-
const status = this.computeStatus();
467-
this.events.emit('status', status);
468-
if (status === 'complete') {
505+
this.events.emit('status', this.computeStatus());
506+
if (this.isSettled()) {
469507
this.events.emit('loading', false);
470508
}
471509
}

‎src/store/image-cache.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ export const useImageCacheStore = defineStore('image-cache', () => {
5858
imageErrors[id] ??= [];
5959
imageErrors[id].push(error);
6060

61+
// One bad instance can make every other chunk of its volume fail the same
62+
// way, so only the first error per image is worth a notification. The
63+
// rest stay in imageErrors.
64+
if (imageErrors[id].length > 1) return;
65+
6166
const messageStore = useMessageStore();
6267
messageStore.addError('Error loading DICOM data', { error });
6368
};
Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
// A DICOM series whose instances disagree on Rows/Columns still groups into a
2+
// single volume, and that volume's buffer is sized from the first instance.
3+
// An instance that does not fit has to be rejected with a message naming it,
4+
// whether it is too big for its slot or too small to fill one, and the rest of
5+
// the series has to reach a usable state anyway.
6+
import * as path from 'path';
7+
import * as fs from 'fs';
8+
import { cleanuptotal } from 'wdio-cleanuptotal-service';
9+
import { volViewPage } from '../pageobjects/volview.page';
10+
import { TEMP_DIR } from '../../wdio.shared.conf';
11+
import { buildSyntheticDicom, newUid } from './syntheticDicom';
12+
import { writeManifestToFile } from './utils';
13+
14+
const IMAGE_ORIENTATION_PATIENT = [1, 0, 0, 0, 1, 0] as const;
15+
const SLICE_COUNT = 5;
16+
const MIDDLE_SLICE = 2;
17+
18+
const SMALL_ROWS = 4;
19+
const SMALL_COLS = 6;
20+
const BIG_ROWS = 8;
21+
const BIG_COLS = 10;
22+
23+
type Size = { rows: number; cols: number };
24+
25+
async function writeSeries(
26+
dirName: string,
27+
series: Size,
28+
outlier: Size,
29+
outlierSlice: number,
30+
manifestName: string
31+
) {
32+
const dir = path.join(TEMP_DIR, dirName);
33+
fs.mkdirSync(dir, { recursive: true });
34+
cleanuptotal.addCleanup(async () => {
35+
fs.rmSync(dir, { recursive: true, force: true });
36+
});
37+
38+
const studyUid = newUid();
39+
const seriesUid = newUid();
40+
const resources = [];
41+
let outlierSopUid = '';
42+
43+
for (let i = 0; i < SLICE_COUNT; i++) {
44+
const mismatched = i === outlierSlice;
45+
const sopUid = newUid();
46+
if (mismatched) outlierSopUid = sopUid;
47+
48+
const { rows, cols } = mismatched ? outlier : series;
49+
const filename = `slice-${i}.dcm`;
50+
fs.writeFileSync(
51+
path.join(dir, filename),
52+
buildSyntheticDicom({
53+
studyUid,
54+
seriesUid,
55+
sopUid,
56+
instanceNumber: i + 1,
57+
imageOrientationPatient: IMAGE_ORIENTATION_PATIENT,
58+
imagePositionPatient: [0, 0, i],
59+
rows,
60+
cols,
61+
})
62+
);
63+
resources.push({ url: `tmp/${dirName}/${filename}`, name: filename });
64+
}
65+
66+
await writeManifestToFile({ resources }, manifestName);
67+
68+
return outlierSopUid;
69+
}
70+
71+
// The slice count in the "Slice: n/total" overlay of the first 2D view.
72+
async function getRenderedSliceCount() {
73+
return browser.execute((selector: string) => {
74+
const view = document.querySelector(selector);
75+
const match = view?.textContent?.match(/Slice:\s*\d+\s*\/\s*(\d+)/);
76+
return match ? parseInt(match[1], 10) : null;
77+
}, 'div[data-testid~="vtk-two-view"]');
78+
}
79+
80+
// A rejected chunk is terminal, so the views stop advertising progress even
81+
// though part of the series never arrived.
82+
async function waitForSeriesToSettle() {
83+
await volViewPage.waitForViews();
84+
await browser.waitUntil(
85+
async () => (await getRenderedSliceCount()) === SLICE_COUNT,
86+
{
87+
timeout: 30000,
88+
timeoutMsg: `expected the 2D view to show a ${SLICE_COUNT} slice volume`,
89+
}
90+
);
91+
await browser.waitUntil(
92+
async () => (await $$('.loading-indicator').length) === 0,
93+
{
94+
timeout: 30000,
95+
timeoutMsg: 'expected the views to stop showing a loading indicator',
96+
}
97+
);
98+
}
99+
100+
// The badge only says "at least one error so far", so a count read as soon as
101+
// it appears can miss a failure still on its way. Wait for it to hold steady.
102+
async function waitForNotificationCountToSettle() {
103+
let previous = await volViewPage.getNotificationsCount();
104+
await browser.waitUntil(
105+
async () => {
106+
await browser.pause(500);
107+
const current = await volViewPage.getNotificationsCount();
108+
const held = current === previous;
109+
previous = current;
110+
return held;
111+
},
112+
{
113+
timeout: 30000,
114+
timeoutMsg: 'expected the notification count to stop changing',
115+
}
116+
);
117+
return previous;
118+
}
119+
120+
async function readErrorDetails() {
121+
await volViewPage.notifications.click();
122+
123+
const errorPanel = $('.message-center .v-expansion-panel-title');
124+
await errorPanel.waitForClickable();
125+
await errorPanel.click();
126+
127+
const details = $('.message-center pre.details');
128+
await details.waitForDisplayed();
129+
// The panel animates open, so the text arrives a frame after the element.
130+
await browser.waitUntil(async () => (await details.getText()).length > 0, {
131+
timeout: 5000,
132+
timeoutMsg: 'expected the error details to render',
133+
});
134+
return details.getText();
135+
}
136+
137+
async function loadAndReadRejection(manifestName: string) {
138+
await volViewPage.open(`?urls=[tmp/${manifestName}]`);
139+
await volViewPage.waitForNotification();
140+
await waitForSeriesToSettle();
141+
const errorCount = await waitForNotificationCountToSettle();
142+
const text = await readErrorDetails();
143+
return { text, errorCount };
144+
}
145+
146+
describe('DICOM series with an instance of a different size', () => {
147+
it('names the offending file when it overruns its slot', async () => {
148+
const manifestName = `dimension-mismatch-big-${Date.now()}.json`;
149+
const outlierSopUid = await writeSeries(
150+
'dimension-mismatch-big',
151+
{ rows: SMALL_ROWS, cols: SMALL_COLS },
152+
{ rows: BIG_ROWS, cols: BIG_COLS },
153+
MIDDLE_SLICE,
154+
manifestName
155+
);
156+
157+
const { text, errorCount } = await loadAndReadRejection(manifestName);
158+
159+
expect(text).toContain(outlierSopUid);
160+
expect(text).toContain(`${BIG_COLS}x${BIG_ROWS}x1`);
161+
expect(text).toContain(`${SMALL_COLS}x${SMALL_ROWS}x1`);
162+
expect(text).toContain('Rows, Columns, and SamplesPerPixel');
163+
164+
// Only the instance that does not fit is rejected; the rest of the series
165+
// still loads, so there is exactly one error to report.
166+
expect(errorCount).toBe(1);
167+
});
168+
169+
// An under-sized instance fits inside its slot, so the write itself cannot
170+
// detect it; only the metadata comparison can.
171+
it('names the offending file when it underfills its slot', async () => {
172+
const manifestName = `dimension-mismatch-small-${Date.now()}.json`;
173+
const outlierSopUid = await writeSeries(
174+
'dimension-mismatch-small',
175+
{ rows: BIG_ROWS, cols: BIG_COLS },
176+
{ rows: SMALL_ROWS, cols: SMALL_COLS },
177+
MIDDLE_SLICE,
178+
manifestName
179+
);
180+
181+
const { text, errorCount } = await loadAndReadRejection(manifestName);
182+
183+
expect(text).toContain(outlierSopUid);
184+
expect(text).toContain(`${SMALL_COLS}x${SMALL_ROWS}x1`);
185+
expect(text).toContain(`${BIG_COLS}x${BIG_ROWS}x1`);
186+
expect(errorCount).toBe(1);
187+
});
188+
189+
// Both axes differing would still be caught if either half of the comparison
190+
// were dropped; a single differing axis pins down both halves.
191+
it('rejects an instance that differs on rows alone', async () => {
192+
const manifestName = `dimension-mismatch-rows-${Date.now()}.json`;
193+
const outlierSopUid = await writeSeries(
194+
'dimension-mismatch-rows',
195+
{ rows: SMALL_ROWS, cols: SMALL_COLS },
196+
{ rows: BIG_ROWS, cols: SMALL_COLS },
197+
MIDDLE_SLICE,
198+
manifestName
199+
);
200+
201+
const { text, errorCount } = await loadAndReadRejection(manifestName);
202+
203+
expect(text).toContain(outlierSopUid);
204+
expect(text).toContain(`${SMALL_COLS}x${BIG_ROWS}x1`);
205+
expect(text).toContain(`${SMALL_COLS}x${SMALL_ROWS}x1`);
206+
expect(errorCount).toBe(1);
207+
});
208+
209+
// The volume is sized from the first instance, so an outlier there rejects
210+
// every other chunk. Each failure carries the same message, and repeating it
211+
// once per instance would bury the series under identical notifications.
212+
it('reports one error when the first instance is the outlier', async () => {
213+
const manifestName = `dimension-mismatch-first-${Date.now()}.json`;
214+
const outlierSopUid = await writeSeries(
215+
'dimension-mismatch-first',
216+
{ rows: SMALL_ROWS, cols: SMALL_COLS },
217+
{ rows: BIG_ROWS, cols: BIG_COLS },
218+
0,
219+
manifestName
220+
);
221+
222+
const { text, errorCount } = await loadAndReadRejection(manifestName);
223+
224+
expect(errorCount).toBe(1);
225+
// The first instance defines the volume, so it is the other four that are
226+
// named as not fitting.
227+
expect(text).not.toContain(outlierSopUid);
228+
expect(text).toContain('does not fit the volume it belongs to');
229+
expect(text).toContain(`${SMALL_COLS}x${SMALL_ROWS}x1`);
230+
expect(text).toContain(`${BIG_COLS}x${BIG_ROWS}x1`);
231+
});
232+
});

0 commit comments

Comments
 (0)