fix(dicom): keep distinct series with same orientation separate

SeparateOnImageOrientation kept a single cosinesToID lookup across all
input volumes. When two series shared an ImageOrientationPatient (e.g.
T1 MP-RAGE and a sagittal T2 CUBE acquired in the same session) the
second volume's files were reassigned to the first volume's bucket,
collapsing them into one scan. Scope cosinesToID per input volume so
the orientation-based split only happens within a series.

Also fixes the Y-cosine row indexing in areCosinesAlmostEqual: the
second loop iteration walked indices 1,2,3 — a sliding window
straddling the X and Y rows — instead of 3,4,5, the actual Y cosine
row. For axial IOP [1,0,0,0,1,0] this happened to dot to 0 and made
same-orientation volumes look mismatched, which masked the cosinesToID
leak for axial acquisitions. With the leak fixed the windowed
comparison no longer affects outputs, but it's still wrong. While
there, honor the epsilon argument instead of always using the EPSILON
constant.

Adds an e2e regression test that synthesizes two minimal DICOM series
with distinct SeriesInstanceUIDs and identical oblique IOP, loads them
through two manifest URLs, and asserts two volume cards.

Closes #861.

Author: PaulHax

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 <int N>
-double dotProduct(const std::vector<double> &vec1,
-                  const std::vector<double> &vec2) {
-  double result = 0;
-  for (int i = 0; i < N; i++) {
-    result += vec1.at(i) * vec2.at(i);
-  }
-  return result;
-}
-
 std::vector<double> ReadImageOrientationValue(const std::string &filename) {
   gdcm::Reader reader;
   reader.SetFileName(filename.c_str());
@@ -98,16 +86,18 @@ std::vector<double> ReadImageOrientationValue(const std::string &filename) {
   return gdcm::ImageHelper::GetDirectionCosinesValue(file);
 }
 
-bool areCosinesAlmostEqual(std::vector<double> cosines1,
-                           std::vector<double> cosines2,
+bool areCosinesAlmostEqual(const std::vector<double> &cosines1,
+                           const std::vector<double> &cosines2,
                            double epsilon = EPSILON) {
-  for (int i = 0; i <= 1; i++) {
-    std::vector<double> vec1{cosines1.at(i), cosines1.at(i + 1),
-                             cosines1.at(i + 2)};
-    std::vector<double> 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<double> cosines1,
 
 VolumeMapType SeparateOnImageOrientation(const VolumeMapType &volumeMap) {
   VolumeMapType newVolumeMap;
-  // Vector< Pair< cosines, volumeID >>
-  std::vector<std::pair<std::vector<double>, 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::pair<std::vector<double>, std::string>> cosinesToID;
+
     for (const auto &filename : names) {
       std::vector<double> curCosines = ReadImageOrientationValue(filename);
 

src/io/itk-dicom/emscripten-build/dicom.wasm

@@ -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' }
+    );
+  });
+});

src/io/itk-dicom/emscripten-build/dicom.wasm.zst

@@ -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}`;
+};