fix jump to measurement #4867

sedghi · 2025-03-18T04:04:27Z

This pull request involves significant changes to the cornerstone extension, primarily focused on refactoring the codebase to improve the handling of measurement jumps and viewport management. The most important changes include the removal of deprecated methods, the addition of new utility functions, and updates to the viewport service.

Refactoring and code cleanup:

extensions/cornerstone-dicom-sr/src/init.ts: Commented out the subscription to MeasurementService.EVENTS.JUMP_TO_MEASUREMENT_LAYOUT to clean up the initialization code.
extensions/cornerstone/src/Viewport/OHIFCornerstoneViewport.tsx: Removed deprecated cache-based jump to measurement logic and replaced it with a direct subscription to MeasurementService.EVENTS.JUMP_TO_MEASUREMENT_VIEWPORT. [1] [2] [3] [4] [5]

Enhancements to viewport service:

extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts: Added a new method getViewportDisplaySets to retrieve display sets for a given viewport and updated the WITH_NAVIGATION constant to disable orientation. [1] [2] [3]

Utility functions:

extensions/cornerstone/src/utils/index.ts: Added the isReferenceViewable utility function to determine if a reference is viewable in a given viewport. [1] [2]

Viewport module updates:

extensions/cornerstone/src/index.tsx: Updated the viewport module to include the isReferenceViewable function for checking reference viewability. [1] [2]
extensions/measurement-tracking/src/getViewportModule.tsx: Integrated the isReferenceViewable function into the tracked cornerstone viewport module. [1] [2]

netlify · 2025-03-18T04:04:46Z

✅ Deploy Preview for ohif-dev canceled.

Name	Link
🔨 Latest commit	`c49aa14`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-dev/deploys/67e6b3cfe4e7a500083828e2

netlify · 2025-03-18T04:04:48Z

❌ Deploy Preview for ohif-platform-docs failed. Why did it fail? →

Name	Link
🔨 Latest commit	`c49aa14`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-docs/deploys/67e6b3cf08e2c900080db9b8

cypress · 2025-03-18T04:08:19Z

Viewers Run #5016

Run Properties: Failed #5016 • c49aa140b7: unit test

Project	`Viewers`
Branch Review	`fix/jump-to-measurements-mpr`
Run status	`Failed #5016`
Run duration	`02m 07s`
Commit	`c49aa140b7: unit test`
Committer	`sedghi`
View all properties for this run ↗︎

Test results
Failures	`1`
Flaky	`0`
Pending	`2`
Skipped	`0`
Passing	`42`
View all changes introduced in this branch ↗︎

Tests for review

cypress/integration/volume/MPR.spec.js • 1 failed test

View Output Video

Test		Artifacts
OHIF MPR > should correctly render Crosshairs for MPR		`Test Replay` `Screenshots` `Video`

…easurement handling - Added `options` parameter to `commandsModule` for better measurement context. - Implemented `isReferenceViewable` utility to determine viewport visibility based on measurement metadata. - Updated `ViewportGrid` to handle measurement events more effectively, ensuring viewports can respond to reference viewability. - Refactored measurement subscription logic to improve performance and clarity. - Introduced `getClosestOrientationFromIOP` utility for better orientation handling in measurements.

- Changed active tool from Length to WindowLevel in default tool groups. - Added Length tool to passive tool groups for better accessibility. - Updated right panel configuration to include segmentation alongside measurements. - Enabled right panel closure for improved user experience. - Introduced isReferenceViewable method in ViewportGridState for enhanced reference viewability checks.

…t options - Updated `isReferenceViewable` to include `displaySetService` for improved reference image checks. - Adjusted viewport options handling in `ViewportGrid` to ensure proper configuration is passed. - Added logic to check for stack viewport types and retrieve image IDs accordingly.

…unction - Moved the jump to measurement logic from the main component to a dedicated helper function for improved readability and maintainability. - Simplified the viewport display check logic in `ViewportGrid` to enhance clarity and performance. - Updated the handling of measurement metadata to streamline the reference viewability checks.

…measurement interactions - Introduced a new test suite for the Jump to Measurement MPR feature using Playwright. - Implemented tests to validate hydration, measurement tracking, and viewport interactions. - Added multiple screenshot paths to capture the state during various test scenarios. - Included new screenshot assets for initial draw, scrolling away, jumping to measurement stack, and changing series in MPR.

extensions/cornerstone-dicom-sr/src/init.ts

wayfarer3130 · 2025-03-25T12:17:08Z

extensions/cornerstone/src/Viewport/OHIFCornerstoneViewport.tsx

-        elementRef,
-        viewportId,
-        servicesManager
+      const { unsubscribe } = measurementService.subscribe(


Can we call it jumpToViewReference? It just feels like there should be uses of it for things other than measurements - I'm thinking segmentation changes or remembering previously displayed layouts on layout change?

wayfarer3130 · 2025-03-25T12:32:37Z

extensions/cornerstone/src/Viewport/OHIFCornerstoneViewport.tsx

-  if (!displaysUIDs?.length) {
+// Helper function to handle jumping to measurements
+function handleJumpToMeasurement(event, elementRef, viewportId, cornerstoneViewportService) {
+  const { measurement, isConsumed } = event;


Can we get rid of the isConsumed and perform the action in a single application, adding the ability to apply directly on view?

I don't think we can... The data loads into the viewports much later. What I've done is:

Try the layout in viewportGrid. If someone already has that displaySet, bail out quickly and let the viewport handle the jump.

Try the layout in viewportGrid. If no one has that displaySet, set the viewports with that DS on the grid based on the HP. Meanwhile, write down the viewReference in the Zustand positionPresentation store so that it gets picked up later in the cornerstoneViewportService to get applied after the data loads.

If you have written it out to the position presentation in Zustand, then you don't need the consume piece - it is already stored/available for that position, so it should get retored automatically exactly the same way as remembered positions get loaded.

…surements-mpr

Copilot

Pull Request Overview

This PR refactors the measurement jumping logic in the cornerstone extension and related modules to fix issues with outdated methods and improve viewport management. Key changes include refactoring the measurement event subscriptions to remove caching, integrating a new isReferenceViewable utility function across viewports, and updating the viewport service and commands to support these changes.

Reviewed Changes

Copilot reviewed 31 out of 38 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
extensions/measurement-tracking/src/getViewportModule.tsx	Integrated isReferenceViewable utility for tracked viewports
extensions/measurement-tracking/src/contexts/TrackedMeasurementsContext/measurementTrackingMachine.js	Removed unused/commented code regarding SR hydration
extensions/measurement-tracking/src/contexts/TrackedMeasurementsContext/TrackedMeasurementsContext.tsx	Updated measurement event logic with confirmation bypass based on appConfig
extensions/default/src/getViewportModule.tsx	Added isReferenceViewable function returning false for the chart viewport
extensions/default/src/ViewerLayout/index.tsx	Adjusted returned object structure to include isReferenceViewable from the entry
extensions/cornerstone/src/utils/isReferenceViewable.ts	Added a new utility to determine reference viewability from orientation data
extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts	Introduced getViewportDisplaySets method and changed WITH_NAVIGATION configuration
extensions/cornerstone/src/init.tsx	Updated measurement service subscriptions to include both viewport and layout events
extensions/cornerstone/src/index.tsx	Integrated isReferenceViewable callback into the cornerstone viewport module
extensions/cornerstone/src/commandsModule.ts	Modified jump-to-measurement presentation data to include additional options
extensions/cornerstone/src/Viewport/OHIFCornerstoneViewport.tsx	Refactored measurement jump handling by removing caching and updating event subscriptions
extensions/cornerstone-dicom-sr/src/init.ts	Removed deprecated measurement service subscription for layout jumps

Files not reviewed (7)

extensions/cornerstone-dicom-pmap/package.json: Language not supported
extensions/cornerstone-dicom-seg/package.json: Language not supported
extensions/cornerstone-dicom-sr/package.json: Language not supported
extensions/cornerstone-dynamic-volume/package.json: Language not supported
extensions/cornerstone/package.json: Language not supported
extensions/measurement-tracking/package.json: Language not supported
platform/app/package.json: Language not supported

Comments suppressed due to low confidence (2)

extensions/default/src/ViewerLayout/index.tsx:85

The removal of the 'content' property (previously returned as entry.component) may break downstream components that rely on it. Please verify that this change is intentional and update consumers accordingly.

return { entry };

extensions/cornerstone/src/utils/isReferenceViewable.ts:59

Returning undefined here may lead to unintended behavior in consumers expecting an OrientationAxis; consider handling this case explicitly by returning a default value or throwing an error.

if (imageOrientationPatient?.length !== 6) { return; }

extensions/cornerstone/src/Viewport/OHIFCornerstoneViewport.tsx

fix jump to measurement

a1bfdfb

sedghi mentioned this pull request Mar 18, 2025

fix(measurement): fix for measurement jump is not switching series in 2DMPR #4779

Merged

5 tasks

sedghi added 5 commits March 24, 2025 16:54

wayfarer3130 reviewed Mar 25, 2025

View reviewed changes

extensions/cornerstone-dicom-sr/src/init.ts Outdated Show resolved Hide resolved

wayfarer3130 reviewed Mar 25, 2025

View reviewed changes

sedghi added 4 commits March 27, 2025 17:20

Merge branch 'master' of github.com:OHIF/Viewers into fix/jump-to-mea…

432ed86

…surements-mpr

ss

54eeae3

Merge branch 'master' of github.com:OHIF/Viewers into fix/jump-to-mea…

c18aad3

…surements-mpr

test

c68baec

sedghi requested a review from Copilot March 28, 2025 14:19

Copilot AI reviewed Mar 28, 2025

View reviewed changes

extensions/cornerstone/src/Viewport/OHIFCornerstoneViewport.tsx Show resolved Hide resolved

unit test

c49aa14

sedghi merged commit d7bd365 into master Mar 28, 2025
24 of 31 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix jump to measurement #4867

fix jump to measurement #4867

Uh oh!

sedghi commented Mar 18, 2025 •

edited

Loading

Uh oh!

netlify bot commented Mar 18, 2025 •

edited

Loading

Uh oh!

netlify bot commented Mar 18, 2025 •

edited

Loading

Uh oh!

cypress bot commented Mar 18, 2025 •

edited

Loading

Uh oh!

Uh oh!

wayfarer3130 Mar 25, 2025

Uh oh!

wayfarer3130 Mar 25, 2025

Uh oh!

sedghi Mar 25, 2025

Uh oh!

wayfarer3130 Mar 25, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fix jump to measurement #4867

fix jump to measurement #4867

Uh oh!

Conversation

sedghi commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for ohif-dev canceled.

Uh oh!

netlify bot commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ Deploy Preview for ohif-platform-docs failed. Why did it fail? →

Uh oh!

cypress bot commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Viewers Run #5016

Tests for review

Uh oh!

Uh oh!

wayfarer3130 Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

wayfarer3130 Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

sedghi Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

wayfarer3130 Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sedghi commented Mar 18, 2025 •

edited

Loading

netlify bot commented Mar 18, 2025 •

edited

Loading

netlify bot commented Mar 18, 2025 •

edited

Loading

cypress bot commented Mar 18, 2025 •

edited

Loading