fix(dicom): keep series with same orientation separate

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);
 

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

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