fix: restrict labelmap to surface conversion #2426

Devu-trenser · 2025-10-24T09:33:59Z

Context

Fixes issue:

[Bug] Labelmap always converts to Surface on each draw in layouts without 3D once switching to 3D volume layout OHIF/Viewers#5427
[Bug] Switching to 2D layout from Advanced 3D layout triggers an unexpected toast message OHIF/Viewers#5346

This merge request refactors the segmentation representation computation and event subscription system.

Changes & Results

This merge request refactors the segmentation representation computation and event subscription system. The goal of this refactor is to:

Expose event subscription and removal APIs to OHIF.
Centralize listener management for better cleanup and memory safety.
Introduced a dedicated segmentationEventManager module to manage segmentation listeners.
Implemented a nested Map structure to track listeners per segmentation and representation type
Updated computeAndAddRepresentation to use centralized event management instead of directly adding/removing listeners.
Added utility functions to:
- Remove a single representation listener (removeRepresentationListener).
- Remove all listeners for a segmentation (removeAllSegmentationListeners).

Testing

Locally link the OHIF Devu-trenser:fix/restrict-labelmap-to-surface-conversion branch with the branch (Devu-trenser:fix/labelmap-to-surface-conversion)
Launch a data in viewer and switch to a volume (MPR) viewport.
Draw a segmentation using the brush tool.
Switch the layout to a 3D volume viewport.
Return to the stack/MPR viewport and continue drawing segments.
Verify Labelmap segmentations should not be converted to Surface on each draw if there is no enabled 3d volume viewport.

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:
[] "Node version:
[] "Browser:

jbocce · 2025-10-26T23:24:09Z

I tested this PR elaborating on the instructions provided (thank you for such detail).

In summary I locally linked the OHIF master branch with the branch (Devu-trenser:fix/labelmap-to-surface-conversion) with the fix and I found that labelmaps failed to convert to surfaces when switching from MPR to 3D four up. That is to say the third step in the instructions failed.

Here are the specific steps I was using to test this PR...

Checkout the branch Devu-trenser:fix/labelmap-to-surface-conversion
Set up my sandbox for local linking following these instructions
I ran OHIF.
Navigated to http://localhost:3000/segmentation?StudyInstanceUIDs=1.3.6.1.4.1.25403.345050719074.3824.20170125095438.5
Launched MPR with the series Body 3.0 CE
Add a segmentation and used the brush tool to segment the coronal view. I added just a single stroke using the brush tool
Launched 3D Four Up
Return to MPR.
Draw more with the brush tool and ensure each draw is NOT converted to a surface.

Unfortunately the testing failed on step 7 with the following exception(s) and failure to create a surface representation...

surfaceComputationStrategies.js:29 Error: Failed to convert labelmap to surface
    at computeSurfaceFromLabelmapSegmentation (surfaceComputationStrategies.js:54:1)
    at async Module.computeSurfaceData (surfaceComputationStrategies.js:22:1)
    at async computeAndAddRepresentation (computeAndAddRepresentation.js:4:1)
    at async Object.render (surfaceDisplay.js:32:1)

webWorkerManager.js:100 Error executing method 'convertLabelmapToSurface' on worker 'polySeg': TypeError: Cannot read properties of null (reading 'instance')
    at Object.convertLabelmapToSurface (vendors-node_modules_cornerstonejs_polymorphic-segmentation_dist_esm_workers_polySegConverter-544fc…:81:38)

Note that I tried the exact steps above using the commit prior to the one for this PR and there was no exception at step 7.

jbocce

Testing failed.

Devu-trenser · 2025-10-27T08:57:03Z

@jbocce
Thank you for the detailed testing and feedback.

I have verified the behavior again using both the latest OHIF master branch and the OHIF branch fix/restrict-labelmap-to-surface-conversion, linked with the cs3d branch Devu-trenser:fix/labelmap-to-surface-conversion.

Following the exact steps mentioned in your comment, I was unable to reproduce the issue and the labelmap to surface conversion worked as expected without any exceptions.

A reference video demonstrating the test workflow and results has been attached for your review.

Could you please confirm if you’re still able to reproduce this issue, or if I might be missing any specific steps in your testing process?

With latest OHIF master:

OSS.main.branch.mp4

With OHIF branch fix/restrict-labelmap-to-surface-conversion:

OSS.fix.branch.mp4

jbocce · 2025-10-29T12:55:38Z

@Devu-trenser, apologies, I am not sure what is wrong with my local system so I had a colleague go through my testing steps and it passed for them. Now I will look more closely at the code.

packages/tools/src/utilities/segmentation/computeAndAddRepresentation.ts

jbocce · 2025-11-06T13:28:02Z

packages/tools/src/utilities/segmentation/computeAndAddRepresentation.ts

- * @param options - Additional options for computing the representation.
+ * @param type - Representation type (e.g., LABELMAP, CONTOUR).
+ * @param computeFunction - Async function that computes representation data.
+ * @param updateFunction - Optional function to update UI/state on segmentation change.


remove this param from the doc too please

jbocce · 2025-11-06T13:34:39Z

packages/tools/src/tools/displayTools/Surface/surfaceDisplay.ts

+): (segmentationId: string) => Promise<void> {
+  const polySeg = getPolySeg();
+  return (segmentationId: string) =>
+    polySeg.updateSurfaceData(segmentationId, { viewport }).then();


Do we need the .then() here?

jbocce · 2025-11-06T14:12:17Z

packages/tools/src/stateManagement/segmentation/SegmentationRenderingEngine.ts

-          const viewportId = display.render(viewport, representation);
-          viewportRenderList.push(viewportId);
+          display.render(viewport, representation).then(() => {
+            if (!existingRepresentation) {


This is a bit curious for me. From what I can see the display.render also does a addDefaultSegmentationListener via a call to computeAndAddRepresentation. So why do we need this as well?

jbocce

This approach looks good to me. Just a few comments and questions to address. I tested the implementation and it looks great.

Devu-trenser mentioned this pull request Oct 24, 2025

fix: restrict labelmap to surface conversion OHIF/Viewers#5521

Draft

jbocce self-requested a review October 26, 2025 23:27

jbocce requested changes Oct 26, 2025

View reviewed changes

Devu-trenser mentioned this pull request Oct 28, 2025

[Bug] Labelmap always converts to Surface on each draw in layouts without 3D once switching to 3D volume layout OHIF/Viewers#5427

Closed

jbocce reviewed Oct 29, 2025

View reviewed changes

packages/tools/src/utilities/segmentation/computeAndAddRepresentation.ts Outdated Show resolved Hide resolved

jbocce reviewed Oct 29, 2025

View reviewed changes

packages/tools/src/utilities/segmentation/computeAndAddRepresentation.ts Show resolved Hide resolved

fix: restrict labelmap to surface conversion

26e53b0

Devu-trenser force-pushed the fix/labelmap-to-surface-conversion branch from 6bd1709 to 26e53b0 Compare October 31, 2025 08:47

Devu-trenser added 5 commits October 31, 2025 14:36

refactor: fix surface handling

8bbeec0

fix: added missed files

514b27e

fix: add/remove event listener for seg rep modification

d66a8ea

chore: removed unused import

30ac55f

chore: removed unintended changes

a1ea068

jbocce reviewed Nov 6, 2025

View reviewed changes

jbocce requested changes Nov 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: restrict labelmap to surface conversion #2426

fix: restrict labelmap to surface conversion #2426

Uh oh!

Devu-trenser commented Oct 24, 2025 •

edited

Loading

Uh oh!

jbocce commented Oct 26, 2025

Uh oh!

jbocce left a comment

Uh oh!

Devu-trenser commented Oct 27, 2025

Uh oh!

jbocce commented Oct 29, 2025

Uh oh!

Uh oh!

Uh oh!

jbocce Nov 6, 2025

Uh oh!

jbocce Nov 6, 2025

Uh oh!

jbocce Nov 6, 2025

Uh oh!

jbocce left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: restrict labelmap to surface conversion #2426

Are you sure you want to change the base?

fix: restrict labelmap to surface conversion #2426

Uh oh!

Conversation

Devu-trenser commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Changes & Results

Testing

Checklist

PR

Code

Public Documentation Updates

Tested Environment

Uh oh!

jbocce commented Oct 26, 2025

Uh oh!

jbocce left a comment

Choose a reason for hiding this comment

Uh oh!

Devu-trenser commented Oct 27, 2025

Uh oh!

jbocce commented Oct 29, 2025

Uh oh!

Uh oh!

Uh oh!

jbocce Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

jbocce Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

jbocce Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

jbocce left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Devu-trenser commented Oct 24, 2025 •

edited

Loading