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

WebGL rendering speed #2533

Closed
thec0keman opened this issue May 15, 2024 · 30 comments · Fixed by #2537
Closed

WebGL rendering speed #2533

thec0keman opened this issue May 15, 2024 · 30 comments · Fixed by #2537
Labels

Comments

@thec0keman
Copy link

We have noticed a consistent performance difference in rendering tiles in WebGL vs. Canvas. There is a noticeable pop-in as tiles are rendered while browsing an image, especially when performing faster pan / zoom movements.

This does appear to be machine dependent, as some devices with a better GPU appear much smooth with no visible pop-in.

Just to clarify though: this is not strictly an FPS issue. If a machine has no hardware acceleration, WebGL will have significantly lower FPS than canvas. However, even with hardware acceleration, on some machines we are seeing acceptable FPS but still a fair amount of pop-in for tiles.

The other clarification is that the tile popin seems to be specifically the rendering pipeline in OSD, as the tile size is significantly smaller than the actual tiles downloaded over the network.
tile-loading

This example is a bit exaggerated from typical use cases to make the effect very apparent, but on some machines it will be very noticeable even with slower pannign.

Is this an expected artifact of using the WebGL renderer? Or is this an area that the caching overhaul PR will help with?

@pearcetm
Copy link
Contributor

Not an expected artifact. Also, there's no reason I can think of that tiles should be drawn smaller than they are downloaded. Neither TiledImage nor the WebGLDrawer is splitting images into smaller tiles as far as I know.

@pearcetm
Copy link
Contributor

Are you using the immediateRender option? It is strange that tiles from the lower-resolution levels of the pyramid aren't displaying while the full-res tiles are being drawn. Is that something you've done on purpose for demonstration, or is it just what you're seeing all the time?

@thec0keman
Copy link
Author

We haven't bee using immediateRender. We'll give that a try and report back what we find.

@pearcetm
Copy link
Contributor

My impression was that immediateRender would possibly cause the tiling behavior you're seeing. I wouldn't expect it to fix it.

@iangilman
Copy link
Member

@thec0keman Do you have a repro you could share? We're not seeing this behavior, especially the tiles popping in like that.

@thec0keman
Copy link
Author

We've been doing some scoping to try and narrow this down and I'd like to clarify some of the behavior.

First off, the tile size loading was a misunderstanding on our part. Sorry for the confusion.

Second, this is not tied to the WebGL renderer. We've switched our testing to canvas to rule that out.

There are two issues here we've identified:

  1. when panning a slide, frequently unloaded regions will appear blank rather than using a cached tile from a higher pyramid level (i.e. a blurry region). We've narrowed this down to 1bc93be from Refactor drawing code, add WebGL drawer, and enable plugin renderers #2310

  2. tile rendering in general seems slower. We don't have objective measures on this, but with the same tile server / response times, there is a noticeable different in tile rendering speed. We're still trying to track down something more objective here, since the prior issue can make this seem worse.

Have there been any changes into how tiles are loaded? For example, between 2.4 and master, is it first-in-first-out? We were wondering if there was a change in that range so that more offscreen tiles may be loading ahead of onscreen...

@pearcetm
Copy link
Contributor

Thanks for helping track this down @thec0keman. Can you try swapping the order of the first two parameters to _updateTile here:

var result = this._updateTile(
drawLevel,
haveDrawn,

I seem to recall that at one point during this process of this huge refactoring that these parameters were being called in the wrong order, which I fixed, but may have un-fixed somehow inadvertently. In the current code, the order in which they are passed in does not match the order they are expected in the function signature. I could see this potentially being the problem...

Fixing the first problem may help the second one too. If not, though, we can hopefully track that down next.

@pearcetm
Copy link
Contributor

I was able to reproduce this issue and locate the cause. It should be fixed by #2537.

@thec0keman I'd be interested to know if the rendering speed decrease you're seeing is fully addressed by this patch, or if there's still something more to chase down.

@iangilman iangilman added bug and removed question labels May 21, 2024
@antond
Copy link

antond commented May 21, 2024

Thanks for following up and working on the issue @pearcetm. We tested #2537 in our setup and posted our findings related to the bug there. Unfortunately, it did not solve the tile rendering speed issue discussed here.

However, the higher priority issue for us is, again, the tile rendering speed. We did a comparison of latest v5 code (f3a942c) with OSD 2.4.2 on the same image, the same magnification level, on the same code and same OSD settings, and took the following videos:

  • OSD v5 (f3a942c) in debug mode is significantly slower than v2.4.2 below, and here are some of the possibly related observations:
    1. OSD seems to no longer be loading the higher pyramid levels while panning? Is this a config change that we missed during the upgrade from OSD 2.4.2?
    2. It seems that some user interactions cause tile loads to delay. Is OSD v5 trying to do some type of predictive/off-screen tile loading?
    3. Why does it seem that tiles closer to the edges render at higher priority compared to the tiles in the middle of the viewport?
    4. The issue is more obvious with images consisting of smaller tiles (as in the attached videos).
OSD5_f3a942c5224012d9bfca2ba4bef71796473cb60dslow_tile_loading_debug_mode_on.mp4
  • OSD v2.4.2 in debug mode is markedly faster in tile rendering than v5 above:
    • Larger tiles would be used to quickly fill in the viewer with reasonably sharp images, and then it would fetch higher resolution tiles afterwards. This is what we’d like to return to in v5 as well.
OSD242_tile_loading_speed_debug_mode_on.mp4

@pearcetm
Copy link
Contributor

  • OSD seems to no longer be loading the higher pyramid levels while panning? Is this a config change that we missed during the upgrade from OSD 2.4.2?

This should be fixed by #2537. The tile sorting was introduced in #2387 and inadvertently sorted tiles from highest to lowest resolution instead of lowest to highest. As noted in #2537 I am not sure why the canvas drawer is not working correctly with this fix, but the webgl drawer appears to work correctly for me. If you could test and confirm that would be helpful.

  • It seems that some user interactions cause tile loads to delay. Is OSD v5 trying to do some type of predictive/off-screen tile loading?

I don't think there's anything predictive or off-screen happening - at least not intentionally. It is possible that the logic in #2387 is impacting this somehow during user navigation I suppose.

  • Why does it seem that tiles closer to the edges render at higher priority compared to the tiles in the middle of the viewport?

On my system, tiles are loading appropriate from the center outward, regardless of whether canvas or webgl is used for the drawer. Can you look further into the actual order they are being requested?

@pearcetm
Copy link
Contributor

@antond I think the rendering speed decrease you're referring to is predominantly related to the bug where the lower-res tiles aren't prioritized. Again, this should be fixed by #2537 (which now should work in both drawers). I'd be interested to know whether, after the low-res tiles are prioritized, you still see tiles near the edges being prioritized somehow in the highest-res levels. This shouldn't be the case, but perhaps there's something else going on still.

@actsasbuffoon
Copy link

This has definitely improved things, but we're still seeing an edge case. So here's the fix working correctly:

osd_fix_non_exact_zoom.mp4

It's doing a great job at loading larger tile levels to quickly fill the screen with sharper images before going back for high resolution images. It's also clearly working on the inner part of the image first.

But here's what happens when we zoom in further:

osd_fix_exact_zoom.mp4

It seems to go back to just grabbing one size of tile over and over again, which means it's taking longer to fill the screen with mostly clear images.

Here's the old version of OSD that we were using (2.4.2) running at the same zoom level:

osd.2.4.2.good.mp4

2.4.2 was still filling the screen with larger tiles on the first pass, then coming back for higher res tiles afterwards. So while this patch gets us a lot closer to the old behavior, it's still not quite there.

I should also note that this issue is more noticeable when the tiles themselves are small. In this case we're working with 256x256 tiles. We have some images with tiles at 512x512, and they fill the OSD viewer quickly enough that this isn't really a problem. But at 256x256, I think it has to request and paint such a large number of tiles that it just can't keep up without those intermediate tile sizes being drawn first.

Thanks for all of your help. We're very impressed by the responsiveness of the maintainers of this project.

@pearcetm
Copy link
Contributor

This has definitely improved things,

Glad we're making progress here! The failure to prioritize lower-res/higher-coverage tiles was definitely an inadvertent bug.

But here's what happens when we zoom in further... It seems to go back to just grabbing one size of tile over and over again

I'm puzzled by this. The logic shouldn't be different whether the image is zoomed all the way in or not.

I did some testing in my browser's debugger. Can you try the same to see what you get? Here's what I did:

// assumes global variable 'viewer'

// immediately zoom way in to the upper left corner of your image (avoids fetching intermediate-res tiles during animation)
// adjust your width/height values as needed to get appropriately sized tiles in your viewport
viewer.viewport.fitBounds(new OpenSeadragon.Rect(0, 0, 0.005, 0.005), true);

// add a debug message to print the jobs being added to the image loader
viewer.imageLoader.addJobOrig = viewer.imageLoader.addJob;
viewer.imageLoader.addJob=function(e){
    t=e.tile;
    console.log(`Loading level ${t.level}, x=${t.x}, y=${t.y}`);
    return this.addJobOrig(e);
}

// "pan" to a new area at the same zoomed-in magnification
viewer.viewport.fitBounds(new OpenSeadragon.Rect(0.1, 0, 0.005, 0.005), true);

Output:

Loading level 5, x=2, y=0
Loading level 5, x=3, y=0
Loading level 6, x=5, y=0
Loading level 6, x=6, y=0
Loading level 7, x=11, y=0
Loading level 7, x=10, y=0
Loading level 7, x=12, y=0
Loading level 8, x=22, y=0
Loading level 8, x=23, y=0
Loading level 8, x=22, y=1
Loading level 8, x=23, y=1
Loading level 8, x=21, y=0
Loading level 8, x=21, y=1
Loading level 8, x=24, y=0
Loading level 8, x=24, y=1
Loading level 9, x=45, y=1
Loading level 9, x=45, y=0
Loading level 9, x=44, y=1
Loading level 9, x=46, y=1
Loading level 9, x=44, y=0
Loading level 9, x=46, y=0
Loading level 9, x=45, y=2
Loading level 9, x=44, y=2
Loading level 9, x=46, y=2
... additional level 9 tiles

To me, this looks like it is doing what we expect - on panning to a new area, it first loads the lower level tiles, then the higher level ones. Are you seeing the same thing? Of course, this is just the order in which the tiles are requested - not necessarily the order that they are returned by the network (which is when they get drawn).

So while this patch gets us a lot closer to the old behavior, it's still not quite there.

The idea wasn't to change the old behavior... and I'm not sure what if anything has actually changed, since the tiles do seem to be requested in the correct order. The more detail you can provide the easier this will be to sort out, so thanks for continuing to dig into it!

I should also note that this issue is more noticeable when the tiles themselves are small. In this case we're working with 256x256 tiles. We have some images with tiles at 512x512, and they fill the OSD viewer quickly enough that this isn't really a problem. But at 256x256, I think it has to request and paint such a large number of tiles that it just can't keep up without those intermediate tile sizes being drawn first.

The intermediate tiles should be drawn first (as long as they're downloaded first). However, you may be running into visual effects of the default implementation of only trying to fetch a single new tile per animation frame. You could try using the new maxTilesPerFrame option which was introduced in #2387 in order to request the tiles faster. You'll probably have to play around with the value here; too high and you'll cause more problems than you solve. But it might help to set it higher than the default of 1.

@iangilman
Copy link
Member

@actsasbuffoon Thank you for continuing to test the patches!

@pearcetm Thank you for hunting this down! It's possible some of my most recent comments in #2537 could point to some of the issues being seen.

@actsasbuffoon
Copy link

@pearcetm Thanks for the debugging code; that's quite helpful!

The image I'm testing with only has 7 levels, but I can confirm that once I zoom in enough, only level 7 tiles are requested. Your debugging code shows a variety of tile sizes when I zoom out, but once I get close, it only requests level 7.

Here's an example where I zoom in and pan around. All of the tile requests are at level 7:

tile_loading_2

@pearcetm
Copy link
Contributor

@actsasbuffoon I can only get the viewer to do this (i.e. only request the highest level while panning) when the viewer has immediateRender: true. Do you have that enabled by any chance? If you turn it off, does it act as you expect and like what I am seeing?

@pearcetm
Copy link
Contributor

This is what the docs say about immediateRender (bolding mine):

immediateRender | Boolean | | false | Render the best closest level first, ignoring the lowering levels which provide the effect of very blurry to sharp. It is recommended to change setting to true for mobile devices.

@actsasbuffoon
Copy link

We don't set immediateRender, and it looks like it should be false by default. Just to be certain, I explicitly set it to false, and I'm still seeing the same behavior of only requesting level 7 tiles.

This is very odd.

@pearcetm
Copy link
Contributor

@actsasbuffoon Very odd indeed. Not being able to reproduce this makes it harder to debug...

Can you run the following in your console to make sure the level interval is being calculated correctly?

tiledImage = viewer.world.getItemAt(0);
tiledImage._getLevelsInterval();

When I do this, I get the following, which defines the levels that should be getting updated on each frame:

{lowestLevel: 0, highestLevel: 8}

@pearcetm
Copy link
Contributor

And can you confirm that tiledImage.immediateRender returns false, just to be extra certain this isn't getting set inadvertently to true on the tiled image somehow?

@actsasbuffoon
Copy link

Yeah, certainly! Here's the result:

{lowestLevel: 0, highestLevel: 7}

And tiledImage.immediateRender returns false.

Here are some other settings (gathered by interrogating the viewer object in the console) in case this helps:

maxTilesPerFrame: 5
minPixelRatio: 0.4
minZoomImageRatio: 0.9
normHeight: 1
smoothTileEdgesMinZoom: 1.1

Our maxTilesPerFrame setting looks pretty aggressive based on the earlier part of this discussion where it was mentioned that the default is 1. We may have recently bumped that up while trying to work around this issue, but I'm not certain. Either way, I suppose it wouldn't explain the debug logging saying that we're only requesting level 7 tiles when zoomed in.

@pearcetm
Copy link
Contributor

Using all of those settings, I am still seeing the following when I am zoomed all the way in and do a quick pan:
image

Can you share your entire viewer settings object? Is there any way you could link to a page that shows this issue?

@pearcetm
Copy link
Contributor

The "visibility" value for a level also affects how the tiles are sorted. Can you use the following to print those values out?

tiledImage.updateLevelOrig = tiledImage._updateLevel;
tiledImage._updateLevel = function(level,a,visibility,b,c,d){
    console.log(`Updating level ${level}, visibility=${visibility}`);
    return this.updateLevelOrig(level,a,visibility,b,c,d);
}

@actsasbuffoon
Copy link

Ah, that's interesting!

Here's what I get when panning around and I'm zoomed in enough that only level 7 tiles are requested:

Updating level 7, visibility=1.0158730158730158
Updating level 6, visibility=1.0158730158730158
Updating level 5, visibility=1.032258064516129
Updating level 4, visibility=1.0666666666666667
Updating level 3, visibility=1.142857142857143
Updating level 2, visibility=1.3333333333333333
Updating level 1, visibility=2

@pearcetm
Copy link
Contributor

Ok, so it is updating all the levels, as it should...

It is interesting that your visibility value for level 6 and level 7 are identical. That's not the case for the tile source I'm using. This is calculated from TileSource.getPixelRatio(level). It is strange that two levels would give the same pixel ratio. You may want to look into your tile source further (not sure what I expect, though...)

@iangilman
Copy link
Member

Forgive me if this has already been asked, but @actsasbuffoon is it possible for you to share your tilesource with @pearcetm ? Sounds like that would make a difference in tracking this down.

@actsasbuffoon
Copy link

@iangilman @pearcetm Okay, I've been looking into this for quite a while, and I have excellent news.

The incorrect behavior we were seeing at high zoom levels was the result of an override we had for getLevelScale. I've fixed that problem, and combined with this patch, we're looking good now.

Thank you so much, not just for the patch, but also for the guidance and debugging advice along the way. That proved crucial when identifying the root cause on our side. Everyone here really appreciates all of the prompt, knowledgeable help from the OSD team.

@thec0keman
Copy link
Author

@pearcetm thank you for the quick turnaround on this! We are going to be continuing testing, but all indications so far is this fully resolves the issues we were running into.

@pearcetm
Copy link
Contributor

@actsasbuffoon @thec0keman Excellent, glad it seems to be sorted out!

@iangilman
Copy link
Member

Wonderful! I'm glad the cause has been discovered!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants