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

Tile loading improvements, spinners and error handling #470

Merged
merged 14 commits into from
May 13, 2024

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Apr 17, 2024

This replaces (builds on) #464 and #456 and also supports movie playing waiting on image loading.

These PRs are combined to avoid conflicts, since the functionality is all related. Also makes it easier to deploy.

  • Tile loading errors are reported with message (as already tested in Tile load error warning #464)
  • A spinner is shown while tiles/planes are loaded (as already tested in Tile load error warning #464)
  • For "large" movies (between 2k x 2k and 3k x 3k) load whole planes instead of tiles (not tested yet, see Don't use tiles for non-big movies #456)
  • When playing a movie, we wait at least 250 millisecs (default delay between frames) but we wait longer if we are still waiting for the image to render from a previous update.
  • When we are playing a movie, show the mouse cursor as a spinner but don't show a spinner in the centre of the viewer as this obscures the image.

@will-moore
Copy link
Member Author

When to use tiled images???

The existing behaviour is:

  • If the server says this is a BIG/TILED image then always use tiles
  • Otherwise, if the image is large (bigger than 2k x 2k) then use tiles (but only if it's not a movie**) - changed in this PR

However, for NGFF images they are all BIG/TILED according to the server. So, they will always be viewed as tiled, even for quite small movies (may be a single tile for the whole plane for the smaller images).

So, do we want to use the server's BIG/TILED status at-all? Or do we simply want to decide based on size alone?
If we only go by size, do we want to distinguish between "large" (over 2k x 2k) and BIG (over 3k x 3k) images?

Originally, #456 was created to handle the case of https://idr.openmicroscopy.org/webclient/?show=image-4007801 which is a "large" image 2169 x 2048. We wanted to NOT load tiles for that image because the tiles could get out of sync.
However, this PR's behaviour of waiting for each time-point to load before loading the next time-point does address that issue.

@joshmoore
Copy link
Member

A few thoughts/questions looking at IDR's image 4007801:

  • can a lower resolution be used if loading is taking too long?
  • rather than blocking the view, could a subtle loading bar (e.g., under the image) be used?
  • is any pre-loading happening? it seems like a little would still be useful.
  • does "movie" also include progressing through z stacks, e.g.?

@will-moore
Copy link
Member Author

Looking for options for loading lower resolution layers in OpenLayers...
https://stackoverflow.com/questions/43538345/how-to-force-load-tiles-for-lower-resolution talks only about adding lower resolution layers to the pyramid. It mentions "preload" option:
https://openlayers.org/en/latest/examples/preload.html

I tested this, but when playing a movie (in this PR as it stands, we simply try to load the lower resolution(s) AND the correct resolution at the same time for each timepoint (as expected, it doesn't pre-load other timepoints and doesn't avoid loading full current resolution):

diff --git a/src/viewers/viewer/Viewer.js b/src/viewers/viewer/Viewer.js
index 3a1843a..21bb4ff 100644
--- a/src/viewers/viewer/Viewer.js
+++ b/src/viewers/viewer/Viewer.js
@@ -590,7 +590,7 @@ class Viewer extends OlObject {
             interactions: interactions,
-            layers: [new Tile({source: source})],
+            layers: [new Tile({source: source, preload: 1})],
             target: this.container_,

@will-moore
Copy link
Member Author

Also looked at a potential 'hack' of loading tiles from the lower resolution by updating the tile url function, but the lower resolution tiles are not rescaled to the appropriate size by OpenLayers, so the don't fill the space:

+++ b/src/viewers/viewer/source/Image.js
@@ -265,7 +265,7 @@ const OmeroImage = function(options) {
                     this.tileGrid.resolutions_.length - tileCoord[0] - 1 : 0;
                 if (this.tiled_) {
-                    url += 'tile=' + zoom  + ',' +
+                    url += 'tile=' + (zoom + 1)  + ',' +
                         tileCoord[1] + ',' + (-tileCoord[2]-1);

@will-moore
Copy link
Member Author

deployed those changes onto idr-testing

@jburel
Copy link
Member

jburel commented Apr 23, 2024

I don't see any change when playing movie across Z/T

@jburel
Copy link
Member

jburel commented Apr 23, 2024

Attempting to play https://idr-testing.openmicroscopy.org/webclient/img_detail/10647409/?dataset=11901
It took a while for the second frame to come so I stopped the movie but the spinner is still displayed in that case. I was expecting the spinner to be removed and the loading being cancelled

@will-moore
Copy link
Member Author

@jburel I think you're not seeing my changes? It took a few minutes for my deployment script on idr-testing to complete, and so I probably commented a bit prematurely above, or possibly something got cached.
It's working for me but I'll also try cache busting...

Could you give it another go?

Handling the Stop button when the plane is still loading is a bit tricky. We don't really have a way to cancel the image plane request except to undo the last Z/T increment (IF the plane hasn't loaded yet).

@jburel
Copy link
Member

jburel commented Apr 24, 2024

Much nicer thank you
have the last 2 commits been deployed too? they were pushed after this morning discussion

@pwalczysko
Copy link
Member

Much nicer thank you have the last 2 commits been deployed too? they were pushed after this morning discussion

Yes, I think so. The cursor spinner was not showing when the mouse was anywhere else but the slider. Now, on idr-testing, the cursor spinner is visible everywhere. This was I believe the point of the last two commits @jburel

@will-moore will-moore merged commit 54c3753 into ome:master May 13, 2024
1 check passed
@will-moore will-moore changed the title Movie playing waits for planes Tile loading improvements, spinners and error handling May 13, 2024
@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/omero-iviewer-step-size-for-t-z-auto-play/103449/2

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.

5 participants