diff --git a/src/core/streaming/dicomChunkImage.ts b/src/core/streaming/dicomChunkImage.ts index 16fbef9a3..2d2df56f7 100644 --- a/src/core/streaming/dicomChunkImage.ts +++ b/src/core/streaming/dicomChunkImage.ts @@ -407,8 +407,36 @@ export default class DicomChunkImage const pixelData = scalars.getData() as TypedArray; const dims = this.vtkImageData.value.getDimensions(); - const offset = - dims[0] * dims[1] * scalars.getNumberOfComponents() * chunkIndex; + const components = scalars.getNumberOfComponents(); + + // The volume buffer is sized from the first chunk's metadata, so each + // chunk gets a fixed slot: one frame per chunk in a multi-file volume, + // the whole volume when a single multi-frame chunk fills it. + const framesPerChunk = this.chunks.length > 1 ? 1 : dims[2]; + const [chunkWidth, chunkHeight] = result.image.size; + const chunkFrames = result.image.size[2] ?? 1; + const chunkComponents = result.image.imageType.components; + if ( + chunkWidth !== dims[0] || + chunkHeight !== dims[1] || + chunkFrames !== framesPerChunk || + chunkComponents !== components + ) { + // A lone chunk defines the volume it fails to fit, so advice about + // agreeing with the other files only makes sense for a multi-file volume. + const advice = + this.chunks.length > 1 + ? ' Every file in a volume must have the same Rows, Columns, and SamplesPerPixel.' + : ''; + throw new Error( + `File ${chunkId} (chunk ${chunkIndex}) does not fit the volume it belongs to. ` + + `It decoded to ${chunkWidth}x${chunkHeight}x${chunkFrames} with ${chunkComponents} component(s), ` + + `but the volume has room for ${dims[0]}x${dims[1]}x${framesPerChunk} with ${components} component(s).` + + advice + ); + } + + const offset = dims[0] * dims[1] * components * chunkIndex; pixelData.set(result.image.data as TypedArray, offset); const rangeAlreadyInitialized = this.chunkStatus.some( @@ -455,17 +483,27 @@ export default class DicomChunkImage this.onChunksUpdated(); } + // Errored is terminal: a rejected chunk never loads, so waiting on it would + // keep the image loading forever. + private isSettled() { + return this.chunkStatus.every( + (status) => + status === ChunkStatus.Loaded || status === ChunkStatus.Errored + ); + } + private computeStatus(): ProgressiveImageStatus { - for (let i = 0; i < this.chunkStatus.length; i++) { - if (this.chunkStatus[i] !== ChunkStatus.Loaded) return 'incomplete'; - } - return 'complete'; + const anyLoaded = this.chunkStatus.some( + (status) => status === ChunkStatus.Loaded + ); + // An image where every chunk errored holds no pixel data, so it is settled + // but never complete. + return this.isSettled() && anyLoaded ? 'complete' : 'incomplete'; } private onChunksUpdated() { - const status = this.computeStatus(); - this.events.emit('status', status); - if (status === 'complete') { + this.events.emit('status', this.computeStatus()); + if (this.isSettled()) { this.events.emit('loading', false); } } diff --git a/tests/specs/dicom-dimension-mismatch.e2e.ts b/tests/specs/dicom-dimension-mismatch.e2e.ts new file mode 100644 index 000000000..657ed0505 --- /dev/null +++ b/tests/specs/dicom-dimension-mismatch.e2e.ts @@ -0,0 +1,232 @@ +// A DICOM series whose instances disagree on Rows/Columns still groups into a +// single volume, and that volume's buffer is sized from the first instance. +// An instance that does not fit has to be rejected with a message naming it, +// whether it is too big for its slot or too small to fill one, and the rest of +// the series has to reach a usable state anyway. +import * as path from 'path'; +import * as fs from 'fs'; +import { cleanuptotal } from 'wdio-cleanuptotal-service'; +import { volViewPage } from '../pageobjects/volview.page'; +import { TEMP_DIR } from '../../wdio.shared.conf'; +import { buildSyntheticDicom, newUid } from './syntheticDicom'; +import { writeManifestToFile } from './utils'; + +const IMAGE_ORIENTATION_PATIENT = [1, 0, 0, 0, 1, 0] as const; +const SLICE_COUNT = 5; +const MIDDLE_SLICE = 2; + +const SMALL_ROWS = 4; +const SMALL_COLS = 6; +const BIG_ROWS = 8; +const BIG_COLS = 10; + +type Size = { rows: number; cols: number }; + +async function writeSeries( + dirName: string, + series: Size, + outlier: Size, + outlierSlice: number, + manifestName: string +) { + const dir = path.join(TEMP_DIR, dirName); + fs.mkdirSync(dir, { recursive: true }); + cleanuptotal.addCleanup(async () => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + const studyUid = newUid(); + const seriesUid = newUid(); + const resources = []; + let outlierSopUid = ''; + + for (let i = 0; i < SLICE_COUNT; i++) { + const mismatched = i === outlierSlice; + const sopUid = newUid(); + if (mismatched) outlierSopUid = sopUid; + + const { rows, cols } = mismatched ? outlier : series; + const filename = `slice-${i}.dcm`; + fs.writeFileSync( + path.join(dir, filename), + buildSyntheticDicom({ + studyUid, + seriesUid, + sopUid, + instanceNumber: i + 1, + imageOrientationPatient: IMAGE_ORIENTATION_PATIENT, + imagePositionPatient: [0, 0, i], + rows, + cols, + }) + ); + resources.push({ url: `tmp/${dirName}/${filename}`, name: filename }); + } + + await writeManifestToFile({ resources }, manifestName); + + return outlierSopUid; +} + +// The slice count in the "Slice: n/total" overlay of the first 2D view. +async function getRenderedSliceCount() { + return browser.execute((selector: string) => { + const view = document.querySelector(selector); + const match = view?.textContent?.match(/Slice:\s*\d+\s*\/\s*(\d+)/); + return match ? parseInt(match[1], 10) : null; + }, 'div[data-testid~="vtk-two-view"]'); +} + +// A rejected chunk is terminal, so the views stop advertising progress even +// though part of the series never arrived. +async function waitForSeriesToSettle() { + await volViewPage.waitForViews(); + await browser.waitUntil( + async () => (await getRenderedSliceCount()) === SLICE_COUNT, + { + timeout: 30000, + timeoutMsg: `expected the 2D view to show a ${SLICE_COUNT} slice volume`, + } + ); + await browser.waitUntil( + async () => (await $$('.loading-indicator').length) === 0, + { + timeout: 30000, + timeoutMsg: 'expected the views to stop showing a loading indicator', + } + ); +} + +// The badge only says "at least one error so far", so a count read as soon as +// it appears can miss a failure still on its way. Wait for it to hold steady. +async function waitForNotificationCountToSettle() { + let previous = await volViewPage.getNotificationsCount(); + await browser.waitUntil( + async () => { + await browser.pause(500); + const current = await volViewPage.getNotificationsCount(); + const held = current === previous; + previous = current; + return held; + }, + { + timeout: 30000, + timeoutMsg: 'expected the notification count to stop changing', + } + ); + return previous; +} + +async function readErrorDetails() { + await volViewPage.notifications.click(); + + const errorPanel = $('.message-center .v-expansion-panel-title'); + await errorPanel.waitForClickable(); + await errorPanel.click(); + + const details = $('.message-center pre.details'); + await details.waitForDisplayed(); + // The panel animates open, so the text arrives a frame after the element. + await browser.waitUntil(async () => (await details.getText()).length > 0, { + timeout: 5000, + timeoutMsg: 'expected the error details to render', + }); + return details.getText(); +} + +async function loadAndReadRejection(manifestName: string) { + await volViewPage.open(`?urls=[tmp/${manifestName}]`); + await volViewPage.waitForNotification(); + await waitForSeriesToSettle(); + const errorCount = await waitForNotificationCountToSettle(); + const text = await readErrorDetails(); + return { text, errorCount }; +} + +describe('DICOM series with an instance of a different size', () => { + it('names the offending file when it overruns its slot', async () => { + const manifestName = `dimension-mismatch-big-${Date.now()}.json`; + const outlierSopUid = await writeSeries( + 'dimension-mismatch-big', + { rows: SMALL_ROWS, cols: SMALL_COLS }, + { rows: BIG_ROWS, cols: BIG_COLS }, + MIDDLE_SLICE, + manifestName + ); + + const { text, errorCount } = await loadAndReadRejection(manifestName); + + expect(text).toContain(outlierSopUid); + expect(text).toContain(`${BIG_COLS}x${BIG_ROWS}x1`); + expect(text).toContain(`${SMALL_COLS}x${SMALL_ROWS}x1`); + expect(text).toContain('Rows, Columns, and SamplesPerPixel'); + + // Only the instance that does not fit is rejected; the rest of the series + // still loads, so there is exactly one error to report. + expect(errorCount).toBe(1); + }); + + // An under-sized instance fits inside its slot, so the write itself cannot + // detect it; only the metadata comparison can. + it('names the offending file when it underfills its slot', async () => { + const manifestName = `dimension-mismatch-small-${Date.now()}.json`; + const outlierSopUid = await writeSeries( + 'dimension-mismatch-small', + { rows: BIG_ROWS, cols: BIG_COLS }, + { rows: SMALL_ROWS, cols: SMALL_COLS }, + MIDDLE_SLICE, + manifestName + ); + + const { text, errorCount } = await loadAndReadRejection(manifestName); + + expect(text).toContain(outlierSopUid); + expect(text).toContain(`${SMALL_COLS}x${SMALL_ROWS}x1`); + expect(text).toContain(`${BIG_COLS}x${BIG_ROWS}x1`); + expect(errorCount).toBe(1); + }); + + // Both axes differing would still be caught if either half of the comparison + // were dropped; a single differing axis pins down both halves. + it('rejects an instance that differs on rows alone', async () => { + const manifestName = `dimension-mismatch-rows-${Date.now()}.json`; + const outlierSopUid = await writeSeries( + 'dimension-mismatch-rows', + { rows: SMALL_ROWS, cols: SMALL_COLS }, + { rows: BIG_ROWS, cols: SMALL_COLS }, + MIDDLE_SLICE, + manifestName + ); + + const { text, errorCount } = await loadAndReadRejection(manifestName); + + expect(text).toContain(outlierSopUid); + expect(text).toContain(`${SMALL_COLS}x${BIG_ROWS}x1`); + expect(text).toContain(`${SMALL_COLS}x${SMALL_ROWS}x1`); + expect(errorCount).toBe(1); + }); + + // The volume is sized from the first instance, so an outlier there rejects + // every other chunk, one error apiece. Collapsing them into a single + // notification belongs with the error-boundary work in issue 911. + it('rejects the rest of the series when the first instance is the outlier', async () => { + const manifestName = `dimension-mismatch-first-${Date.now()}.json`; + const outlierSopUid = await writeSeries( + 'dimension-mismatch-first', + { rows: SMALL_ROWS, cols: SMALL_COLS }, + { rows: BIG_ROWS, cols: BIG_COLS }, + 0, + manifestName + ); + + const { text, errorCount } = await loadAndReadRejection(manifestName); + + expect(errorCount).toBe(SLICE_COUNT - 1); + // The first instance defines the volume, so it is the other four that are + // named as not fitting. + expect(text).not.toContain(outlierSopUid); + expect(text).toContain('does not fit the volume it belongs to'); + expect(text).toContain(`${SMALL_COLS}x${SMALL_ROWS}x1`); + expect(text).toContain(`${BIG_COLS}x${BIG_ROWS}x1`); + }); +});