Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(adapters): overlapping segs with labelmap images #1815

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

pedrokohler
Copy link
Contributor

@pedrokohler pedrokohler commented Feb 5, 2025

Context

In order to address OHIF/Viewers#3496 we're taking a two steps approach:

1 - chopping the volume data buffer array into planar data buffer arrays
2 - adopt some strategy to duplicate these planar data buffer arrays when we get overlapping segs

This PR aims to cover the second step.

Changes & Results

Before - loss of information:
image

After - overlapping segs maintained:
image

Testing

Checklist

PR

  • [] My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

Public Documentation Updates

  • [] The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • [] "OS:
  • [] "Node version:
  • [] "Browser:

Copy link

netlify bot commented Feb 5, 2025

Deploy Preview for cornerstone-3d-docs failed. Why did it fail? →

Name Link
🔨 Latest commit 12b61ca
🔍 Latest deploy log https://app.netlify.com/sites/cornerstone-3d-docs/deploys/67b5083af3556b0008b3244a

@sedghi
Copy link
Member

sedghi commented Feb 6, 2025

I don't think we're on the right track here. The adapter should return imageIds, and the example/consumer/ohif should create a single segmentation from those imageIds. We might need to tweak the internal segmentation addition and hydration process to make this work, but it's pretty likely to be okay. For now, let's put the volume one on the back burner - we'll need more optimizations to create sub-volumes for each imageId group, and that's a bit more complicated since we'd need to do it only where they're actually located. So make sure the example work with having one segmentation, the segmentation is consisted of multiple imageIds

@pedrokohler
Copy link
Contributor Author

I don't think we're on the right track here. The adapter should return imageIds, and the example/consumer/ohif should create a single segmentation from those imageIds. We might need to tweak the internal segmentation addition and hydration process to make this work, but it's pretty likely to be okay. For now, let's put the volume one on the back burner - we'll need more optimizations to create sub-volumes for each imageId group, and that's a bit more complicated since we'd need to do it only where they're actually located. So make sure the example work with having one segmentation, the segmentation is consisted of multiple imageIds

I was really hoping you would point to specific parts of the code and ask for specific changes instead of abstract ones. I'm not following what you're asking me to do.

@@ -104,7 +104,7 @@ const state = {
toolGroup: null,
toolGroupId: "MY_TOOL_GROUP_ID",
viewportIds: ["CT_AXIAL"],
segmentationId: "LOAD_SEG_ID:" + cornerstone.utilities.uuidv4(),
segmentationIds: [],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we should have the segmentationId, there is one segmentation for this case and it happens to be overlapping, so don't change this to an array

state.viewportIds[0],
[{ segmentationId: state.segmentationId }]
);
for (const segmentationId of state.segmentationIds) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, there is only one segmentation here

@sedghi
Copy link
Member

sedghi commented Feb 6, 2025

So basically the idea is to have this work.

const { labelMapImages } = 
        await Cornerstone3D.Segmentation.createFromDICOMSegBuffer(
            referenceImageIds,
            arrayBuffer,
            {
                metadataProvider: metaData,
                skipOverlapping: false
            }
        );

Then we do

csToolsSegmentation.addSegmentations([
        {
            segmentationId,
            representation: {
                type: cornerstoneTools.Enums.SegmentationRepresentations
                    .Labelmap,
                data: {
                    imageIds: labelMapImages.flat()
                }
            }
        }
    ]);

The key files that need to be changed are

https://github.com/cornerstonejs/cornerstone3D/blob/main/packages/tools/src/eventListeners/segmentation/imageChangeEventListener.ts

Here

const derivedImageId = getCurrentLabelmapImageIdForViewport(
        viewportId,
        representation.segmentationId
      );

as you see it only get one labelmapimageId for viewport => but we have changed that so that it can have multiple for one segmentationId (overlapping)

and then later on in that file

viewport.addImages([
        {
          imageId: derivedImageId,
          representationUID: `${segmentationId}-${SegmentationRepresentations.Labelmap}`,
          callback: ({ imageActor }) => {
            imageActor.getMapper().setInputData(imageData);
          },
        },
      ]);

you see we only add one derivedImageId to the viewport (since we only had support for one slice -> one imageId -> per labelmap) but now we want to support multiple

so we need to loop over derivedImageIds for that slice and then add them one by one which should work

Hope this helps

@pedrokohler pedrokohler force-pushed the feat-overlapping-segs branch from 3f0ab31 to 3113419 Compare February 12, 2025 12:21
@pedrokohler pedrokohler force-pushed the feat-overlapping-segs branch from 713eb66 to ec8b82a Compare February 12, 2025 12:46
@pedrokohler pedrokohler requested a review from sedghi February 12, 2025 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants