diff --git a/extensions/cornerstone/src/commandsModule.ts b/extensions/cornerstone/src/commandsModule.ts index 3a45bda00..a5b9e9627 100644 --- a/extensions/cornerstone/src/commandsModule.ts +++ b/extensions/cornerstone/src/commandsModule.ts @@ -2096,6 +2096,15 @@ function commandsModule({ displaySetInstanceUIDs, }); + if (!updatedViewports?.length) { + return; + } + + updatedViewports.forEach(({ viewportId: csViewportId }) => { + const csViewport = cornerstoneViewportService.getCornerstoneViewport(csViewportId); + csViewport?.setNeedsRender?.(); + }); + actions.setDisplaySetsForViewports({ viewportsToUpdate: updatedViewports.map(viewport => ({ viewportId: viewport.viewportId, diff --git a/extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts b/extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts index f896ed0a4..b2ad4b7cd 100644 --- a/extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts +++ b/extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts @@ -49,6 +49,47 @@ const EVENTS = { const MIN_STACK_VIEWPORTS_TO_ENQUEUE_RESIZE = 12; const MIN_VOLUME_VIEWPORTS_TO_ENQUEUE_RESIZE = 6; +function getVolumeActorReferencedIds(viewport: Types.IVolumeViewport): string[] { + const actors = viewport.getActors?.() ?? []; + return actors + .filter(ac => ac.actor?.getClassName?.() === 'vtkVolume') + .map(ac => ac.referencedId) + .filter(Boolean) as string[]; +} + +function volumeIdPrefixesMatch( + existingIds: string[], + prefixLen: number, + targetIds: string[] +): boolean { + if (prefixLen > targetIds.length) { + return false; + } + for (let i = 0; i < prefixLen; i++) { + if (existingIds[i] !== targetIds[i]) { + return false; + } + } + return true; +} + +/** + * Returns true when the viewport type matches a volume-based presentation (ORTHOGRAPHIC or VOLUME_3D). + */ +function viewportMatchesDesiredVolumePresentation( + viewport: Types.IViewport, + desiredViewportInfo: ViewportInfo +): boolean { + const desiredType = desiredViewportInfo.getViewportType(); + if (viewport.type !== desiredType) { + return false; + } + return ( + desiredType === csEnums.ViewportType.ORTHOGRAPHIC || + desiredType === csEnums.ViewportType.VOLUME_3D + ); +} + export const WITH_NAVIGATION = { withNavigation: true, withOrientation: false }; export const WITH_ORIENTATION = { withNavigation: true, withOrientation: true }; @@ -1112,7 +1153,44 @@ class CornerstoneViewportService extends PubSubService implements IViewportServi }); } - await viewport.setVolumes(volumeInputArray); + const baseVolumeInputs = filteredVolumeInputArray.map(({ volumeInput }) => volumeInput); + const nextBaseVolumeIds = baseVolumeInputs.map(v => v.volumeId); + const existingVolumeIds = getVolumeActorReferencedIds(viewport); + + let skippedIdenticalBaseVolumes = false; + + if (baseVolumeInputs.length) { + const singleBaseViewport = nextBaseVolumeIds.length === 1; + + // Only skip setVolumes() when the viewport type already matches the desired OHIF type + // (ORTHOGRAPHIC / VOLUME_3D); otherwise a stack → MPR switch with the same volumeId + // would incorrectly skip rebuilding the viewport. + if ( + singleBaseViewport && + existingVolumeIds.length >= nextBaseVolumeIds.length && + volumeIdPrefixesMatch(existingVolumeIds, nextBaseVolumeIds.length, nextBaseVolumeIds) && + viewportMatchesDesiredVolumePresentation(viewport, viewportInfo) + ) { + // Same primary volume already loaded (e.g. labelmap / extra actors after it) — avoid + // setVolumes(), which tears down all actors and blanks MPR during SEG hydrate. + skippedIdenticalBaseVolumes = true; + } else if ( + existingVolumeIds.length && + nextBaseVolumeIds.length > existingVolumeIds.length && + volumeIdPrefixesMatch(existingVolumeIds, existingVolumeIds.length, nextBaseVolumeIds) && + typeof viewport.addVolumes === 'function' + ) { + const toAdd = baseVolumeInputs.slice(existingVolumeIds.length); + if (toAdd.length) { + await viewport.addVolumes(toAdd); + } + } else { + await viewport.setVolumes(baseVolumeInputs); + } + } else if (volumeInputArray.length) { + await viewport.setVolumes(volumeInputArray); + } + await this._addOverlayRepresentations(overlayProcessingResults); viewport.render(); @@ -1124,8 +1202,11 @@ class CornerstoneViewportService extends PubSubService implements IViewportServi }); this.setPresentations(viewport.id, presentations); + // Presentations apply segmentation (hydrated labelmap etc.) after the render above — redraw so + // every orthographic/3D tile shows the updated scene (fixes MPR siblings blank after SEG hydrate). + viewport.render(); - if (!presentations.positionPresentation) { + if (!presentations.positionPresentation && !skippedIdenticalBaseVolumes) { const imageIndex = this._getInitialImageIndexForViewport(viewportInfo); if (imageIndex !== undefined) { diff --git a/extensions/cornerstone/src/stores/useSegmentationPresentationStore.ts b/extensions/cornerstone/src/stores/useSegmentationPresentationStore.ts index 8bdbb9e06..1e81b2a9a 100644 --- a/extensions/cornerstone/src/stores/useSegmentationPresentationStore.ts +++ b/extensions/cornerstone/src/stores/useSegmentationPresentationStore.ts @@ -135,16 +135,27 @@ const _getSegmentationPresentationId = ({ return; } + const { displaySetService } = servicesManager.services; const { displaySetInstanceUIDs, viewportOptions } = viewport; + // Match keys used by updateStoredSegmentationPresentation (referenced volume only). + // Including overlay UIDs (e.g. SEG) produced ids like "MR&SEG" while the store + // entry is under "MR", so hydrated segmentations never applied and viewports could mis-render. + const nonOverlayUIDs = displaySetInstanceUIDs.filter(uid => { + const ds = displaySetService.getDisplaySetByUID(uid); + return ds && !ds.isOverlayDisplaySet; + }); + + if (!nonOverlayUIDs.length) { + return; + } + let orientation = viewportOptions.orientation; if (!orientation) { // Calculate orientation from the viewport sample image - const displaySet = servicesManager.services.displaySetService.getDisplaySetByUID( - displaySetInstanceUIDs[0] - ); - const sampleImage = displaySet.images?.[0]; + const displaySet = displaySetService.getDisplaySetByUID(nonOverlayUIDs[0]); + const sampleImage = displaySet?.images?.[0]; const imageOrientationPatient = sampleImage?.ImageOrientationPatient; orientation = getViewportOrientationFromImageOrientationPatient(imageOrientationPatient); @@ -152,7 +163,7 @@ const _getSegmentationPresentationId = ({ const segmentationPresentationArr = []; - segmentationPresentationArr.push(...displaySetInstanceUIDs); + segmentationPresentationArr.push(...nonOverlayUIDs); // Uncomment if unique indexing is needed // addUniqueIndex( diff --git a/extensions/cornerstone/src/utils/hydrationUtils.test.ts b/extensions/cornerstone/src/utils/hydrationUtils.test.ts index abf52ac19..e10acc008 100644 --- a/extensions/cornerstone/src/utils/hydrationUtils.test.ts +++ b/extensions/cornerstone/src/utils/hydrationUtils.test.ts @@ -262,6 +262,57 @@ describe('getUpdatedViewportsForSegmentation', () => { expect(result).toEqual(mockUpdatedViewports); }); + it('should merge all grid viewports that reference the volume after hydration (e.g. 3D four-up)', () => { + const volumeUid = 'volume-1'; + const viewports = new Map([ + [ + 'vp-axial', + { + viewportId: 'vp-axial', + viewportOptions: { viewportId: 'vp-axial' }, + displaySetInstanceUIDs: [volumeUid], + }, + ], + [ + 'vp-sagittal', + { + viewportId: 'vp-sagittal', + viewportOptions: { viewportId: 'vp-sagittal' }, + displaySetInstanceUIDs: [volumeUid], + }, + ], + [ + 'vp-other-study', + { + viewportId: 'vp-other-study', + viewportOptions: { viewportId: 'vp-other-study' }, + displaySetInstanceUIDs: ['other-volume'], + }, + ], + ]); + + mockViewportGridService.getState.mockReturnValue({ + isHangingProtocolLayout: true, + viewports, + activeViewportId: 'vp-axial', + }); + + mockHangingProtocolService.getViewportsRequireUpdate.mockReturnValue([ + { viewportId: 'vp-axial', displaySetInstanceUIDs: [volumeUid] }, + ]); + + const result = getUpdatedViewportsForSegmentation({ + viewportId: 'vp-axial', + servicesManager: mockServicesManager as unknown as AppTypes.ServicesManager, + displaySetInstanceUIDs: [volumeUid], + }); + + expect(result).toEqual([ + { viewportId: 'vp-axial', displaySetInstanceUIDs: [volumeUid] }, + { viewportId: 'vp-sagittal', displaySetInstanceUIDs: [volumeUid] }, + ]); + }); + it('should handle complex viewport structure', () => { const complexViewport = { viewportOptions: { diff --git a/extensions/cornerstone/src/utils/hydrationUtils.ts b/extensions/cornerstone/src/utils/hydrationUtils.ts index c1a9715b2..3f6636404 100644 --- a/extensions/cornerstone/src/utils/hydrationUtils.ts +++ b/extensions/cornerstone/src/utils/hydrationUtils.ts @@ -1,3 +1,63 @@ +import type { Types } from '@ohif/core'; + +/** + * After SEG hydration we must refresh every viewport that shows the referenced volume so + * presentations (including segmentation) apply to all MPR/3D tiles. Hanging-protocol matching + * can return only the active viewport when protocol definitions omit viewportId (e.g. 3D four-up) + * or when layout state diverges from the protocol; this merges in all grid panes that already + * list that volume in `displaySetInstanceUIDs`. + * + * Only exact displaySetInstanceUID matches are used (no Frame-of-Reference inference): sibling + * MPR planes must already share the same referenced volume UID in grid state, or HP matching + * must list them; otherwise forcing a different UID onto a volume viewport can leave it blank. + */ +function mergeVolumeSharingViewports( + hangingProtocolUpdates: Types.HangingProtocol.ViewportUpdate[] | null | undefined, + volumeUid: string | undefined, + viewports: AppTypes.ViewportGrid.GridViewports +): Types.HangingProtocol.ViewportUpdate[] { + if (!volumeUid) { + return (hangingProtocolUpdates ?? []) as Types.HangingProtocol.ViewportUpdate[]; + } + + const byId = new Map(); + + const add = (viewportId: string, uids: string[]) => { + if (!viewportId) { + return; + } + byId.set(viewportId, { viewportId, displaySetInstanceUIDs: uids }); + }; + + if (Array.isArray(hangingProtocolUpdates)) { + for (const entry of hangingProtocolUpdates) { + const vid = entry.viewportId ?? entry.viewportOptions?.viewportId; + if (vid) { + add(vid, entry.displaySetInstanceUIDs?.length ? entry.displaySetInstanceUIDs : [volumeUid]); + } + } + } + + viewports.forEach(vp => { + const uids = vp.displaySetInstanceUIDs || []; + if (uids.includes(volumeUid)) { + add(vp.viewportId, [volumeUid]); + } + }); + + const merged = Array.from(byId.values()); + + if ( + merged.length === 0 && + Array.isArray(hangingProtocolUpdates) && + hangingProtocolUpdates.length > 0 + ) { + return hangingProtocolUpdates; + } + + return merged; +} + function getUpdatedViewportsForSegmentation({ viewportId, servicesManager, @@ -5,7 +65,7 @@ function getUpdatedViewportsForSegmentation({ }: withAppTypes) { const { hangingProtocolService, viewportGridService } = servicesManager.services; - const { isHangingProtocolLayout } = viewportGridService.getState(); + const { isHangingProtocolLayout, viewports } = viewportGridService.getState(); const viewport = getTargetViewport({ viewportId, viewportGridService }); const targetViewportId = viewport.viewportOptions.viewportId; @@ -16,7 +76,11 @@ function getUpdatedViewportsForSegmentation({ isHangingProtocolLayout ); - return updatedViewports; + if (updatedViewports == null) { + return updatedViewports; + } + + return mergeVolumeSharingViewports(updatedViewports, displaySetInstanceUIDs[0], viewports); } const getTargetViewport = ({ viewportId, viewportGridService }) => { diff --git a/extensions/default/src/utils/layerConfigurationUtils.ts b/extensions/default/src/utils/layerConfigurationUtils.ts index 44bad77e9..4d8dda488 100644 --- a/extensions/default/src/utils/layerConfigurationUtils.ts +++ b/extensions/default/src/utils/layerConfigurationUtils.ts @@ -67,8 +67,14 @@ export function configureViewportForLayerAddition(params: { // If a viewport type was already set do not reset it. if (!viewport.viewportOptions.viewportType) { - // Special handling for overlay display sets - if (requestedLayerDisplaySet.isOverlayDisplaySet) { + // If the live Cornerstone viewport is already orthographic (volume), keep it as + // 'volume'. Prevents accidentally downgrading a volume viewport to 'stack' when + // viewportOptions arrives without a viewportType (e.g. from the HP overlay path). + const csViewport = cornerstoneViewportService.getCornerstoneViewport(viewportId); + if (csViewport?.type === 'orthographic') { + viewport.viewportOptions.viewportType = 'volume'; + // Special handling for overlay display sets + } else if (requestedLayerDisplaySet.isOverlayDisplaySet) { // Do not force volume for SEG and RTSTRUCT if it and all the current display sets are for the same display set const isSameDisplaySet = currentDisplaySetUIDs.every(uid => { const currentDisplaySet = displaySetService.getDisplaySetByUID(uid); @@ -131,7 +137,9 @@ export function configureViewportForLayerRemoval(params: { viewport.viewportOptions = {}; } - viewport.viewportOptions.viewportType = 'volume'; + const csViewportOrthographic = + cornerstoneViewportService.getCornerstoneViewport(viewportId)?.type === 'orthographic'; + viewport.viewportOptions.viewportType = csViewportOrthographic ? 'volume' : 'stack'; // orientation if (!viewport.viewportOptions.orientation) { diff --git a/platform/core/src/services/HangingProtocolService/HangingProtocolService.ts b/platform/core/src/services/HangingProtocolService/HangingProtocolService.ts index 67de64e57..9ee83a558 100644 --- a/platform/core/src/services/HangingProtocolService/HangingProtocolService.ts +++ b/platform/core/src/services/HangingProtocolService/HangingProtocolService.ts @@ -661,6 +661,12 @@ export default class HangingProtocolService extends PubSubService { if (displaySet?.unsupported) { throw new Error('Unsupported displaySet'); } + // Overlays (SEG, RTSTRUCT) share selectors with referenced volumes but must not + // sync to every matched viewport before hydration (would replace e.g. 3D primary). + if (displaySet?.isOverlayDisplaySet) { + return defaultReturn; + } + const protocol = this.protocol; const protocolStage = protocol.stages[this.stageIndex]; const protocolViewports = protocolStage.viewports; diff --git a/platform/core/src/types/HangingProtocol.ts b/platform/core/src/types/HangingProtocol.ts index 189fe0dcc..6b7be084c 100644 --- a/platform/core/src/types/HangingProtocol.ts +++ b/platform/core/src/types/HangingProtocol.ts @@ -24,6 +24,13 @@ export type DisplaySetAndViewportOptions = { displaySetOptions: DisplaySetOptions; }; +export interface ViewportUpdate { + viewportId?: string; + displaySetInstanceUIDs: string[]; + viewportOptions?: ViewportOptions; + displaySetOptions?: DisplaySetOptions[]; +} + export type DisplayArea = { type?: 'SCALE' | 'FIT'; scale?: number; diff --git a/tests/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts b/tests/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts index 81852a5d5..a1fa88d41 100644 --- a/tests/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts +++ b/tests/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts @@ -1,4 +1,10 @@ -import { checkForScreenshot, screenShotPaths, test, visitStudy } from './utils'; +import { + checkForScreenshot, + screenShotPaths, + test, + visitStudy, + waitForViewportRenderCycle, +} from './utils'; import { press } from './utils/keyboardUtils'; import { assertNumberOfModalityLoadBadges } from './utils/assertions'; @@ -15,9 +21,13 @@ test('should overlay an unhydrated SEG over a display set that the SEG does NOT }) => { await leftPanelPageObject.loadSeriesByDescription('Apparent Diffusion Coefficient'); - const dataOverlayPageObject = (await viewportPageObject.getById('default')).overlayMenu.dataOverlay; + const dataOverlayPageObject = (await viewportPageObject.getById('default')).overlayMenu + .dataOverlay; await dataOverlayPageObject.toggle(); + // Start watching for viewport to render + const viewportRenderCycle = waitForViewportRenderCycle(page); + await dataOverlayPageObject.addSegmentation('T2 Weighted Axial Segmentations'); // Adding an overlay should not show the LOAD button. @@ -25,22 +35,21 @@ test('should overlay an unhydrated SEG over a display set that the SEG does NOT // Hide the overlay menu. await dataOverlayPageObject.toggle(); - await page.waitForTimeout(5000); + + await viewportRenderCycle; await checkForScreenshot( page, - page, + viewportPageObject.grid, screenShotPaths.segDataOverlayForUnreferencedDisplaySetNoHydration.overlayFirstImage ); // Navigate to the middle image of the default viewport. await press({ page, key: 'ArrowDown', nTimes: 12 }); - await page.waitForTimeout(5000); - await checkForScreenshot( page, - page, + viewportPageObject.grid, screenShotPaths.segDataOverlayForUnreferencedDisplaySetNoHydration.overlayMiddleImage ); }); diff --git a/tests/SEGHydrationFrom3DFourUp.spec.ts b/tests/SEGHydrationFrom3DFourUp.spec.ts index 83dc03d15..51db1d1af 100644 --- a/tests/SEGHydrationFrom3DFourUp.spec.ts +++ b/tests/SEGHydrationFrom3DFourUp.spec.ts @@ -5,6 +5,8 @@ import { screenShotPaths, test, visitStudy, + waitForViewportsRendered, + waitForViewportRenderCycle, } from './utils'; test.beforeEach(async ({ page }) => { @@ -19,35 +21,45 @@ test.describe('3D four up SEG hydration', async () => { DOMOverlayPageObject, leftPanelPageObject, mainToolbarPageObject, + viewportPageObject, }) => { await mainToolbarPageObject.layoutSelection.threeDFourUp.click(); await attemptAction(() => reduce3DViewportSize(page), 10, 100); - await page.waitForTimeout(5000); + await waitForViewportsRendered(page); await checkForScreenshot( page, - page, + viewportPageObject.grid, screenShotPaths.segHydrationFrom3DFourUp.threeDFourUpBeforeSEG ); await leftPanelPageObject.loadSeriesByDescription('SEG'); - await page.waitForTimeout(5000); + await waitForViewportsRendered(page); + await checkForScreenshot( page, - page, + viewportPageObject.grid, screenShotPaths.segHydrationFrom3DFourUp.threeDFourUpAfterSEG ); + // start watching for viewports to render + + // High rendered timeout needed: layout has 4 viewports (3D volume + MPR planes + SEG overlays), + // which can take significantly longer time to fully render + const viewportRenderCycle = waitForViewportRenderCycle(page, { renderedTimeout: 180000 }); + await DOMOverlayPageObject.viewport.segmentationHydration.yes.click(); - await page.waitForTimeout(5000); - await checkForScreenshot( + // Wait until all viewports have finished rendering + await viewportRenderCycle; + + await checkForScreenshot({ page, - page, - screenShotPaths.segHydrationFrom3DFourUp.threeDFourUpAfterSegHydrated - ); + locator: viewportPageObject.grid, + screenshotPath: screenShotPaths.segHydrationFrom3DFourUp.threeDFourUpAfterSegHydrated, + }); }); }); diff --git a/tests/SEGHydrationFromMPR.spec.ts b/tests/SEGHydrationFromMPR.spec.ts index bf621978f..7b40bfa4c 100644 --- a/tests/SEGHydrationFromMPR.spec.ts +++ b/tests/SEGHydrationFromMPR.spec.ts @@ -1,4 +1,11 @@ -import { checkForScreenshot, screenShotPaths, test, visitStudy } from './utils'; +import { + checkForScreenshot, + screenShotPaths, + test, + visitStudy, + waitForViewportRenderCycle, + waitForViewportsRendered, +} from './utils'; test.beforeEach(async ({ page }) => { const studyInstanceUID = '1.3.12.2.1107.5.2.32.35162.30000015050317233592200000046'; @@ -12,30 +19,52 @@ test('should properly display MPR for MR', async ({ leftPanelPageObject, mainToolbarPageObject, rightPanelPageObject, + viewportPageObject, }) => { await rightPanelPageObject.toggle(); await mainToolbarPageObject.layoutSelection.MPR.click(); - await page.waitForTimeout(5000); - await checkForScreenshot(page, page, screenShotPaths.segHydrationFromMPR.mprBeforeSEG); + await waitForViewportsRendered(page); + + await checkForScreenshot( + page, + viewportPageObject.grid, + screenShotPaths.segHydrationFromMPR.mprBeforeSEG + ); await leftPanelPageObject.loadSeriesByDescription('SEG'); - await page.waitForTimeout(5000); - await checkForScreenshot(page, page, screenShotPaths.segHydrationFromMPR.mprAfterSEG); + await waitForViewportsRendered(page); + + await checkForScreenshot( + page, + viewportPageObject.grid, + screenShotPaths.segHydrationFromMPR.mprAfterSEG + ); + + // start watching for viewports to render + const viewportRenderCycle = waitForViewportRenderCycle(page); await DOMOverlayPageObject.viewport.segmentationHydration.yes.click(); - await page.waitForTimeout(5000); - await checkForScreenshot(page, page, screenShotPaths.segHydrationFromMPR.mprAfterSegHydrated); + await viewportRenderCycle; + + await checkForScreenshot( + page, + viewportPageObject.grid, + screenShotPaths.segHydrationFromMPR.mprAfterSegHydrated + ); + + const viewportRenderAfterLayoutChange = waitForViewportRenderCycle(page); await mainToolbarPageObject.layoutSelection.axialPrimary.click(); - await page.waitForTimeout(5000); + await viewportRenderAfterLayoutChange; + await checkForScreenshot( page, - page, + viewportPageObject.grid, screenShotPaths.segHydrationFromMPR.mprAfterSegHydratedAfterLayoutChange ); }); diff --git a/tests/screenshots/chromium/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts/overlayFirstImage.png b/tests/screenshots/chromium/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts/overlayFirstImage.png index 8676f8e42..e4fd6ea00 100644 Binary files a/tests/screenshots/chromium/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts/overlayFirstImage.png and b/tests/screenshots/chromium/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts/overlayFirstImage.png differ diff --git a/tests/screenshots/chromium/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts/overlayMiddleImage.png b/tests/screenshots/chromium/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts/overlayMiddleImage.png index 8b80c0f46..1bdbc77fb 100644 Binary files a/tests/screenshots/chromium/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts/overlayMiddleImage.png and b/tests/screenshots/chromium/SEGDataOverlayForUnreferencedDisplaySetNoHydration.spec.ts/overlayMiddleImage.png differ diff --git a/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpAfterSEG.png b/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpAfterSEG.png index 8ee6bb43f..5d39dd116 100644 Binary files a/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpAfterSEG.png and b/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpAfterSEG.png differ diff --git a/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpAfterSegHydrated.png b/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpAfterSegHydrated.png index 6d4b47051..2b0f2c248 100644 Binary files a/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpAfterSegHydrated.png and b/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpAfterSegHydrated.png differ diff --git a/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpBeforeSEG.png b/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpBeforeSEG.png index 6d42c451f..e1ec19403 100644 Binary files a/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpBeforeSEG.png and b/tests/screenshots/chromium/SEGHydrationFrom3DFourUp.spec.ts/threeDFourUpBeforeSEG.png differ diff --git a/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSEG.png b/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSEG.png index b7e71eae8..1cbd07b2d 100644 Binary files a/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSEG.png and b/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSEG.png differ diff --git a/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSegHydrated.png b/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSegHydrated.png index 50d3ff801..03e0e1d0a 100644 Binary files a/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSegHydrated.png and b/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSegHydrated.png differ diff --git a/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSegHydratedAfterLayoutChange.png b/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSegHydratedAfterLayoutChange.png index 6441b28c5..64db9256c 100644 Binary files a/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSegHydratedAfterLayoutChange.png and b/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprAfterSegHydratedAfterLayoutChange.png differ diff --git a/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprBeforeSEG.png b/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprBeforeSEG.png index c3cf748f9..9539fe7a9 100644 Binary files a/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprBeforeSEG.png and b/tests/screenshots/chromium/SEGHydrationFromMPR.spec.ts/mprBeforeSEG.png differ diff --git a/tests/utils/checkForScreenshot.ts b/tests/utils/checkForScreenshot.ts index 9faa9428d..ea5f9ab53 100644 --- a/tests/utils/checkForScreenshot.ts +++ b/tests/utils/checkForScreenshot.ts @@ -1,5 +1,7 @@ -import { expect } from 'playwright-test-coverage'; +import { expect, test } from 'playwright-test-coverage'; import { Locator, Page } from 'playwright'; +import { promises as fs } from 'fs'; +import path from 'path'; type CheckForScreenshotProps = { page: Page; @@ -18,12 +20,69 @@ type CheckForScreenshotProps = { fullPage?: boolean; }; +const _isIntermediateScreenshotArtifact = (filename: string, screenshotPath: string) => { + const { name } = path.parse(screenshotPath); + const lowerFilename = filename.toLowerCase(); + + if (!lowerFilename.endsWith('.png')) { + return false; + } + + return ( + lowerFilename.startsWith(`${name.toLowerCase()}-`) && + (lowerFilename.endsWith('-actual.png') || + lowerFilename.endsWith('-diff.png') || + lowerFilename.endsWith('-expected.png')) + ); +}; + +const _cleanupIntermediateScreenshotArtifacts = async ( + outputDir: string, + screenshotPath: string +) => { + const stack = [outputDir]; + + while (stack.length) { + const currentDir = stack.pop(); + + if (!currentDir) { + continue; + } + + let entries; + try { + entries = await fs.readdir(currentDir, { withFileTypes: true }); + } catch { + continue; + } + + for (const entry of entries) { + const fullPath = path.join(currentDir, entry.name); + + if (entry.isDirectory()) { + stack.push(fullPath); + continue; + } + + if (!_isIntermediateScreenshotArtifact(entry.name, screenshotPath)) { + continue; + } + + try { + await fs.unlink(fullPath); + } catch { + // Best-effort cleanup only. + } + } + } +}; + const _checkForScreenshot = async (props: CheckForScreenshotProps) => { const { page, screenshotPath, attempts = 10, - delay = 500, + delay = 1250, maxDiffPixelRatio = 0.02, threshold = 0.05, normalizedClip, @@ -31,6 +90,7 @@ const _checkForScreenshot = async (props: CheckForScreenshotProps) => { } = props; let { locator = page } = props; + const testOutputDir = test.info().outputDir; await page.waitForLoadState('networkidle'); @@ -70,6 +130,7 @@ const _checkForScreenshot = async (props: CheckForScreenshotProps) => { console.debug('Screenshot comparison failed after all attempts'); throw error; // Throw the original error with details instead of a generic message } + await _cleanupIntermediateScreenshotArtifacts(testOutputDir, screenshotPath); await new Promise(resolve => setTimeout(resolve, delay)); } } diff --git a/tests/utils/index.ts b/tests/utils/index.ts index c15967fa8..fe37bcb32 100644 --- a/tests/utils/index.ts +++ b/tests/utils/index.ts @@ -24,6 +24,11 @@ import { getSvgPath } from './getSvgPath'; import { navigateWithViewportArrow } from './navigateWithViewportArrow'; import { test, expect } from './fixture'; import { subscribeToMeasurementAdded } from './subscribeToMeasurement'; +import { + waitForAnyViewportNeedsRender, + waitForViewportsRendered, + waitForViewportRenderCycle, +} from './waitForViewportsRendered'; export { visitStudy, @@ -48,6 +53,9 @@ export { subscribeToMeasurementAdded, getSvgPath, navigateWithViewportArrow, + waitForAnyViewportNeedsRender, + waitForViewportsRendered, + waitForViewportRenderCycle, test, expect, }; diff --git a/tests/utils/waitForViewportsRendered.ts b/tests/utils/waitForViewportsRendered.ts new file mode 100644 index 000000000..261a0e43a --- /dev/null +++ b/tests/utils/waitForViewportsRendered.ts @@ -0,0 +1,163 @@ +import type { Page } from '@playwright/test'; + +type WaitForAnyViewportNeedsRenderOptions = { + timeout?: number; +}; + +type WaitForViewportsRenderedOptions = { + timeout?: number; + /** + * If true (default), also waits for any volume actors referenced by the + * viewports to report loaded. + */ + waitVolumeLoad?: boolean; +}; + +type WaitForRenderCycleToCompleteOptions = { + /** + * Timeout for waiting until at least one viewport reaches `needsRender`. + */ + needsRenderTimeout?: number; + /** + * Timeout for waiting until all viewports are `rendered` + * (and optionally volume-loaded). + */ + renderedTimeout?: number; + /** + * If true (default), also waits for any volume actors referenced by the + * viewports to report loaded during the rendered phase. + */ + waitVolumeLoad?: boolean; +}; + +/** + * Waits for a full render cycle: + * 1) any viewport requests render (`needsRender`) + * 2) all viewports finish rendering (`rendered`) + */ +const waitForViewportRenderCycle = async ( + page: Page, + options: WaitForRenderCycleToCompleteOptions = {} +) => { + const { needsRenderTimeout = 5000, renderedTimeout = 15000, waitVolumeLoad = true } = options; + await waitForAnyViewportNeedsRender(page, { timeout: needsRenderTimeout }); + await waitForViewportsRendered(page, { timeout: renderedTimeout, waitVolumeLoad }); +}; + +/** + * Waits until at least one viewport enters the 'needsRender' state, indicating + * that a render has been requested but not yet started. + */ +const waitForAnyViewportNeedsRender = async ( + page: Page, + options: WaitForAnyViewportNeedsRenderOptions = {} +) => { + const { timeout = 5000 } = options; + await page.waitForFunction( + () => { + const cornerstone = (window as any).cornerstone; + if (!cornerstone?.getRenderingEngines) { + return false; + } + + const renderingEngines = cornerstone.getRenderingEngines(); + const viewports = renderingEngines.flatMap(engine => + engine.getViewports ? engine.getViewports() : [] + ); + + if (!viewports.length) { + return false; + } + + const needsRender = viewports.some(viewport => viewport?.viewportStatus === 'needsRender'); + + return needsRender; + }, + {}, + { timeout } + ); +}; + +/** + * Stabilize tests by waiting for a short tick, network idle, then viewport render completion. + * To use this method safely, you may need to make changes to OHIF and/or CS3D + * methods handling clicks (SHOULD be commands modules only). These should set the + * state to needs render synchronously so that this method can safely wait for the render to complete. + * Examples such as changing the hanging protocol currently don't set such a state + * and thus can't be rendered without a delay. + * + * If options.waitVolumeLoad is not false, then this method will wait for all volumes + * associated with viewports to be loaded. + */ +const waitForViewportsRendered = async ( + page: Page, + options: WaitForViewportsRenderedOptions = {} +) => { + const { timeout = 15000, waitVolumeLoad = true } = options; + + await page.waitForFunction( + ({ waitVolumeLoad }) => { + const cornerstone = (window as any).cornerstone; + if (!cornerstone?.getRenderingEngines) { + return false; + } + + const renderingEngines = cornerstone.getRenderingEngines(); + const viewports = renderingEngines.flatMap(engine => + engine.getViewports ? engine.getViewports() : [] + ); + + if (!viewports.length) { + return false; + } + + const allRendered = viewports.every(viewport => viewport?.viewportStatus === 'rendered'); + + if (!allRendered) { + return false; + } + + if (!waitVolumeLoad) { + return true; + } + + const cache = cornerstone.cache; + if (!cache?.getVolume) { + return true; + } + + const actorEntries = viewports.flatMap(viewport => + viewport?.getActors ? viewport.getActors() : [] + ); + + for (const actorEntry of actorEntries) { + const id = actorEntry?.referencedId || actorEntry?.uid; + if (!id) { + continue; + } + + let volume: any; + try { + volume = cache.getVolume(id); + } catch { + continue; + } + + const loaded = + volume?.loadStatus && typeof volume.loadStatus.loaded === 'boolean' + ? volume.loadStatus.loaded + : true; + + if (!loaded) { + return false; + } + } + + return true; + }, + { waitVolumeLoad }, + { timeout } + ); +}; + +export { waitForViewportsRendered, waitForAnyViewportNeedsRender, waitForViewportRenderCycle };