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

Add functionality to convert images to labelmaps #444

Merged
merged 41 commits into from
Nov 13, 2023
Merged

Conversation

floryst
Copy link
Collaborator

@floryst floryst commented Oct 4, 2023

  • Improve image space comparison to check physical spacing
  • Add support to convert an image to a labelmap for painting
  • Update the labelmap store API
    • support the aforementioned functionality and future metadata
    • explicitly store ordering of labelmaps per image
  • Update the UX around layering to instead be accessed via a dropdown menu

@netlify
Copy link

netlify bot commented Oct 4, 2023

Deploy Preview for volview-dev ready!

Name Link
🔨 Latest commit 2936961
🔍 Latest deploy log https://app.netlify.com/sites/volview-dev/deploys/65525e42fb5ba8000762dca2
😎 Deploy Preview https://deploy-preview-444--volview-dev.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@floryst floryst requested a review from PaulHax October 4, 2023 17:38
@floryst
Copy link
Collaborator Author

floryst commented Oct 4, 2023

I'm open to workflow suggestions surrounding labelmap conversion. Right now this does not support converting DICOM datasets to labelmaps.

Copy link
Collaborator

@PaulHax PaulHax left a comment

Choose a reason for hiding this comment

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

Geting TypeError when:

  1. Load Fetus
  2. Immediately click Annotations tab

When converting image to labelmap, it disapears from DataBrowser. 🥹 No going back?

Nice to have an icon or something indicate when a image is a Layer on another image.

When do we do the great layer and labelmap unification? First step unifies the Labelmap stack on the Annotations tab with the Layers stack on the Rendering tab?

src/components/LabelmapList.vue Outdated Show resolved Hide resolved
@aylward
Copy link
Contributor

aylward commented Oct 4, 2023

Painted labels appear as very thin lines in orthogonal slices, and they should represent the resolution in the orthogonal direction.

Steps to reproduce:

  • Load Prostate data
  • Draw in any slice
  • Error: in the other windows only a thin line is drawn.
image

@aylward
Copy link
Contributor

aylward commented Oct 4, 2023

Instead of presenting items at the labelmap level, they should be presented at the paint-color level. Or maybe this should be under the labelmap grouping. That is, if a color appears in a labelmap, it should appear as an item under that labelmap in the annotations tab, and it should be possible to rename, generate a 3D rendering, convert to contour, etc for that specific paint color. The idea is that we're generating objects (annotations) and they can be rendered in different ways in the 2D and 3D views. But things are presented at the object level, not at the labelmap level.

@floryst
Copy link
Collaborator Author

floryst commented Oct 4, 2023

Painted labels appear as very thin lines in orthogonal slices...

If you zoom in, those lines do correspond to the thickness of the painted slice.

Instead of presenting items at the labelmap level, they should be presented at the paint-color level.

Good point. The next step is to expose an API that operates at the segmentation level. This is closely related to configurable default segmentation labels (paint labels) and how those affect labelmaps.

@aylward
Copy link
Contributor

aylward commented Oct 4, 2023

You're right! We need to include resolution (voxel size) info in the information box and give physical position of the cursor in the corner annotations. Additionally, for the brush size we need to give physical units. I was mistakenly thinking it was corresponding to 1 voxel, but it does not.

This gets hard to grok when drawing things with a brush size of 1 in the different planes. In the following I draw with brush size 1 as follows

  • Green square box was drawn in axial and correctly shows as a square box in that isotropic plane
  • Red boxes were drawn in the coronal and sagittal planes, and I expected them to produce a square box in the axial plane (reflecting one voxel being painted) - but it is approximating a in-plane circle, not drawing 1 voxel. So, I think everything is ok, but we need to convey the physical size of the brush. We should also have the option to turn off texture interpolation when rendering the slice data, so that voxel edges become more apparent and the data's resolution becomes appropriately represented.
image

@floryst
Copy link
Collaborator Author

floryst commented Oct 12, 2023

When converting image to labelmap, it disapears from DataBrowser. 🥹 No going back?

I'll be adding a means of going from labelmap -> image.

Nice to have an icon or something indicate when a image is a Layer on another image.

Agreed. I can add an icon indicating that an image is a layer. We can worry about indicating which images its a layer of when we need it.

When do we do the great layer and labelmap unification? First step unifies the Labelmap stack on the Annotations tab with the Layers stack on the Rendering tab?

Layer + labelmap unification is more tricky, so we won't touch on it in this work. For one, overlaying other images is a purely rendering thing, while labelmaps are an annotation thing. Their colormap requirements are different (you can edit labelmap colormaps), and same with the opacity. Right now their data models are distinct enough that we should keep them apart. We can revisit this down the line.

- Add a new onImageDeleted listener to delete labelmaps when the parent
image is gone.
- Explicitly store labelmap insertion order.
- Add new APIs for adding and deleting labelmaps. Existing APIs are
preserved.
Instead of comparing image spaces via metadata comparison, we instead
check to see that the volume corners are the same (module rotations and
flips).
The IsolatedDialog currently stops certain keys from propagating out of
the dialog.
@floryst floryst force-pushed the convert-to-labelmap branch from ee10b65 to 3b68ef7 Compare October 13, 2023 19:28
@floryst
Copy link
Collaborator Author

floryst commented Oct 13, 2023

I've added the segment editor. Please try it out and let me know if there are any bugs/basic issues, e.g. the eraser isn't available yet.

@PaulHax
Copy link
Collaborator

PaulHax commented Oct 17, 2023

Segment editor is snazy. Basic stuff:

  1. Should opacity be per labelmap like layers?
  2. Should new labelmap copy all of last labelmap segments?
  3. Saving a session.zip has error.
  4. Should we call segments -> labels? Or vice versa...
  5. Should the label editor UI match the segment editor UI or vice versa?

@floryst
Copy link
Collaborator Author

floryst commented Oct 18, 2023

  1. Should opacity be per labelmap like layers?

Yes, that's a good idea to consider. I'm thinking we can even make opacity be applied per-segment, but I'm not sure how useful that is.

  1. Should new labelmap copy all of last labelmap segments?

I like that behavior. I'll add it unless we have good reasons to not do such a thing otherewise.

  1. Saving a session.zip has error.

Thanks, will look into it.

  1. Should we call segments -> labels? Or vice versa...

This naming is mostly going off of Slicer, where a labelmap is a type of segmentation, and segments are just a particular label. I'm keeping with segments for now to reflect that, but if labels is more intuitive then we can call them labels.

  1. Should the label editor UI match the segment editor UI or vice versa?

Good point, I haven't considered that. I think I'll try using the label editor UI for the segment editor UI.

@aylward
Copy link
Contributor

aylward commented Oct 25, 2023

How do you give a name to a ruler, square, or polyline annotation. Much each new name have a new label? Can I have multiple things labeled and painted as "tumor" and yet name each one independently, e.g., tumor 1, tumor big, maybe-its-a-tumor tumor...

@floryst
Copy link
Collaborator Author

floryst commented Oct 26, 2023

Let's discuss the behaviors for annotation labels, since I think there are some conflicting expectations.

@floryst
Copy link
Collaborator Author

floryst commented Oct 31, 2023

Updated:

  • measurements and labelmaps are now organized into tabs. Selecting certain annotation tools will focus the tab.

I do not plan on addressing the expected behavior of labels in this PR because this is intended to only be related to labelmaps.

@floryst
Copy link
Collaborator Author

floryst commented Nov 3, 2023

Update:

  • add eraser
  • merged main into this branch

@aylward @finetjul how does the following naming seem to you?

  • Labelmap -> SegmentGroup
  • Label/Segment -> SegmentMask (runner-up suggestion: Segment)

Once the naming is locked down, I will begin renaming various parts of VolView to reflect the terminology change.

@floryst
Copy link
Collaborator Author

floryst commented Nov 6, 2023

Updated:

  • eraser is now global and available via a tool mode swatch in the paint controls
  • user is prompted for a new name when a labelmap is created
  • dropdown menu has been replaced in favor of a radio list of labelmaps

@floryst floryst force-pushed the convert-to-labelmap branch from bccd0db to 4a34db6 Compare November 6, 2023 23:05
@floryst
Copy link
Collaborator Author

floryst commented Nov 7, 2023

@aylward @PaulHax this is ready for functional evaluation again. Ignore the naming for now.

@floryst
Copy link
Collaborator Author

floryst commented Nov 13, 2023

I'll probably merge this soon and we can iterate on it after the fact.

@floryst floryst added this pull request to the merge queue Nov 13, 2023
Merged via the queue into main with commit cbd3d93 Nov 13, 2023
6 checks passed
@floryst floryst deleted the convert-to-labelmap branch March 21, 2024 16:26
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.

3 participants