ImagingDataCommons / slim

Interoperable web-based slide microscopy viewer and annotation tool
https://imagingdatacommons.github.io/slim/
Apache License 2.0
111 stars 36 forks source link

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

Open igoroctaviano opened 9 months ago

igoroctaviano commented 9 months ago

Related PR: https://github.com/ImagingDataCommons/dicom-microscopy-viewer/pull/104

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

Testing

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 commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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:

fyi @fedorov @dclunie

igoroctaviano commented 9 months ago

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[bot] commented 9 months ago

Visit the preview URL for this PR (updated for commit 90140df):

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

(expires Wed, 03 Jul 2024 15:11:01 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 88aacecd98ba54d2f9c8d201a9444e43d1ad8307

dclunie commented 9 months ago

... 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 commented 9 months ago
  • 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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

igoroctaviano commented 5 months ago

@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 commented 4 months ago

@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).

DanielaSchacherer commented 4 months ago

[like] Schacherer, Daniela reacted to your message:


From: David Clunie @.> Sent: Saturday, February 10, 2024 1:52:08 PM To: ImagingDataCommons/slim @.> Cc: Schacherer, Daniela @.>; Mention @.> Subject: Re: [ImagingDataCommons/slim] feat(viewer): Polygons support for bulk annotations (PR #170)

@fedorovhttps://github.com/fedorov @igoroctavianohttps://github.com/igoroctaviano @DanielaSchachererhttps://github.com/DanielaSchacherer @CPBridgehttps://github.com/CPBridge @ahomeyerhttps://github.com/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 #172https://github.com/ImagingDataCommons/slim/issues/172).

— Reply to this email directly, view it on GitHubhttps://github.com/ImagingDataCommons/slim/pull/170#issuecomment-1937012391, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACVDZQLFBQZ2O4MZLDPNABTYS53QRAVCNFSM6AAAAAA4VGAS6KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZXGAYTEMZZGE. You are receiving this because you were mentioned.Message ID: @.***>

fedorov commented 4 months ago

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