Skip to content

fix(synchronization): Slice Synchronization Fails for Series with Mismatched FOR - #2825

Open
arul-trenser wants to merge 1 commit into
cornerstonejs:mainfrom
arul-trenser:fix-scroll-sync
Open

fix(synchronization): Slice Synchronization Fails for Series with Mismatched FOR#2825
arul-trenser wants to merge 1 commit into
cornerstonejs:mainfrom
arul-trenser:fix-scroll-sync

Conversation

@arul-trenser

@arul-trenser arul-trenser commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Context

When two viewports have different FrameOfReferenceUID, scroll synchronization falls back to calculateViewportsSpatialRegistration() to align them using imagePositionPatient

This fallback was calling viewport.getSliceIndex() and passing the returned slice number as the imageId to metaData.get(imagePlaneModule, imageId). Since a slice index is not a valid imageId, the metadata lookup returned undefined, so the spatial registration was silently skipped, breaking scroll sync whenever viewports belonged to different frames of reference. Sync only appeared to work when FrameOfReferenceUID was identical (or empty/undefined on both sides), because that path bypasses the fallback entirely.

The PR is incorporated by FlyWheel.io.

Fixes: #2762

Changes & Results

File changed: calculateViewportsSpatialRegistration.ts

  • Replaced viewport.getSliceIndex() with viewport.getImageIds()[0] to get the imageId of each viewport.
  • Passed this imageId directly to metaData.get('imagePlaneModule', imageId), instead of the slice index.

Result: imagePlaneModule metadata now resolves correctly for both viewports, so the translation between imagePositionPatient values is calculated properly, and scroll synchronization works again for viewports with mismatched FrameOfReferenceUID.

Before:

2762-before.mp4

After:

2762-after.mp4

Testing

  • Launch the OHIF Viewer using this study link: OHIF Viewer Dev
  • Change the layout to a 2-viewport configuration (1x2 or 2x1).
  • Load the following series into each viewport:
  • Viewport 1: Body 3.0 CE
  • Viewport 2: Body 5.0 Lung I+ CE
  • Enable the Image Slice Sync tool.
  • Scroll through the images in Viewport 1.
  • Verify that Viewport 2 scrolls in sync with Viewport 1, matching the corresponding slice position.

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: Windows 11
  • "Node version: 22.19.0
  • "Browser: 150.0.7871.187

Summary by CodeRabbit

  • Bug Fixes
    • Improved spatial registration across viewports by using the active image identifiers directly.
    • Increased reliability when calculating image alignment and translation between viewports.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

calculateViewportsSpatialRegistration now uses each viewport’s first image ID for image plane metadata lookup, replacing slice-index-derived keys while preserving the existing registration calculations.

Changes

Viewport spatial registration

Layer / File(s) Summary
Use image IDs for metadata lookup
packages/core/src/utilities/calculateViewportsSpatialRegistration.ts
The utility obtains the first image ID from each viewport and passes those IDs directly to image plane metadata lookup instead of converting slice indices to strings.

Estimated code review effort: 2 (Simple) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change directly fixes #2762 by using a valid imageId for spatial registration, restoring slice sync for mismatched FrameOfReferenceUIDs.
Out of Scope Changes check ✅ Passed The PR stays focused on the reported sync bug and changes only the relevant utility file.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly describes the slice synchronization fix and follows the semantic-release format.
Description check ✅ Passed The description includes context, changes, results, testing steps, and completed checklist items.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sen-trenser

Copy link
Copy Markdown

@sedghi Could you please take a look at this PR?
Thanks!

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] Slice Synchronization Fails for Series with Mismatched FOR

2 participants