fix(MetadataProvider): Correctly assign imageId for multiframe images and remove unused frame information retrieval method (#5965)

extensions/default/src/getSopClassHandlerModule.js

@@ -14,14 +14,13 @@ const DYNAMIC_VOLUME_LOADER_SCHEME = 'cornerstoneStreamingDynamicImageVolume';
 const sopClassHandlerName = 'stack';
 let appContext = {};
 
-const getDynamicVolumeInfo = instances => {
+const getDynamicVolumeInfo = imageIds => {
   const { extensionManager } = appContext;
 
   if (!extensionManager) {
     throw new Error('extensionManager is not available');
   }
 
-  const imageIds = instances.map(({ imageId }) => imageId);
   const volumeLoaderUtility = extensionManager.getModuleEntry(
     '@ohif/extension-cornerstone.utilityModule.volumeLoader'
   );
@@ -34,8 +33,8 @@ const isMultiFrame = instance => {
   return instance.NumberOfFrames > 1;
 };
 
-function getDisplaySetInfo(instances) {
+function getDisplaySetInfo(instances, imageIds) {
-  const dynamicVolumeInfo = getDynamicVolumeInfo(instances);
+  const dynamicVolumeInfo = getDynamicVolumeInfo(imageIds);
   const { isDynamicVolume, timePoints } = dynamicVolumeInfo;
   let displaySetInfo;
 
@@ -82,12 +81,13 @@ const makeDisplaySet = (instances, index) => {
   const imageSet = new ImageSet(instances);
   const { extensionManager } = appContext;
   const dataSource = extensionManager.getActiveDataSource()[0];
+  const imageIds = dataSource.getImageIdsForDisplaySet(imageSet);
   const {
     isDynamicVolume,
     value: isReconstructable,
     averageSpacingBetweenFrames,
     dynamicVolumeInfo,
-  } = getDisplaySetInfo(instances);
+  } = getDisplaySetInfo(instances, imageIds);
 
   const volumeLoaderSchema = isDynamicVolume
     ? DYNAMIC_VOLUME_LOADER_SCHEME
@@ -124,7 +124,6 @@ const makeDisplaySet = (instances, index) => {
     FrameOfReferenceUID: instance.FrameOfReferenceUID,
   });
 
-  const imageIds = dataSource.getImageIdsForDisplaySet(imageSet);
   let imageId = imageIds[Math.floor(imageIds.length / 2)];
   let thumbnailInstance = instances[Math.floor(instances.length / 2)];
   if (isDynamicVolume) {

extensions/default/src/getSopClassHandlerModule.test.js

@@ -0,0 +1,74 @@
+jest.mock('./getDisplaySetMessages', () => jest.fn(() => ({ addMessage: jest.fn() })));
+jest.mock('./getDisplaySetsFromUnsupportedSeries', () => jest.fn());
+
+import getSopClassHandlerModule from './getSopClassHandlerModule';
+
+const makeInstance = () => ({
+  imageId: 'dicomfile:blob://local-multiframe',
+  url: 'dicomfile:blob://local-multiframe',
+  NumberOfFrames: 4,
+  Rows: 2,
+  Columns: 2,
+  PixelSpacing: [1, 1],
+  SliceThickness: 1,
+  ImageOrientationPatient: [1, 0, 0, 0, 1, 0],
+  ImagePositionPatient: [0, 0, 0],
+  FrameOfReferenceUID: 'frame-of-reference',
+  StudyInstanceUID: 'study',
+  SeriesInstanceUID: 'series',
+  SOPInstanceUID: 'sop',
+  SOPClassUID: '1.2.840.10008.5.1.4.1.1.2.1',
+  Modality: 'CT',
+  SeriesDescription: 'Local multiframe CT',
+  SeriesNumber: 1,
+});
+
+describe('getSopClassHandlerModule', () => {
+  it('checks dynamic volume grouping with generated multiframe imageIds', () => {
+    const frameImageIds = [
+      'dicomfile:blob://local-multiframe&frame=1',
+      'dicomfile:blob://local-multiframe&frame=2',
+      'dicomfile:blob://local-multiframe&frame=3',
+      'dicomfile:blob://local-multiframe&frame=4',
+    ];
+    const getDynamicVolumeInfo = jest.fn(() => ({
+      isDynamicVolume: false,
+      timePoints: [frameImageIds],
+      splittingTag: null,
+    }));
+    const dataSource = {
+      getImageIdsForDisplaySet: jest.fn(() => frameImageIds),
+      retrieve: {
+        getGetThumbnailSrc: jest.fn(),
+      },
+    };
+    const customizationService = {
+      getCustomization: jest.fn(() => ({
+        sortFunctions: {},
+        defaultSortFunctionName: 'default',
+      })),
+    };
+    const appContext = {
+      appConfig: {},
+      extensionManager: {
+        getActiveDataSource: jest.fn(() => [dataSource]),
+        getModuleEntry: jest.fn(() => ({
+          exports: {
+            getDynamicVolumeInfo,
+          },
+        })),
+      },
+      servicesManager: {
+        services: {
+          customizationService,
+        },
+      },
+    };
+
+    const stackHandler = getSopClassHandlerModule(appContext)[0];
+
+    stackHandler.getDisplaySetsFromSeries([makeInstance()]);
+
+    expect(getDynamicVolumeInfo).toHaveBeenCalledWith(frameImageIds);
+  });
+});

platform/core/src/classes/MetadataProvider.test.ts

@@ -0,0 +1,82 @@
+import metadataProvider from './MetadataProvider';
+
+beforeEach(() => {
+  (
+    metadataProvider as unknown as {
+      imageURIToUIDs: Map<string, unknown>;
+    }
+  ).imageURIToUIDs.clear();
+});
+
+describe('MetadataProvider', () => {
+  it('uses the WADO frame query parameter as the frame number', () => {
+    expect(
+      metadataProvider.getUIDsFromImageID(
+        'dicomweb:http://localhost/wado?requestType=WADO&studyUID=study-wado&seriesUID=series-wado&objectUID=sop-wado&contentType=application/dicom&transferSyntax=*&frame=5'
+      )
+    ).toEqual({
+      StudyInstanceUID: 'study-wado',
+      SeriesInstanceUID: 'series-wado',
+      SOPInstanceUID: 'sop-wado',
+      frameNumber: '5',
+    });
+  });
+
+  it('uses the frame query parameter for local multiframe imageIds registered by base URL', () => {
+    const baseImageId = 'blob:http://localhost/local-multiframe';
+    const uids = {
+      StudyInstanceUID: 'study-local',
+      SeriesInstanceUID: 'series-local',
+      SOPInstanceUID: 'sop-local',
+    };
+
+    metadataProvider.addImageIdToUIDs(baseImageId, uids);
+
+    expect(metadataProvider.getUIDsFromImageID(`${baseImageId}&frame=3`)).toEqual({
+      ...uids,
+      frameNumber: '3',
+    });
+  });
+
+  it('prefers the frame query parameter over stored frame metadata', () => {
+    const baseImageId = 'blob:http://localhost/local-multiframe-with-frame-number';
+    const uids = {
+      StudyInstanceUID: 'study-local-with-frame-number',
+      SeriesInstanceUID: 'series-local-with-frame-number',
+      SOPInstanceUID: 'sop-local-with-frame-number',
+      frameNumber: '1',
+    };
+
+    metadataProvider.addImageIdToUIDs(baseImageId, uids);
+
+    expect(metadataProvider.getUIDsFromImageID(`${baseImageId}&frame=4`)).toEqual({
+      ...uids,
+      frameNumber: '4',
+    });
+  });
+
+  it('prefers an exact frame imageId mapping before falling back to the base URL', () => {
+    const baseImageId = 'blob:http://localhost/local-multiframe-exact-frame';
+    metadataProvider.addImageIdToUIDs(baseImageId, {
+      StudyInstanceUID: 'study-base',
+      SeriesInstanceUID: 'series-base',
+      SOPInstanceUID: 'sop-base',
+      frameNumber: '1',
+    });
+
+    const frameImageId = `${baseImageId}&frame=3`;
+    metadataProvider.addImageIdToUIDs(frameImageId, {
+      StudyInstanceUID: 'study-frame',
+      SeriesInstanceUID: 'series-frame',
+      SOPInstanceUID: 'sop-frame',
+      frameNumber: '3',
+    });
+
+    expect(metadataProvider.getUIDsFromImageID(frameImageId)).toEqual({
+      StudyInstanceUID: 'study-frame',
+      SeriesInstanceUID: 'series-frame',
+      SOPInstanceUID: 'sop-frame',
+      frameNumber: '3',
+    });
+  });
+});

platform/core/src/classes/MetadataProvider.ts

@@ -1,6 +1,7 @@
 import queryString from 'query-string';
 import dicomParser from 'dicom-parser';
 import { utilities } from '@cornerstonejs/core';
+import { utilities as csMetadataUtilities } from '@cornerstonejs/metadata';
 import { baseImageURIForMetadata } from '../utils/imageIdToURI';
 import DicomMetadataStore from '../services/DicomMetadataStore';
 import fetchPaletteColorLookupTableData from '../utils/metadataProvider/fetchPaletteColorLookupTableData';
@@ -8,6 +9,7 @@ import toNumber from '../utils/toNumber';
 import combineFrameInstance from '../utils/combineFrameInstance';
 
 const { calibratedPixelSpacingMetadataProvider, getPixelSpacingInformation } = utilities;
+const { getUriModule } = csMetadataUtilities;
 
 class MetadataProvider {
   private readonly imageURIToUIDs: Map<string, any> = new Map();
@@ -60,7 +62,22 @@ class MetadataProvider {
       return;
     }
 
-    return (frameNumber && combineFrameInstance(frameNumber, instance)) || instance;
+    const result = (frameNumber && combineFrameInstance(frameNumber, instance)) || instance;
+
+    // We reassign the imageId on the instance because multiframe images processed
+    // through combineFrameInstance will mistakenly get the first imageId.
+    // This happens because the DICOM web data store only keeps the first instance.
+    // Defined non-enumerable so spreading the instance into another object does
+    // not carry the imageId over (it belongs to this frame only).
+    if (result) {
+      Object.defineProperty(result, 'imageId', {
+        value: imageId,
+        writable: true,
+        enumerable: false,
+        configurable: true,
+      });
+    }
+    return result;
   }
 
   get(query, imageId, options = { fallback: false }) {
@@ -423,37 +440,6 @@ class MetadataProvider {
     return metadata;
   }
 
-  /**
-   * Retrieves the frameNumber information, depending on the url style
-   * wadors /frames/1
-   * wadouri &frame=1
-   * @param {*} imageId
-   * @returns
-   */
-  getFrameInformationFromURL(imageId) {
-    function getInformationFromURL(informationString, separator) {
-      let result = '';
-      const splittedStr = imageId.split(informationString)[1];
-      if (splittedStr.includes(separator)) {
-        result = splittedStr.split(separator)[0];
-      } else {
-        result = splittedStr;
-      }
-      return result;
-    }
-
-    if (imageId.includes('/frames')) {
-      return getInformationFromURL('/frames', '/');
-    }
-    if (imageId.includes('?frame=')) {
-      return getInformationFromURL('?frame=', '&');
-    }
-    if (imageId.includes('&frame=')) {
-      return getInformationFromURL('&frame=', '&');
-    }
-    return;
-  }
-
   getUIDsFromImageID(imageId) {
     if (imageId.startsWith('wadors:')) {
       const strippedImageId = imageId.split('/studies/')[1];
@@ -467,23 +453,26 @@ class MetadataProvider {
       };
     } else if (imageId.includes('?requestType=WADO')) {
       const qs = queryString.parse(imageId);
+      const frameNumber = qs.frameNumber || qs.frame;
 
       return {
         StudyInstanceUID: qs.studyUID,
         SeriesInstanceUID: qs.seriesUID,
         SOPInstanceUID: qs.objectUID,
-        frameNumber: qs.frameNumber,
+        frameNumber,
       };
     }
 
     const imageURI = baseImageURIForMetadata(imageId);
     const uids = this.imageURIToUIDs.get(imageURI);
-    const frameNumber = this.getFrameInformationFromURL(imageId) || '1';
 
-    if (uids && frameNumber !== undefined) {
+    if (!uids) {
-      return { ...uids, frameNumber };
+      return;
     }
-    return uids;
+
+    const frameNumber = getUriModule(imageId)?.framesString || uids.frameNumber || '1';
+
+    return { ...uids, frameNumber };
   }
 }
 

tests/MultiframeRendering.spec.ts

@@ -0,0 +1,103 @@
+import {
+  test,
+  expect,
+  visitStudy,
+  getViewportCanvasStats,
+  waitForViewportsRendered,
+  waitForViewportRenderCycle,
+} from './utils';
+
+// Ultrasound cine study: one series, nine instances, all but one multiframe.
+// The first display set is the InstanceNumber=1 instance with 94 frames.
+const usMultiframeStudyUID = '1.2.840.113663.1500.1.248223208.1.1.20110323.105903.687';
+
+// NM brain SPECT study: one series of five multiframe instances whose per-frame
+// positions come from DetectorInformationSequence rather than per-frame groups.
+const nmMultiframeStudyUID = '1.2.276.0.7230010.3.1.2.447481088.1.1669202398.851612';
+
+// A rendered frame must have some visible content; cine/SPECT frames are mostly
+// dark background, so the bar is intentionally low. A blank or black viewport
+// scores 0.
+const minimumNonBlackRatio = 0.01;
+
+test.describe('Multiframe ultrasound rendering', () => {
+  test.beforeEach(async ({ page }) => {
+    await visitStudy(page, usMultiframeStudyUID, 'viewer', 2000);
+    await waitForViewportsRendered(page);
+  });
+
+  test('should render a multiframe instance as a stack of all its frames', async ({
+    page,
+    viewportPageObject,
+  }) => {
+    const activeViewport = await viewportPageObject.active;
+
+    // All 94 frames of the instance must be enumerated as the stack size.
+    await expect(activeViewport.overlayText.bottomRight.instanceNumber).toContainText('(1/94)', {
+      timeout: 10000,
+    });
+
+    // The first frame must actually be painted, not a blank/black canvas.
+    const stats = await getViewportCanvasStats({ page });
+    expect(stats.nonBlackRatio).toBeGreaterThan(minimumNonBlackRatio);
+  });
+
+  test('should render distinct content when navigating through frames', async ({
+    page,
+    viewportPageObject,
+  }) => {
+    const activeViewport = await viewportPageObject.active;
+    await expect(activeViewport.overlayText.bottomRight.instanceNumber).toContainText('(1/94)', {
+      timeout: 10000,
+    });
+    const firstFrameStats = await getViewportCanvasStats({ page });
+    expect(firstFrameStats.nonBlackRatio).toBeGreaterThan(minimumNonBlackRatio);
+
+    // Advance one frame: the overlay must track the frame index and the canvas
+    // must not go black (a past regression rendered black from the 2nd frame on).
+    let renderCycle = waitForViewportRenderCycle(page);
+    await activeViewport.sliceNavigation.scrollBy(1);
+    await renderCycle;
+
+    await expect(activeViewport.overlayText.bottomRight.instanceNumber).toContainText('(2/94)');
+    const secondFrameStats = await getViewportCanvasStats({ page });
+    expect(secondFrameStats.nonBlackRatio).toBeGreaterThan(minimumNonBlackRatio);
+
+    // Jump to the last frame: the canvas content must differ from frame 1,
+    // which fails if the stack is stuck rendering the first frame.
+    renderCycle = waitForViewportRenderCycle(page);
+    await activeViewport.sliceNavigation.toLastSlice();
+    await renderCycle;
+
+    await expect(activeViewport.overlayText.bottomRight.instanceNumber).toContainText('(94/94)');
+    const lastFrameStats = await getViewportCanvasStats({ page });
+    expect(lastFrameStats.nonBlackRatio).toBeGreaterThan(minimumNonBlackRatio);
+    expect(lastFrameStats.digest).not.toBe(firstFrameStats.digest);
+  });
+});
+
+test.describe('Multiframe NM rendering', () => {
+  test.beforeEach(async ({ page }) => {
+    await visitStudy(page, nmMultiframeStudyUID, 'viewer', 2000);
+    await waitForViewportsRendered(page);
+  });
+
+  test('should render a multiframe NM instance with all frames enumerated', async ({
+    page,
+    viewportPageObject,
+  }) => {
+    const activeViewport = await viewportPageObject.active;
+    const instanceNumberOverlay = activeViewport.overlayText.bottomRight.instanceNumber;
+
+    await expect(instanceNumberOverlay).toContainText('(1/', { timeout: 10000 });
+
+    // The stack size shown by the overlay must be the instance's frame count,
+    // not 1 (which is what a broken multiframe split would produce).
+    const overlayText = await instanceNumberOverlay.textContent();
+    const stackSize = Number(overlayText.match(/\(1\/(\d+)\)/)?.[1]);
+    expect(stackSize).toBeGreaterThan(1);
+
+    const stats = await getViewportCanvasStats({ page });
+    expect(stats.nonBlackRatio).toBeGreaterThan(minimumNonBlackRatio);
+  });
+});

tests/utils/getViewportCanvasStats.ts

@@ -0,0 +1,77 @@
+import { Page } from '@playwright/test';
+
+export type ViewportCanvasStats = {
+  width: number;
+  height: number;
+  /** Fraction of sampled pixels whose luminance is above the black cutoff (0-1). */
+  nonBlackRatio: number;
+  /** Order-sensitive digest of the sampled pixels; differs when the rendered frame differs. */
+  digest: number;
+};
+
+type GetViewportCanvasStatsParams = {
+  page: Page;
+  /** Cornerstone viewport id; the single-viewport default layout uses 'default'. */
+  viewportId?: string;
+};
+
+/**
+ * Reads the pixels currently painted on a viewport's on-screen canvas and
+ * returns aggregate stats. This asserts real rendered output (unlike a
+ * services-state read) without a screenshot baseline: `nonBlackRatio` catches
+ * blank/black viewports and `digest` catches a viewport stuck on a previous
+ * frame after navigation.
+ */
+export const getViewportCanvasStats = async ({
+  page,
+  viewportId = 'default',
+}: GetViewportCanvasStatsParams): Promise<ViewportCanvasStats> => {
+  return page.evaluate(
+    ({ services, viewportId }: withTestTypes<{ viewportId: string }>) => {
+      const { cornerstoneViewportService } = services;
+      const viewport = cornerstoneViewportService.getCornerstoneViewport(viewportId) as any;
+      if (!viewport) {
+        throw new Error(`getViewportCanvasStats: no cornerstone viewport with id "${viewportId}"`);
+      }
+
+      const sourceCanvas = viewport.getCanvas() as HTMLCanvasElement;
+      const { width, height } = sourceCanvas;
+      if (!width || !height) {
+        throw new Error('getViewportCanvasStats: viewport canvas has zero size');
+      }
+
+      // Copy onto a plain 2d canvas so pixels are readable regardless of the
+      // source canvas context type.
+      const copy = document.createElement('canvas');
+      copy.width = width;
+      copy.height = height;
+      const ctx = copy.getContext('2d');
+      ctx.drawImage(sourceCanvas, 0, 0);
+      const { data } = ctx.getImageData(0, 0, width, height);
+
+      const blackCutoff = 16; // 0-255 luminance below this counts as black
+      const sampleStride = 4; // sample every 4th pixel to keep this fast
+      let sampled = 0;
+      let nonBlack = 0;
+      let digest = 0;
+
+      for (let i = 0; i < data.length; i += 4 * sampleStride) {
+        const luminance = 0.2126 * data[i] + 0.7152 * data[i + 1] + 0.0722 * data[i + 2];
+        sampled += 1;
+        if (luminance > blackCutoff) {
+          nonBlack += 1;
+        }
+        // FNV-style rolling hash over the sampled luminance values
+        digest = (Math.imul(digest ^ Math.round(luminance), 16777619) >>> 0) as number;
+      }
+
+      return {
+        width,
+        height,
+        nonBlackRatio: sampled ? nonBlack / sampled : 0,
+        digest,
+      };
+    },
+    { viewportId, services: await page.evaluateHandle('window.services') }
+  );
+};

tests/utils/index.ts

@@ -25,6 +25,7 @@ import { scrollVolumeViewport } from './scrollVolumeViewport';
 import { attemptAction } from './attemptAction';
 import { addLengthMeasurement } from './addLengthMeasurement';
 import { getSvgAttribute } from './getSvgAttribute';
+import { getViewportCanvasStats } from './getViewportCanvasStats';
 import { navigateWithViewportArrow } from './navigateWithViewportArrow';
 import { contourShowOnlyNthSegment } from './contourShowOnlyNthSegment';
 import { visitStudyAndHydrate } from './visitStudyAndHydrate';
@@ -62,6 +63,7 @@ export {
   addLengthMeasurement,
   subscribeToMeasurementAdded,
   getSvgAttribute,
+  getViewportCanvasStats,
   navigateWithViewportArrow,
   contourShowOnlyNthSegment,
   visitStudyAndHydrate,