Skip to content

fix(seg): prevent viewport orientation change when loading SEG in manual grid layout - #6021

Merged
jbocce merged 7 commits into
OHIF:masterfrom
GhadeerAlbattarni:fix/6012-seg-layout-switching
May 19, 2026
Merged

fix(seg): prevent viewport orientation change when loading SEG in manual grid layout#6021
jbocce merged 7 commits into
OHIF:masterfrom
GhadeerAlbattarni:fix/6012-seg-layout-switching

Conversation

@GhadeerAlbattarni

@GhadeerAlbattarni GhadeerAlbattarni commented May 14, 2026

Copy link
Copy Markdown
Collaborator

Context

Fixes #6012

When a user switches from 3D four up layout to a manual grid layout (e.g. 3×2) and then drags a SEG into an empty viewport, all viewports reset to axial orientation and the segmentation loads in the wrong viewport.

Changes & Results

  • hydrationUtils.ts: skip mergeVolumeSharingViewports when isHangingProtocolLayout=false.
  • SEGHydrationFrom3DFourUp.spec.ts: add playwright test covering SEG hydration after switching from 3D four up to a manual 3x2 grid layout.
  • MainToolbarPageObject.ts: add grid(cols, rows) helper for selecting custom grid layouts by column and row count
load-seg-3DfourUp-to-3By2.mov

Testing

  1. Open a study that has SEG (e.g., StudyInstanceUIDs=1.3.6.1.4.1.14519.5.2.1.256467663913010332776401703474716742458)
  2. Switch to 3D four up
  3. Switch to 3x2
  4. Drag SEG series into the 3rd Viewport
  • Verify the segmentation is loaded only to the target viewport (3rd) and all other viewports remain unchanged
  1. Switch back to 3D four up
  • Verify no segmentation in the viewports

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • [] My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • [] The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • OS: macOS 10.15.4
  • Node version: v22.13.0
  • Browser: Chrome 83.0.4103.116

Greptile Summary

Prevents viewport orientation resets when a SEG is dragged into a viewport under a manually-selected grid layout. The root cause was mergeVolumeSharingViewports propagating updates to all volume-sharing viewports regardless of layout type; a single guard now skips that merge when isHangingProtocolLayout is false.

  • hydrationUtils.ts: adds || !isHangingProtocolLayout to the early-return condition so mergeVolumeSharingViewports is only invoked for hanging-protocol-driven layouts, not manual grids.
  • SEGHydrationFrom3DFourUp.spec.ts: new describe block verifies that hydrating a SEG into a specific viewport in a 3×2 grid leaves other viewports unchanged, then confirms no segmentation appears after switching back to 3D four-up.
  • MainToolbarPageObject.ts: adds grid(cols, rows) helper to select custom grid layouts by column/row count, consistent with the existing layout-selector pattern.

Confidence Score: 5/5

Safe to merge; the core change is a one-line guard that only affects the manual-grid code path.

The fix is minimal and targeted: it skips mergeVolumeSharingViewports exclusively when isHangingProtocolLayout is false, leaving the hanging-protocol path unchanged. The accompanying test exercises the exact scenario described in the bug report.

No files require special attention; test quality notes were already raised in prior review threads.

Important Files Changed

Filename Overview
extensions/cornerstone/src/utils/hydrationUtils.ts Single-line guard added to skip mergeVolumeSharingViewports when isHangingProtocolLayout=false; fix is focused and correct
tests/SEGHydrationFrom3DFourUp.spec.ts New describe block tests the SEG hydration after layout switch; uses correct render-cycle pattern but skips { renderedTimeout: 180000 } for backTo3DFourUp screenshot (already discussed in review threads)
tests/pages/MainToolbarPageObject.ts Adds grid(cols, rows) helper method consistent with the existing layout-selector pattern; test-ID convention (Layout-${cols-1}-${rows-1}) matches usage in other spec files
tests/utils/screenShotPaths.ts Adds segHydrationAfterLayoutSwitchTo3By2 entry; filenames are unique within the spec's screenshot directory

Reviews (5): Last reviewed commit: "add small change to the test" | Re-trigger Greptile

@netlify

netlify Bot commented May 14, 2026

Copy link
Copy Markdown

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit a8860dc
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/6a0cbedd9b949dee2543830c
😎 Deploy Preview https://deploy-preview-6021--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment thread tests/SEGHydrationFrom3DFourUp.spec.ts Outdated
Comment thread tests/SEGHydrationFrom3DFourUp.spec.ts Outdated
Comment thread tests/SEGHydrationFrom3DFourUp.spec.ts Outdated
Comment thread tests/SEGHydrationFrom3DFourUp.spec.ts
@GhadeerAlbattarni
GhadeerAlbattarni requested a review from jbocce May 14, 2026 20:58
@jbocce

jbocce commented May 15, 2026

Copy link
Copy Markdown
Collaborator

@claude review

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, comment @claude review on this pull request to trigger a review.

@jbocce jbocce left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the one suggestion from AI and let me know how it goes. Thanks for this.

@jbocce jbocce left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome.

@jbocce
jbocce merged commit 95251ab into OHIF:master May 19, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Segment not loading correctly after layout changes

2 participants