diff --git a/src/io/itk-dicom/dicom.cpp b/src/io/itk-dicom/dicom.cpp index 9ddf76aa8..5558bce91 100644 --- a/src/io/itk-dicom/dicom.cpp +++ b/src/io/itk-dicom/dicom.cpp @@ -101,13 +101,16 @@ std::vector ReadImageOrientationValue(const std::string &filename) { bool areCosinesAlmostEqual(std::vector cosines1, 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)}; + // 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; + std::vector vec1{cosines1.at(offset), cosines1.at(offset + 1), + cosines1.at(offset + 2)}; + std::vector vec2{cosines2.at(offset), cosines2.at(offset + 1), + cosines2.at(offset + 2)}; double dot = dotProduct<3>(vec1, vec2); - if (dot < (1 - EPSILON)) { + if (dot < (1 - epsilon)) { return false; } } @@ -116,8 +119,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 +142,11 @@ VolumeMapType SeparateOnImageOrientation(const VolumeMapType &volumeMap) { }; for (const auto &[volumeID, names] : volumeMap) { + // Per-input-volume orientation lookup. Two distinct series sharing an + // orientation must remain separate, so this list is rebuilt per input + // volume rather than carried across them (issue #861). + 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..bd3cbd13f 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..5c5846a79 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/data/create-dicom-japanese-patient-name.py b/tests/data/create-dicom-japanese-patient-name.py new file mode 100644 index 000000000..61687bbda --- /dev/null +++ b/tests/data/create-dicom-japanese-patient-name.py @@ -0,0 +1,87 @@ +#!/usr/bin/env python3 +"""Generate the Japanese-Patient-Name DICOM fixture for the dicom-japanese-name +e2e spec. + +Run from the VolView repo root: + + python3 tests/data/create-dicom-japanese-patient-name.py + +Output goes to .tmp/ (the wdio static-server mount and shared dump folder), +which is gitignored. + +The patient name (山田倍太郎) is encoded with Specific Character Set +"ISO 2022 IR 87" (JIS X 0208). Character 倍 has JIS code 0x475C, whose +trailing byte is 0x5C — the original "Unterminated string in JSON" failure +mode in #841 surfaces from that 0x5C leaking into JSON output. +""" + +from pathlib import Path +import numpy as np +import pydicom +from pydicom.dataset import Dataset, FileDataset +from pydicom.uid import CTImageStorage, generate_uid + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent +OUT_DIR = REPO_ROOT / ".tmp" +OUT = OUT_DIR / "dicom-japanese-patient-name.dcm" + + +def build() -> FileDataset: + file_meta = Dataset() + file_meta.MediaStorageSOPClassUID = CTImageStorage + file_meta.MediaStorageSOPInstanceUID = generate_uid() + file_meta.TransferSyntaxUID = pydicom.uid.ExplicitVRLittleEndian + file_meta.ImplementationClassUID = generate_uid() + + ds = FileDataset(str(OUT), {}, file_meta=file_meta, preamble=b"\x00" * 128) + + ds.SOPClassUID = CTImageStorage + ds.SOPInstanceUID = file_meta.MediaStorageSOPInstanceUID + ds.StudyInstanceUID = generate_uid() + ds.SeriesInstanceUID = generate_uid() + ds.Modality = "CT" + + ds.SpecificCharacterSet = ["", "ISO 2022 IR 87"] + ds.PatientName = "山田倍太郎" + ds.PatientID = "TEST001" + ds.StudyDate = "20240101" + ds.StudyDescription = "Test" + ds.SeriesNumber = "1" + ds.InstanceNumber = "1" + + ds.ImagePositionPatient = [0.0, 0.0, 0.0] + ds.ImageOrientationPatient = [1.0, 0.0, 0.0, 0.0, 1.0, 0.0] + ds.PixelSpacing = [1.0, 1.0] + ds.SliceThickness = 1.0 + ds.RescaleIntercept = -1024 + ds.RescaleSlope = 1 + + ds.SamplesPerPixel = 1 + ds.PhotometricInterpretation = "MONOCHROME2" + ds.Rows = 64 + ds.Columns = 64 + ds.BitsAllocated = 16 + ds.BitsStored = 16 + ds.HighBit = 15 + ds.PixelRepresentation = 0 + + pixels = np.zeros((64, 64), dtype=np.uint16) + cy, cx = 32, 32 + for i in range(64): + for j in range(64): + d = ((i - cy) ** 2 + (j - cx) ** 2) ** 0.5 + pixels[i, j] = max(0, int(2000 - d * 30)) + ds.PixelData = pixels.tobytes() + + return ds + + +def main() -> None: + OUT_DIR.mkdir(parents=True, exist_ok=True) + ds = build() + ds.save_as(OUT, write_like_original=False) + print(f"Wrote {OUT} ({OUT.stat().st_size} bytes)") + + +if __name__ == "__main__": + main() diff --git a/tests/specs/dicom-japanese-name.e2e.ts b/tests/specs/dicom-japanese-name.e2e.ts new file mode 100644 index 000000000..685dcd7d2 --- /dev/null +++ b/tests/specs/dicom-japanese-name.e2e.ts @@ -0,0 +1,27 @@ +import * as path from 'path'; +import * as fs from 'fs'; +import { TEMP_DIR } from '../../wdio.shared.conf'; +import { volViewPage } from '../pageobjects/volview.page'; +import { openVolViewPage } from './utils'; + +const FIXTURE_NAME = 'dicom-japanese-patient-name.dcm'; + +describe('DICOM with Japanese Patient Name (ISO 2022 IR 87)', () => { + before(() => { + const fixturePath = path.join(TEMP_DIR, FIXTURE_NAME); + if (!fs.existsSync(fixturePath)) { + throw new Error( + `Missing fixture ${fixturePath}. Generate it once with:\n` + + ` python3 tests/data/create-dicom-japanese-patient-name.py` + ); + } + }); + + it('should load without errors', async () => { + await openVolViewPage(FIXTURE_NAME); + + const views = await volViewPage.views; + const viewCount = await views.length; + expect(viewCount).toBeGreaterThan(0); + }); +}); diff --git a/tests/specs/multi-series-load.e2e.ts b/tests/specs/multi-series-load.e2e.ts new file mode 100644 index 000000000..d52a92918 --- /dev/null +++ b/tests/specs/multi-series-load.e2e.ts @@ -0,0 +1,74 @@ +// 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('study'); + +function writeSeries(label: 'A' | 'B', dirName: string, manifestName: string) { + const seriesUid = newUid(`series${label}`); + 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(`sop${label}${i}`), + 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) >= 1, + { timeout: 30000, timeoutMsg: 'no volume cards appeared' } + ); + // Allow the second series's chunks to finish import. + await browser.pause(2000); + + const cards = await $$('.volume-card'); + expect(cards.length).toEqual(2); + }); +}); diff --git a/tests/specs/syntheticDicom.ts b/tests/specs/syntheticDicom.ts new file mode 100644 index 000000000..499e6082d --- /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); +} + +// Stable pseudo-UID with the given suffix; not a real OID but unique +// per call within a test process. +let counter = 0; +export const newUid = (label: string) => { + counter += 1; + return `1.2.826.0.1.3680043.10.999.${process.pid}.${Date.now()}.${counter}.${label}`; +};