feat: add lock label visibility checkbox for multi-label cases - #25
Open
zapaishchykova wants to merge 3 commits into
Open
feat: add lock label visibility checkbox for multi-label cases#25zapaishchykova wants to merge 3 commits into
zapaishchykova wants to merge 3 commits into
Conversation
Adds a "Lock label visibility across cases" checkbox that lets users persist per-label visibility settings when reviewing multi-label masks. Previously the restore logic was always a no-op because visibility was keyed by Slicer's auto-generated segment ID (e.g. "Segment_1_20240901_…") which differs across NIfTI files, so every get() fell back to True (fully visible). Visibility is now keyed by position index which is stable across cases with the same label set. Also fixes the centroid fiducial filter in set_segmentation_and_mask_for_segmentation_editor to use the same index-based lookup. Closes #16
There was a problem hiding this comment.
Pull request overview
Adds an opt-in UI control to persist segmentation label visibility across multiple reviewed cases, and updates visibility/centroid logic to use a stable per-segment index key rather than Slicer’s per-file auto-generated segment IDs.
Changes:
- Introduces a “Lock label visibility across cases” checkbox and a
lock_visibilityflag. - Stores/restores segment visibility keyed by segment position index (instead of segment ID) and clears stored state on directory change.
- Updates centroid jump filtering logic to reference index-based visibility state.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
661
to
665
| seg_ids = list(self.segmentation_node.GetSegmentation().GetSegmentIDs()) | ||
| for segmentId in stats["SegmentIDs"]: | ||
| if self.segment_visiblity_states.get(segmentId, True): | ||
| idx = seg_ids.index(segmentId) if segmentId in seg_ids else None | ||
| if self.segment_visiblity_states.get(idx, True): | ||
| centroid_ras = stats[segmentId, "LabelmapSegmentStatisticsPlugin.centroid_ras"] |
Comment on lines
+661
to
+664
| seg_ids = list(self.segmentation_node.GetSegmentation().GetSegmentIDs()) | ||
| for segmentId in stats["SegmentIDs"]: | ||
| if self.segment_visiblity_states.get(segmentId, True): | ||
| idx = seg_ids.index(segmentId) if segmentId in seg_ids else None | ||
| if self.segment_visiblity_states.get(idx, True): |
When lock_visibility=False, restore_segment_visiblity_states returns early so the new case renders with all labels visible. The centroid filter in set_segmentation_and_mask_for_segmentation_editor was still reading stale stored states from the previous case, causing centroids to be hidden for labels that are actually visible. Skip the stored-state filter entirely when the lock is off so centroid markers always match actual display state.
Replace the stored-state + index-lookup approach with a direct call to GetSegmentVisibility on the display node. This fixes both Copilot issues: - the filter now reads the true display state (already correct after restore_segment_visiblity_states runs), so it works regardless of whether the lock checkbox is on or off - eliminates the O(n²) seg_ids.index() loop in favour of a single O(1) display-node call per segment
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.
Summary
Segment_1_20240901_123456) which differs between NIfTI files — everyget()silently fell back toTrue(fully visible), making the existingstore/restorecode a no-op; now keyed by position index which is stable across cases with the same label setset_segmentation_and_mask_for_segmentation_editorwhich had the same segment-ID key bugChanges
__init__— new flag:setup()— new checkbox (unchecked by default, so existing behaviour is unchanged):store_segment_visiblity_states— key by index:restore_segment_visiblity_states— conditional restore:onAtlasDirectoryChanged— reset visibility state on directory switch:Test plan