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

feature(viewer.js): Polygon/Point/Ellipse/Rectangle 2D and 3D bulk annotations support #170

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

igoroctaviano
Copy link
Collaborator

@igoroctaviano igoroctaviano commented Sep 12, 2023

Related PR: ImagingDataCommons/dicom-microscopy-viewer#104

Polygon/Point/Ellipse/Rectangle 2D and 3D bulk annotations support

  • Render full 2D and 3D polygon/ellipse/rectangle at the highest resolution
  • Render clusters at lower resolutions

Testing

  • Check out to dicom-microscopy-viewer's branch feat/bulk-annotations and link that library to SLIM via yarn link
  • If you are not using SLIM's docker-compose, check out the docker-compose changes from this branch feat/polygon-bulk-annotations and apply them to your dcm4chee docker configuration. There's a change there to increase the Java heap size (1024) to properly handle the large bulk annotation without buffer/memory errors

Configuration

You can define a fixed zoom level to transition from clusters to full annotations as shown below (the default value is the highest zoom level available)

{
   annotationOptions: {
      maxZoom: 3
   }
}

@igoroctaviano igoroctaviano changed the title Feat/polygon bulk annotations WIP: Polygon bulk annotations Sep 12, 2023
@igoroctaviano igoroctaviano changed the title WIP: Polygon bulk annotations feat(viewer): Polygons support for bulk annotations Sep 12, 2023
@igoroctaviano
Copy link
Collaborator Author

igoroctaviano commented Sep 18, 2023

@CPBridge @DanielaSchacherer

Linking to DMV

# From the DMV root folder run
yarn link
# You should expect the message: success / registered dicom-microscopy-viewer
# From SLIM's root folder run
yarn link dicom-microscopy-viewer

From this point on SLIM's should be linked.

Then just run yarn start from SLIM's root folder and you are good to go.

@CPBridge
Copy link

Hi @igoroctaviano, I have tried to run this on my machine with the relevant branches of slim and the dicom-microscopy-viewer via yarn link but unfortunately I am not seeing any polygons I'm afraid. I'm not sure what would be the most useful way for me to help debug, please let me know.

Separately, I found that I was not able to use the docker-compose in its updated form. Specifically I had to revert the ldap image tag back to dcm4che/slapd-dcm4chee:2.6.0-26.0 otherwise I got a 500 error whenever I tried to add something to the container on my linux machine (which is very similar to a problem that @DanielaSchacherer was experiencing under I think slightly different circumstances). Obviously this is not the place to debug this issue but I am wondering what the reason for the version bump in this PR was and whether it could be avoid.

@igoroctaviano
Copy link
Collaborator Author

Hi @igoroctaviano, I have tried to run this on my machine with the relevant branches of slim and the dicom-microscopy-viewer via yarn link but unfortunately I am not seeing any polygons I'm afraid. I'm not sure what would be the most useful way for me to help debug, please let me know.

Separately, I found that I was not able to use the docker-compose in its updated form. Specifically I had to revert the ldap image tag back to dcm4che/slapd-dcm4chee:2.6.0-26.0 otherwise I got a 500 error whenever I tried to add something to the container on my linux machine (which is very similar to a problem that @DanielaSchacherer was experiencing under I think slightly different circumstances). Obviously this is not the place to debug this issue but I am wondering what the reason for the version bump in this PR was and whether it could be avoid.

Let's schedule a pair to figure this out. I'll reach out via Slack.

@CPBridge
Copy link

CPBridge commented Oct 2, 2023

Update: Igor and I met this afternoon to debug why I was unable to view the annotations. We found that it was related to the way that my local environment was set up not correctly for the bulkdata retrieval from the archive, and as such the annotations were not displaying. We were able to workaround the issue (although a more permanent solution to achieve a convenient testing environment is something that probably needs more thought) and I was able to display the annotations correctly.

However, there are also some pretty severe performance issues at low magnification (when many annotations are visible) that will probably need to be addressed before this could be seriously used (even when they are rendered as points at low magnification). We brainstormed some ideas to tackle this, but this probably warrants some discussion in a future meeting. Specifically:

  • Zooming in to highest magnification when a user first toggles on the annotations. This "hides" the performance issues at low magnification by giving the browser time to render the large numbers of polygons. This is how segmentations work currently.
  • Grouping/aggregating annotations at low magnifications
  • Replacing the logic for calculating centroids (not a trivial mathematical operation) with one that does something simpler (e.g. just chooses a point), since at low magnifications the difference is completely negligible (assuming polygons are small).

fyi @fedorov @dclunie

@igoroctaviano
Copy link
Collaborator Author

Update: Igor and I met this afternoon to debug why I was unable to view the annotations. We found that it was related to the way that my local environment was set up not correctly for the bulkdata retrieval from the archive, and as such the annotations were not displaying. We were able to workaround the issue (although a more permanent solution to achieve a convenient testing environment is something that probably needs more thought) and I was able to display the annotations correctly.

However, there are also some pretty severe performance issues at low magnification (when many annotations are visible) that will probably need to be addressed before this could be seriously used (even when they are rendered as points at low magnification). We brainstormed some ideas to tackle this, but this probably warrants some discussion in a future meeting. Specifically:

  • Zooming in to highest magnification when a user first toggles on the annotations. This "hides" the performance issues at low magnification by giving the browser time to render the large numbers of polygons. This is how segmentations work currently.
  • Grouping/aggregating annotations at low magnifications
  • Replacing the logic for calculating centroids (not a trivial mathematical operation) with one that does something simpler (e.g. just chooses a point), since at low magnifications the difference is completely negligible (assuming polygons are small).

fyi @fedorov @dclunie

FYI I pushed a new script that addresses the CORS issues.
Now you can run the viewer with this command: yarn start:proxy

@github-actions
Copy link

github-actions bot commented Oct 3, 2023

Visit the preview URL for this PR (updated for commit 9768b3e):

https://idc-external-006--pr170-feat-polygon-bulk-an-cky4cmoo.web.app

(expires Fri, 17 May 2024 12:05:48 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 88aacecd98ba54d2f9c8d201a9444e43d1ad8307

@dclunie
Copy link

dclunie commented Oct 5, 2023

  • changing zoom/pan state when annotations are selected is undesirable - I want to be able to turn them on and off relative to what I am currently viewing (I know there is a different and distinct use case where the 1st time annotations are selected (or later, when desired), one might want to "go to where they are", so we might want to offer the user a choice) - this is a functionality choice and should not be influenced by performance factors
  • grouping/aggregating annotations at low magnifications - or perhaps not showing them at all if they are "too small", i.e., you don't see them until you zoom in "sufficiently"?
  • change centroid calculation - it makes sense to use whatever is simplest/fastest if it makes no difference to what is seen at the current zoom level (i.e., no point in doing the calculation if the result is "excessively precise" for the need).

... We brainstormed some ideas to tackle this, but this probably warrants some discussion in a future meeting. Specifically:

  • Zooming in to highest magnification when a user first toggles on the annotations. This "hides" the performance issues at low magnification by giving the browser time to render the large numbers of polygons. This is how segmentations work currently.
  • Grouping/aggregating annotations at low magnifications
  • Replacing the logic for calculating centroids (not a trivial mathematical operation) with one that does something simpler (e.g. just chooses a point), since at low magnifications the difference is completely negligible (assuming polygons are small).

@igoroctaviano
Copy link
Collaborator Author

  • changing zoom/pan state when annotations are selected is undesirable - I want to be able to turn them on and off relative to what I am currently viewing (I know there is a different and distinct use case where the 1st time annotations are selected (or later, when desired), one might want to "go to where they are", so we might want to offer the user a choice) - this is a functionality choice and should not be influenced by performance factors
  • grouping/aggregating annotations at low magnifications - or perhaps not showing them at all if they are "too small", i.e., you don't see them until you zoom in "sufficiently"?
  • change centroid calculation - it makes sense to use whatever is simplest/fastest if it makes no difference to what is seen at the current zoom level (i.e., no point in doing the calculation if the result is "excessively precise" for the need).

... We brainstormed some ideas to tackle this, but this probably warrants some discussion in a future meeting. Specifically:

  • Zooming in to highest magnification when a user first toggles on the annotations. This "hides" the performance issues at low magnification by giving the browser time to render the large numbers of polygons. This is how segmentations work currently.
  • Grouping/aggregating annotations at low magnifications
  • Replacing the logic for calculating centroids (not a trivial mathematical operation) with one that does something simpler (e.g. just chooses a point), since at low magnifications the difference is completely negligible (assuming polygons are small).

Grouping/aggregating annotations at low magnifications might be a good option.

I'm currently profiling performance with the new datasets I got from Chris, these are way bigger than the one I had. Also looks like a good time is spent instantiating geometry classes, so if we reduce instantiation by grouping we get some time back. There's still the possibility of unrelated events that might be stealing the main thread's attention or other things inside the OpenLayers lib so I'll continue the investigation. I think there's still hope given this example: https://codesandbox.io/s/sharp-wind-2tv2k?file=/src/index.js

FYI Changing from centroid to first coord didn't help much.

@Sumitkumar1331
Copy link

Update: Igor and I met this afternoon to debug why I was unable to view the annotations. We found that it was related to the way that my local environment was set up not correctly for the bulkdata retrieval from the archive, and as such the annotations were not displaying. We were able to workaround the issue (although a more permanent solution to achieve a convenient testing environment is something that probably needs more thought) and I was able to display the annotations correctly.

However, there are also some pretty severe performance issues at low magnification (when many annotations are visible) that will probably need to be addressed before this could be seriously used (even when they are rendered as points at low magnification). We brainstormed some ideas to tackle this, but this probably warrants some discussion in a future meeting. Specifically:

  • Zooming in to highest magnification when a user first toggles on the annotations. This "hides" the performance issues at low magnification by giving the browser time to render the large numbers of polygons. This is how segmentations work currently.
  • Grouping/aggregating annotations at low magnifications
  • Replacing the logic for calculating centroids (not a trivial mathematical operation) with one that does something simpler (e.g. just chooses a point), since at low magnifications the difference is completely negligible (assuming polygons are small).

fyi @fedorov @dclunie

Could you please tell me what steps did you follow to show annotation?
@CPBridge

@igoroctaviano
Copy link
Collaborator Author

Update: Igor and I met this afternoon to debug why I was unable to view the annotations. We found that it was related to the way that my local environment was set up not correctly for the bulkdata retrieval from the archive, and as such the annotations were not displaying. We were able to workaround the issue (although a more permanent solution to achieve a convenient testing environment is something that probably needs more thought) and I was able to display the annotations correctly.
However, there are also some pretty severe performance issues at low magnification (when many annotations are visible) that will probably need to be addressed before this could be seriously used (even when they are rendered as points at low magnification). We brainstormed some ideas to tackle this, but this probably warrants some discussion in a future meeting. Specifically:

  • Zooming in to highest magnification when a user first toggles on the annotations. This "hides" the performance issues at low magnification by giving the browser time to render the large numbers of polygons. This is how segmentations work currently.
  • Grouping/aggregating annotations at low magnifications
  • Replacing the logic for calculating centroids (not a trivial mathematical operation) with one that does something simpler (e.g. just chooses a point), since at low magnifications the difference is completely negligible (assuming polygons are small).

fyi @fedorov @dclunie

Could you please tell me what steps did you follow to show annotation? @CPBridge

  1. Checkout to feat/polygon-bulk-annotations branch for both slim and dicom-microscopy-viewer.
  2. Link dicom-microscopy-viewer to slim via yarn link
  3. Run dcm4chee using SLIM's docker-compose.yml (you have to use SLIM's docker-compose since it has configuration to increase memory size for Java in order to load those large bulk data annotation datasets)

@fedorov
Copy link
Member

fedorov commented Jan 25, 2024

@igoroctaviano would be great if you could share a test environment so we could further test this before merging!

@igoroctaviano
Copy link
Collaborator Author

igoroctaviano commented Jan 25, 2024

@igoroctaviano would be great if you could share a test environment so we could further test this before merging!

You can use this one:
https://slim-dmv.web.app/studies/2.25.68803095896966276583382138924964839274/series/1.3.6.1.4.1.5962.99.1.1163866303.1057408148.1637546438847.2.0

Keep in mind that it takes some time to retrieve the data after you toggle the annotation group. (Toggle the one with graphic type polygon, otherwise it won’t work)

@dclunie
Copy link

dclunie commented Feb 10, 2024

@fedorov @igoroctaviano @DanielaSchacherer @CPBridge @ahomeyer

FYI, wrt. what other viewers do with rendering high resolution polygons at lower resolution zoom, see Figure 10 of this paper [1], described as:

"Figure 10 - Nuclear segmentation and Tumor Infiltrating Lymphocytes (TILs) result sets at four different scales. a) Full WSI with heatmap of TILs b) Closer view with panning sync’d c) Even closer view, but now with nuclear segmentation results superimposed d) maximum zoom."

The discussion of multi-resolution Hilbert space-filling curves, spatial queries and the use of RDF and corresponding databases is also interesting, but not immediately germane since we (DICOM WG26) selected a Cartesian representation rather than a Hilbert curve for the coordinates (for now).

  1. Bremer E, DiPrima T, Balsamo J, Almeida J, Gupta R, Saltz J. Halcyon -- A Pathology Imaging and Feature analysis and Management System. arXiv; 2023. Available from: http://arxiv.org/abs/2304.10612 doi:10.48550/arXiv.2304.10612

(also posted to Issue #172).

@igoroctaviano igoroctaviano changed the title feat(viewer): Polygons support for bulk annotations feat(viewer): Polygon and point bulk annotations support Feb 12, 2024
@DanielaSchacherer
Copy link

DanielaSchacherer commented Feb 12, 2024 via email

@fedorov
Copy link
Member

fedorov commented Feb 22, 2024

Following the discussion today, here are some additional features that might be good to consider:

  • add controls to allow users to choose
    • whether clustering should be applied or not
    • whether polygons should be represented as polygons, or by selecting a random point from the contour

@igoroctaviano igoroctaviano changed the title feat(viewer): Polygon and point bulk annotations support feat(viewer): Polygon/Point/Ellipse/Rectangle 2D and 3D bulk annotations support Mar 14, 2024
@igoroctaviano igoroctaviano changed the title feat(viewer): Polygon/Point/Ellipse/Rectangle 2D and 3D bulk annotations support feature(viewer.js): Polygon/Point/Ellipse/Rectangle 2D and 3D bulk annotations support Mar 14, 2024
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.

None yet

6 participants