fix(seg): prevent segmentations from spreading to all viewports before hydration confirmation in 3D four-up (#5967)

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

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

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

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

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,
 };

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