diff --git a/src/io/itk-dicom/dicom.cpp b/src/io/itk-dicom/dicom.cpp index 9ddf76aa8..38dd80d74 100644 --- a/src/io/itk-dicom/dicom.cpp +++ b/src/io/itk-dicom/dicom.cpp @@ -75,18 +75,6 @@ void replaceChars(std::string &str, char search, char replaceChar) { } } -// doesn't actually do any length checks, or overflow checks, or anything -// really. -template -double dotProduct(const std::vector &vec1, - const std::vector &vec2) { - double result = 0; - for (int i = 0; i < N; i++) { - result += vec1.at(i) * vec2.at(i); - } - return result; -} - std::vector ReadImageOrientationValue(const std::string &filename) { gdcm::Reader reader; reader.SetFileName(filename.c_str()); @@ -98,16 +86,18 @@ std::vector ReadImageOrientationValue(const std::string &filename) { return gdcm::ImageHelper::GetDirectionCosinesValue(file); } -bool areCosinesAlmostEqual(std::vector cosines1, - std::vector cosines2, +bool areCosinesAlmostEqual(const std::vector &cosines1, + const std::vector &cosines2, double epsilon = EPSILON) { - for (int i = 0; i <= 1; i++) { - std::vector vec1{cosines1.at(i), cosines1.at(i + 1), - cosines1.at(i + 2)}; - std::vector vec2{cosines2.at(i), cosines2.at(i + 1), - cosines2.at(i + 2)}; - double dot = dotProduct<3>(vec1, vec2); - if (dot < (1 - EPSILON)) { + // ImageOrientationPatient is two row vectors: X cosines at [0..2] and + // Y cosines at [3..5]. Compare each row. + for (int row = 0; row < 2; row++) { + const int offset = row * 3; + double dot = 0; + for (int i = 0; i < 3; i++) { + dot += cosines1.at(offset + i) * cosines2.at(offset + i); + } + if (dot < (1 - epsilon)) { return false; } } @@ -116,8 +106,6 @@ bool areCosinesAlmostEqual(std::vector cosines1, VolumeMapType SeparateOnImageOrientation(const VolumeMapType &volumeMap) { VolumeMapType newVolumeMap; - // Vector< Pair< cosines, volumeID >> - std::vector, std::string>> cosinesToID; // append unique ID part to the volume ID, based on cosines // The format replaces non-alphanumeric chars to be semi-consistent with DICOM @@ -141,6 +129,10 @@ VolumeMapType SeparateOnImageOrientation(const VolumeMapType &volumeMap) { }; for (const auto &[volumeID, names] : volumeMap) { + // Scope the orientation lookup to a single input volume so distinct + // series with identical orientations stay separate. + std::vector, std::string>> cosinesToID; + for (const auto &filename : names) { std::vector curCosines = ReadImageOrientationValue(filename); diff --git a/src/io/itk-dicom/emscripten-build/dicom.wasm b/src/io/itk-dicom/emscripten-build/dicom.wasm index a321899a5..f3a160482 100755 Binary files a/src/io/itk-dicom/emscripten-build/dicom.wasm and b/src/io/itk-dicom/emscripten-build/dicom.wasm differ diff --git a/src/io/itk-dicom/emscripten-build/dicom.wasm.zst b/src/io/itk-dicom/emscripten-build/dicom.wasm.zst index f41dd9887..c7bc00632 100755 Binary files a/src/io/itk-dicom/emscripten-build/dicom.wasm.zst and b/src/io/itk-dicom/emscripten-build/dicom.wasm.zst differ diff --git a/tests/specs/multi-series-load.e2e.ts b/tests/specs/multi-series-load.e2e.ts new file mode 100644 index 000000000..b15140ac3 --- /dev/null +++ b/tests/specs/multi-series-load.e2e.ts @@ -0,0 +1,69 @@ +// Regression for https://github.com/Kitware/VolView/issues/861 +// +// Two DICOM series with distinct SeriesInstanceUIDs but identical +// ImageOrientationPatient must remain two volume cards, not collapse +// into one merged scan. +// +// Synthetic DICOMs are generated on the fly so the test has no external +// dependencies. +import * as path from 'path'; +import * as fs from 'fs'; +import { volViewPage } from '../pageobjects/volview.page'; +import { TEMP_DIR } from '../../wdio.shared.conf'; +import { buildSyntheticDicom, newUid } from './syntheticDicom'; + +// Oblique ImageOrientationPatient — same value for both series. With +// this orientation the pre-fix areCosinesAlmostEqual sees a non-zero +// second window and the cross-volume cosinesToID leak collapses both +// series into one. +const SHARED_IMAGE_ORIENTATION_PATIENT = [ + -0.00964, 0.99248, 0.12202, 0.06932, 0.12239, -0.99006, +] as const; + +const SLICE_COUNT = 5; +const STUDY_UID = newUid(); + +function writeSeries(label: 'A' | 'B', dirName: string, manifestName: string) { + const seriesUid = newUid(); + const dir = path.join(TEMP_DIR, dirName); + fs.mkdirSync(dir, { recursive: true }); + + const resources: { url: string; name: string }[] = []; + for (let i = 0; i < SLICE_COUNT; i++) { + const filename = `slice-${i}.dcm`; + const bytes = buildSyntheticDicom({ + studyUid: STUDY_UID, + seriesUid, + sopUid: newUid(), + instanceNumber: i + 1, + imageOrientationPatient: SHARED_IMAGE_ORIENTATION_PATIENT, + // Step along the slice-normal so GDCM can sort within the series. + imagePositionPatient: [0, 0, i], + }); + fs.writeFileSync(path.join(dir, filename), bytes); + resources.push({ url: `tmp/${dirName}/${filename}`, name: filename }); + } + + fs.writeFileSync( + path.join(TEMP_DIR, manifestName), + JSON.stringify({ resources }) + ); +} + +describe('Multi-series load: two series with identical ImageOrientationPatient', () => { + before(() => { + writeSeries('A', 'multi-series-A', 'multi-series-A.json'); + writeSeries('B', 'multi-series-B', 'multi-series-B.json'); + }); + + it('keeps the two series separate (two volume cards)', async () => { + await volViewPage.open( + '?urls=[tmp/multi-series-A.json,tmp/multi-series-B.json]' + ); + await volViewPage.waitForViews(); + await browser.waitUntil( + async () => (await $$('.volume-card').length) === 2, + { timeout: 30000, timeoutMsg: 'expected exactly two volume cards' } + ); + }); +}); diff --git a/tests/specs/syntheticDicom.ts b/tests/specs/syntheticDicom.ts new file mode 100644 index 000000000..8b2c4c410 --- /dev/null +++ b/tests/specs/syntheticDicom.ts @@ -0,0 +1,196 @@ +// Minimal synthetic DICOM (Explicit VR Little Endian) for tests. +// Emits just enough tags for ITK/GDCM to categorize and load a series: +// SOP Class/Instance UIDs, Study/SeriesInstanceUID, SeriesNumber, Modality, +// Patient identifiers, ImageOrientationPatient, ImagePositionPatient, +// PixelSpacing, SliceThickness, image geometry, and zeroed PixelData. + +const SOP_CLASS_MR = '1.2.840.10008.5.1.4.1.1.4'; +const TS_EXPLICIT_VR_LE = '1.2.840.10008.1.2.1'; + +const enc = new TextEncoder(); + +const padUi = (s: string) => (s.length % 2 === 0 ? s : `${s}\0`); +const padText = (s: string) => (s.length % 2 === 0 ? s : `${s} `); + +const writeShort = (v: number) => { + const b = new Uint8Array(2); + new DataView(b.buffer).setUint16(0, v, true); + return b; +}; + +const writeLong = (v: number) => { + const b = new Uint8Array(4); + new DataView(b.buffer).setUint32(0, v, true); + return b; +}; + +const tagBytes = (group: number, element: number) => { + const b = new Uint8Array(4); + const dv = new DataView(b.buffer); + dv.setUint16(0, group, true); + dv.setUint16(2, element, true); + return b; +}; + +const combine = (...arr: Uint8Array[]) => { + const total = arr.reduce((acc, a) => acc + a.length, 0); + const out = new Uint8Array(total); + let off = 0; + for (const a of arr) { + out.set(a, off); + off += a.length; + } + return out; +}; + +// Short-form explicit VR (2-byte length): UI, CS, DA, DS, IS, LO, PN, SH, US, UL, ... +const elemShort = ( + group: number, + element: number, + vr: string, + value: Uint8Array +) => + combine( + tagBytes(group, element), + enc.encode(vr), + writeShort(value.length), + value + ); + +// Long-form explicit VR (2-byte reserved + 4-byte length): OB, OW, UN, SQ, ... +const elemLong = ( + group: number, + element: number, + vr: string, + value: Uint8Array +) => + combine( + tagBytes(group, element), + enc.encode(vr), + new Uint8Array(2), + writeLong(value.length), + value + ); + +const ui = (g: number, e: number, v: string) => + elemShort(g, e, 'UI', enc.encode(padUi(v))); +const cs = (g: number, e: number, v: string) => + elemShort(g, e, 'CS', enc.encode(padText(v))); +const pn = (g: number, e: number, v: string) => + elemShort(g, e, 'PN', enc.encode(padText(v))); +const lo = (g: number, e: number, v: string) => + elemShort(g, e, 'LO', enc.encode(padText(v))); +const sh = (g: number, e: number, v: string) => + elemShort(g, e, 'SH', enc.encode(padText(v))); +const da = (g: number, e: number, v: string) => + elemShort(g, e, 'DA', enc.encode(padText(v))); +const is = (g: number, e: number, v: string) => + elemShort(g, e, 'IS', enc.encode(padText(v))); +const ds = (g: number, e: number, v: string) => + elemShort(g, e, 'DS', enc.encode(padText(v))); +const us = (g: number, e: number, v: number) => + elemShort(g, e, 'US', writeShort(v)); + +export type SyntheticSliceOptions = { + studyUid: string; + seriesUid: string; + sopUid: string; + instanceNumber: number; + imageOrientationPatient: readonly [ + number, + number, + number, + number, + number, + number, + ]; + imagePositionPatient: readonly [number, number, number]; + rows?: number; + cols?: number; + pixelSpacing?: readonly [number, number]; + sliceThickness?: number; + modality?: string; + patientName?: string; + patientId?: string; + seriesNumber?: number; + studyDate?: string; +}; + +export function buildSyntheticDicom(opts: SyntheticSliceOptions): Uint8Array { + const { + studyUid, + seriesUid, + sopUid, + instanceNumber, + imageOrientationPatient, + imagePositionPatient, + rows = 4, + cols = 4, + pixelSpacing = [1, 1] as const, + sliceThickness = 1, + modality = 'MR', + patientName = 'TEST', + patientId = 'TEST001', + seriesNumber = 1, + studyDate = '20260101', + } = opts; + + const dataset = combine( + ui(0x0008, 0x0016, SOP_CLASS_MR), + ui(0x0008, 0x0018, sopUid), + da(0x0008, 0x0020, studyDate), + da(0x0008, 0x0021, studyDate), + cs(0x0008, 0x0060, modality), + pn(0x0010, 0x0010, patientName), + lo(0x0010, 0x0020, patientId), + da(0x0010, 0x0030, '19700101'), + cs(0x0010, 0x0040, 'O'), + ds(0x0018, 0x0050, String(sliceThickness)), + ui(0x0020, 0x000d, studyUid), + ui(0x0020, 0x000e, seriesUid), + sh(0x0020, 0x0010, '1'), + is(0x0020, 0x0011, String(seriesNumber)), + is(0x0020, 0x0013, String(instanceNumber)), + ds( + 0x0020, + 0x0032, + imagePositionPatient.map((n) => n.toString()).join('\\') + ), + ds( + 0x0020, + 0x0037, + imageOrientationPatient.map((n) => n.toString()).join('\\') + ), + us(0x0028, 0x0002, 1), + cs(0x0028, 0x0004, 'MONOCHROME2'), + us(0x0028, 0x0010, rows), + us(0x0028, 0x0011, cols), + ds(0x0028, 0x0030, pixelSpacing.map((n) => n.toString()).join('\\')), + us(0x0028, 0x0100, 16), + us(0x0028, 0x0101, 16), + us(0x0028, 0x0102, 15), + us(0x0028, 0x0103, 0), + elemLong(0x7fe0, 0x0010, 'OW', new Uint8Array(rows * cols * 2)) + ); + + const fileMetaBody = combine( + elemLong(0x0002, 0x0001, 'OB', new Uint8Array([0x00, 0x01])), + ui(0x0002, 0x0002, SOP_CLASS_MR), + ui(0x0002, 0x0003, sopUid), + ui(0x0002, 0x0010, TS_EXPLICIT_VR_LE) + ); + const fileMeta = combine( + elemShort(0x0002, 0x0000, 'UL', writeLong(fileMetaBody.length)), + fileMetaBody + ); + + return combine(new Uint8Array(128), enc.encode('DICM'), fileMeta, dataset); +} + +// Pseudo-UID unique per call within a test process. All-numeric so it +// stays within DICOM UID character constraints. +let counter = 0; +export const newUid = () => { + counter += 1; + return `1.2.826.0.1.3680043.10.999.${process.pid}.${Date.now()}.${counter}`; +};