Fix: Segmentation "Hide All" fails when switching from 2D to 3D four-…#5995
Open
nithin-trenser wants to merge 3 commits into
Open
Fix: Segmentation "Hide All" fails when switching from 2D to 3D four-…#5995nithin-trenser wants to merge 3 commits into
nithin-trenser wants to merge 3 commits into
Conversation
❌ Deploy Preview for ohif-dev failed. Why did it fail? →
|
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
@sedghi Could you please take a look at this PR and provide your feedback? |
Contributor
|
@wayfarer3130 Could you please review this PR? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Fix issue: #5948
After loading a segmentation file in the 2D viewport, switching to the 3D four-up viewport causes the "Hide All" functionality to stop working. When the user selects "Hide All", the segmentations remain visible instead of being hidden.
Changes & Results
Refactor segmentation type selection to use explicit viewport-based logic.
For 3D viewports, enforce
Surface; for 2D, allowContouror default toLabelmap.This change will ensure consistent behavior and prevent unsupported types from propagating
Seg-3d-four-up-hide-error.mp4
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Greptile Summary
Fixes the "Hide All" regression that occurred when switching from a 2D viewport (with a Labelmap/Contour segmentation) to a 3D four-up layout. The root cause was that adding a Surface representation to the 3D viewport fired a
SEGMENTATION_REPRESENTATION_MODIFIEDevent, and the old?? Labelmapfallback would propagateSurfaceto fresh 2D viewports instead of defaulting toLabelmap.Surfacebeing assigned to a 2D viewport:Contouris kept as-is, any other non-Surfacetruthy value is passed through, andSurface(or null/undefined) falls back toLabelmap.Surfaceis always enforced there.Confidence Score: 5/5
The change is narrow and well-targeted — it only affects the type-selection step inside the synchroniser callback, and the new logic correctly handles all current representation values (Labelmap, Contour, Surface) as well as null/undefined.
The fix is a single ternary chain that guards 2D viewports against receiving a Surface representation when the synchroniser fires after the 3D viewport is initialised. All existing cases (Contour preserved, Labelmap preserved, Surface → Labelmap, null → Labelmap) are handled correctly, and the 3D path is untouched.
No files require special attention.
Important Files Changed
Reviews (3): Last reviewed commit: "Merge branch 'master' into fix-seg-load-..." | Re-trigger Greptile