OHIF / Viewers

OHIF zero-footprint DICOM viewer and oncology specific Lesion Tracker, plus shared extension packages
https://docs.ohif.org/
MIT License
3.36k stars 3.38k forks source link

2D MPR not working on Safari #2005

Closed dma123 closed 1 year ago

dma123 commented 4 years ago

I am having issues with the 2D MPR functionality in Safari: Screen Shot 2020-08-24 at 4 48 58 pm

The same study loads perfectly fine if I use Chrome: Screen Shot 2020-08-24 at 4 49 05 pm

I am using Safari 13.1.2 (14609.3.5.1.5). I built the viewer using the current master build (ohif/viewer@4.5.1).

pieper commented 4 years ago

Thanks for the report. I can reproduce this on Safari Version 13.1.2 (15609.3.5.1.3) using this link:

https://viewer.ohif.org/viewer/1.3.6.1.4.1.25403.345050719074.3824.20170125113417.1

@fedorov this also occurs in the IDC viewer. I marked it as candidate and we can discuss whether it's a priority.

JamesAPetts commented 4 years ago

So after a bunch of testing I realised this was only happening on CTs, and wasn't due to strange array sizes or anything.

Data inserted in react-vtkjs-viewport had no difference between browsers, and I could get individual slices of the problematic data to render using the vtkImageSlice.

Turns out the issue is WebGL 1 and large textures:

https://github.com/Kitware/vtk-js/blob/acd940c9eb35efde4d16ffe2946ae31fe1d02507/Sources/Rendering/OpenGL/Texture/index.js#L1166

The max texture size is throttled here to stop the context from throwing errors, but this means not all of the data is being copied fully, this will happen with any large dataset.

Unfortuantely turning on Safari's experimental "Web GL 2.0" feature does not help here, as safari's implementation is far from complete.

So it looks like there is no easy fix for this. We could detect the browser being used and the volume size, and throw a warning when MPR is used or something?

pieper commented 4 years ago

Yes, it sounds like detecting the issue and displaying a warning is the best we can do here. Kind of a shame, but not really under our control.

fedorov commented 4 years ago

@pieper do we have the fix above in the IDC?

pieper commented 4 years ago

No, I don't believe anything has been implemented yet. Detecting the browser and bringing up a warning still seems like the best thing. To me the correct implementation would be to detect the issue at the lowest level (vtk.js) and propagate the information up through the layers and present it to the user at the application level. But maybe the best short-term would be just to detect Safari in the IDC viewer and generate a warning.

fedorov commented 4 years ago

I mean, did #2106 make it into the IDC fork?

pieper commented 4 years ago

Yes, at the OHIF level: https://github.com/ImagingDataCommons/Viewers/commits/idc-ohif-2020-10-15

fedorov commented 4 years ago

Thanks! @wlongabaugh FYI

sedghi commented 1 year ago

Please review the latest code in the master branch. This issue might have been resolved. If it persists, kindly reopen the issue with updated details.

Try viewer-dev.ohif.org instead of viewer.ohif.org Our viewer.ohif.org is deployed from release branch while viewer-dev.ohif.org is our master branch Read more about branch explanations here https://docs.ohif.org/development/getting-started#developing