fix(cornerstone): share one color LUT across a segmentation's viewport representations#6141
Open
TFRadicalImaging wants to merge 2 commits into
Open
fix(cornerstone): share one color LUT across a segmentation's viewport representations#6141TFRadicalImaging wants to merge 2 commits into
TFRadicalImaging wants to merge 2 commits into
Conversation
…t representations A labelmap segmentation created by createLabelmapForDisplaySet had no dedicated color LUT, so every viewport that rendered it created its own copy of the default LUT. Editing a segment color only mutated the active viewport's LUT, so the change was lost as soon as a drawing tool was used in another viewport and the segment reverted to the default color. Create a dedicated color LUT when the segmentation is created and record its index, mirroring the SEG and RT paths, so all representations reuse the same LUT and color edits are retained everywhere.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
TFRadicalImaging
temporarily deployed
to
fork-pr-approval
July 13, 2026 15:48 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Author
|
@wayfarer3130 @jbocce could one of you take a look? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesSegmentation Color LUT
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@extensions/cornerstone/src/services/SegmentationService/SegmentationService.test.ts`:
- Around line 1335-1354: The regression test for createLabelmapForDisplaySet
must exercise representation creation, not only LUT registration. After creating
the segmentation, invoke the relevant addSegmentationRepresentation flow for two
viewports and assert each call receives the stored color LUT index 7, confirming
both representations reuse it.
In
`@extensions/cornerstone/src/services/SegmentationService/SegmentationService.ts`:
- Around line 515-521: Update addOrUpdateSegmentation so reusing an existing
segmentationId does not allocate a new LUT or overwrite
_segmentationIdToColorLUTIndexMap. Before calling addColorLUT, retrieve and
reuse the existing LUT index for that ID, or reject the duplicate ID; preserve
the shared LUT for all current and future viewport representations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c3108dff-0a84-44e8-b9e3-877c6e088c8c
📒 Files selected for processing (2)
extensions/cornerstone/src/services/SegmentationService/SegmentationService.test.tsextensions/cornerstone/src/services/SegmentationService/SegmentationService.ts
…is reused The caller may pass the id of an existing segmentation, which this method updates rather than replaces. Allocating a new LUT and overwriting the mapping in that case would strand the representations already rendering the segmentation on the previous LUT while later ones used the new index, reintroducing the colour divergence this is meant to fix. Only allocate a LUT when the segmentation has none.
TFRadicalImaging
temporarily deployed
to
fork-pr-approval
July 13, 2026 16:18 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
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
1 participant
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
Editing a segment's color on a labelmap segmentation created by
createLabelmapForDisplaySet(i.e. a segmentation the user creates in the viewer, as opposed to one loaded from a SEG or RT display set) does not stick: as soon as a drawing tool is used in another viewport, the segment reverts to its default color.The cause is that
createLabelmapForDisplaySetnever creates a dedicated color LUT for the segmentation. The SEG and RT paths both do — they calladdColorLUTand record the index in_segmentationIdToColorLUTIndexMap— but the labelmap path does not. With no index recorded, every viewport that renders the segmentation ends up with its own copy of the default LUT. A color edit therefore mutates only the LUT belonging to the viewport it was made in, and any other viewport keeps rendering the old default.Changes & Results
extensions/cornerstone/src/services/SegmentationService/SegmentationService.ts:createLabelmapForDisplaySet, create a dedicated color LUT up front and record its index in_segmentationIdToColorLUTIndexMap, mirroring whatcreateSegmentationForSEGDisplaySetandcreateSegmentationForRTDisplaySetalready do.Before: change a segment's colour, then draw in another viewport → the segment reverts to the default colour.
After: the edited colour is retained across all viewports.
This is 8 lines and does not change the SEG/RT paths or the LUT plumbing itself; it just brings the labelmap path in line with them.
Testing
extensions/cornerstone/src/services/SegmentationService/SegmentationService.test.ts— new case in thecreateLabelmapForDisplaySetblock asserting that a dedicated color LUT is created and its index recorded against the segmentation id. It is a genuine regression test: it fails onmaster(addColorLUTis never called) and passes with this change.The full
SegmentationServicesuite passes (86 tests).Manually:
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
This fix is contributed by the University of Calgary, who are donating it back to OHIF. It was co-authored with Ron Leisti.
Summary by CodeRabbit