fix(viewport-sync): Enable re-sync image slices in a different position when needed

[mateusfreira]

Context

Changes & Results

This is a specific change SyncGroupService.ts to clean up the previously registered spatialRegistrationMetadataProvider by cornerstone.js.

When creating the correlation matrix among the viewports cornerstone.js for performance reasons, it caches the matrix using the spatialRegistrationMetadataProvider, but when the tool is disabled, it does not clean it up. This makes it impossible to re-sync the viewports to a different point if the user wants to.

This is a common use case when radiologists are comparing 2 studies (one current and one prior) and by mistake, enable the sync on a non-optimal point and want to disable it, scrool one of the viewports to the correct point and then lock the scroll again.

Here Cornestone calculateViewportsSpatialRegistration and it never gets cleaned up.

Testing

I Have demonstrated this Bug on a video available in youtube also show the fix

(Watch from 2:20:50 to 2:25:52 where I demo the Bug and the fix after my code change)

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 apple M1
• Node version:

• v18.17.0

• Browser:

• Arc Chromium Engine Version 119.0.6045.199
• Chrome Version 122.0.6261.112 (Official Build) (arm64)

[netlify]

✅ Deploy Preview for ohif-dev canceled.

Name	Link
🔨 Latest commit	273b9d1
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-dev/deploys/6626b13166110c00088518d0

[netlify]

✅ Deploy Preview for ohif-platform-docs canceled.

Name	Link
🔨 Latest commit	273b9d1
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-docs/deploys/6626b1316d1641000892cc34

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.41%. Comparing base (8a335bd) to head (273b9d1).
Report is 343 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3984      +/-   ##
==========================================
- Coverage   46.23%   44.41%   -1.83%     
==========================================
  Files          78       80       +2     
  Lines        1276     1333      +57     
  Branches      312      327      +15     
==========================================
+ Hits          590      592       +2     
- Misses        548      588      +40     
- Partials      138      153      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sedghi]

can we filter and only do this for image slice sync synchronizer?

[mateusfreira]

Thanks for the review, @sedghi; I sure can do it; Must does not seem to exists a way to differentiate the synchronizer other than the name.

Is checking the name of the sincronizer enough for this situation?

[sedghi]

sure

[mateusfreira]

Thanks, @sedghi. I have updated the code, and it is working as expected now. Let me know if you think I should improve anything else.

[IbrahimCSAE]

LGTM

[sedghi]

I still need to test this, @mateusfreira can you provide reproducible steps to follow that shows the bug?

[sedghi]

Tested it and worked great, Lovely fix, thanks a lot!