diff --git a/extensions/default/src/getSopClassHandlerModule.js b/extensions/default/src/getSopClassHandlerModule.js index 7c312d7ef..f77913d16 100644 --- a/extensions/default/src/getSopClassHandlerModule.js +++ b/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) { - const dynamicVolumeInfo = getDynamicVolumeInfo(instances); +function getDisplaySetInfo(instances, imageIds) { + 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) { diff --git a/extensions/default/src/getSopClassHandlerModule.test.js b/extensions/default/src/getSopClassHandlerModule.test.js new file mode 100644 index 000000000..bd87668f5 --- /dev/null +++ b/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); + }); +}); diff --git a/platform/core/src/classes/MetadataProvider.test.ts b/platform/core/src/classes/MetadataProvider.test.ts new file mode 100644 index 000000000..265b38ff9 --- /dev/null +++ b/platform/core/src/classes/MetadataProvider.test.ts @@ -0,0 +1,82 @@ +import metadataProvider from './MetadataProvider'; + +beforeEach(() => { + ( + metadataProvider as unknown as { + imageURIToUIDs: Map; + } + ).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', + }); + }); +}); diff --git a/platform/core/src/classes/MetadataProvider.ts b/platform/core/src/classes/MetadataProvider.ts index 57f9abff3..40f7b805c 100644 --- a/platform/core/src/classes/MetadataProvider.ts +++ b/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 = 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) { - return { ...uids, frameNumber }; + if (!uids) { + return; } - return uids; + + const frameNumber = getUriModule(imageId)?.framesString || uids.frameNumber || '1'; + + return { ...uids, frameNumber }; } } diff --git a/tests/MultiframeRendering.spec.ts b/tests/MultiframeRendering.spec.ts new file mode 100644 index 000000000..55bc868a5 --- /dev/null +++ b/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); + }); +}); diff --git a/tests/utils/getViewportCanvasStats.ts b/tests/utils/getViewportCanvasStats.ts new file mode 100644 index 000000000..7581bdaed --- /dev/null +++ b/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 => { + 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') } + ); +}; diff --git a/tests/utils/index.ts b/tests/utils/index.ts index dca3fc884..70f664a1d 100644 --- a/tests/utils/index.ts +++ b/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,