Bug 228012 - REGRESSION(ANGLE+METAL): WebGL2 content low frame rate
Summary: REGRESSION(ANGLE+METAL): WebGL2 content low frame rate
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: Safari Technology Preview
Hardware: iPhone / iPad iOS 14
: P2 Normal
Assignee: Kyle Piddington
URL: https://noclip.website/#smg/DarkRoomG...
Keywords: InRadar
Depends on: 227059
Blocks: 228192
  Show dependency treegraph
 
Reported: 2021-07-15 18:06 PDT by Jasper St. Pierre
Modified: 2021-07-22 17:56 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.80 KB, patch)
2021-07-16 17:29 PDT, Kyle Piddington
no flags Details | Formatted Diff | Diff
Patch (1.84 KB, patch)
2021-07-19 17:32 PDT, Kyle Piddington
no flags Details | Formatted Diff | Diff
Patch (1.81 KB, patch)
2021-07-19 17:41 PDT, Kyle Piddington
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jasper St. Pierre 2021-07-15 18:06:15 PDT
URL:  https://noclip.website/#smg/DarkRoomGalaxy;ShareData=AZju&UDsRrT3IyH97Uy9=3k]%5eQY7VbUi%29BDT:JHOVvKe=UwLZ+873GrUDmZ%29=2L

According to a user of mine, this runs fine on iOS 14 (60 fps), but now runs poorly on iOS 15 Beta (10 fps).

The user was on an iPhone 12 Pro.
Comment 1 Jasper St. Pierre 2021-07-15 18:32:15 PDT
According to the same user:

1. The low performance happens in many different scenes, not just the link provided.
2. The performance seems to act strangely -- e.g. the more things that appear on the screen, the *faster* the application seems to be.
3. This *only* happens in portrait mode. Landscape mode performs just fine.
Comment 2 Sam Sneddon [:gsnedders] 2021-07-16 03:30:51 PDT
Performance is fine with the Metal backend disabled via experimental features, so regression is from enabling it.

To note: at least in the above case, this isn't per-se a regression in iOS 15, given WebGL 2 wasn't enabled in iOS 14. Might affect other WebGL content, however.
Comment 3 Radar WebKit Bug Importer 2021-07-16 03:31:03 PDT
<rdar://problem/80677758>
Comment 4 Kenneth Russell 2021-07-16 11:46:17 PDT
This might have been addressed by Bug 227059. Kyle or other Apple engineers, possible for you to test on iOS with top-of-tree?
Comment 5 Kyle Piddington 2021-07-16 12:15:18 PDT
Repo'd, I'm seeing most of the slowdown happening with a 'bufferSubData' call. As Jasper reports, landscape mode performs just fine, oddly enough.
Comment 6 Kyle Piddington 2021-07-16 14:00:53 PDT
Unfortunately, this has not been addressed by Bug 227059. I'm looking into it, however. Updates soon :).

I suspect this is some sort of run-ahead issue, but now we seem to be falling behind rather than running ahead.
Comment 7 Jasper St. Pierre 2021-07-16 15:46:01 PDT
The bufferSubData is likely uploading my giant UBO for the frame. Either that or vertex data for some dynamic draws. All uploads happening every frame should be marked GL_DYNAMIC_DRAW (which I believe should be correct? Can never remember the difference between that and GL_STREAM_DRAW), and I try to keep UBO data limited to 65536-sized buffer pages.

If there's anything I'm doing wrong on my side, please let me know.
Comment 8 Kyle Piddington 2021-07-16 17:29:32 PDT
Created attachment 433720 [details]
Patch
Comment 9 Kyle Piddington 2021-07-16 18:17:47 PDT
(In reply to Jasper St. Pierre from comment #7)
> The bufferSubData is likely uploading my giant UBO for the frame. Either
> that or vertex data for some dynamic draws. All uploads happening every
> frame should be marked GL_DYNAMIC_DRAW (which I believe should be correct?
> Can never remember the difference between that and GL_STREAM_DRAW), and I
> try to keep UBO data limited to 65536-sized buffer pages.
> 
> If there's anything I'm doing wrong on my side, please let me know.

While I'm working on a patch, the best thing you can do for performance is limit the number of draw calls you submit. Try to keep submissions around 60 FPS, and you should see a performance improvement.
Comment 10 Jasper St. Pierre 2021-07-16 18:24:59 PDT
Hmm, I do try to rely on requestAnimationFrame to manage my scheduling. I should be submitting around 200 draw calls in that link provided, which should hopefully be on the low side. It's possible there's a bug in my animation scheduling.

If that doesn't match what you're seeing, I'd be happy to hear what might be going wrong, or how else I can rate-limit my draw calls submission.
Comment 11 Dean Jackson 2021-07-18 12:09:26 PDT
Comment on attachment 433720 [details]
Patch

Do we need to do this for macOS too?
Comment 12 Kyle Piddington 2021-07-19 17:29:47 PDT
(In reply to Jasper St. Pierre from comment #10)
> Hmm, I do try to rely on requestAnimationFrame to manage my scheduling. I
> should be submitting around 200 draw calls in that link provided, which
> should hopefully be on the low side. It's possible there's a bug in my
> animation scheduling.
> 
> If that doesn't match what you're seeing, I'd be happy to hear what might be
> going wrong, or how else I can rate-limit my draw calls submission.

We definitely have a problem with our fencing code, though I'm able to address that now. That patch will 

One thing I'm interested in is your render target size when the iPhone is in portrait vs Landscape mode. I suspect we're getting a significantly larger render target.

This patch will address our bugs in the fencing code, so we don't run-ahead too much.
Comment 13 Kyle Piddington 2021-07-19 17:32:08 PDT
Created attachment 433837 [details]
Patch
Comment 14 Kyle Piddington 2021-07-19 17:41:07 PDT
Created attachment 433839 [details]
Patch
Comment 15 Kyle Piddington 2021-07-20 11:27:28 PDT
(In reply to Kyle Piddington from comment #12)
> (In reply to Jasper St. Pierre from comment #10)
> > Hmm, I do try to rely on requestAnimationFrame to manage my scheduling. I
> > should be submitting around 200 draw calls in that link provided, which
> > should hopefully be on the low side. It's possible there's a bug in my
> > animation scheduling.
> > 
> > If that doesn't match what you're seeing, I'd be happy to hear what might be
> > going wrong, or how else I can rate-limit my draw calls submission.
> 
> We definitely have a problem with our fencing code, though I'm able to
> address that now. That patch will 
> 
> One thing I'm interested in is your render target size when the iPhone is in
> portrait vs Landscape mode. I suspect we're getting a significantly larger
> render target.
> 
> This patch will address our bugs in the fencing code, so we don't run-ahead
> too much.


One last thing: Locally, I'm seeing more draw detail with the Metal backend. For example, in the Dark Room Galaxy sample, enabling and disabling the metal backend toggles whether or not the floor and floating pillars are textured. Could that account for the performance change?
Comment 16 Dean Jackson 2021-07-20 11:48:34 PDT
Comment on attachment 433839 [details]
Patch

This looks good but I think it might be in danger of breaking on future OS releases.
Comment 17 Kenneth Russell 2021-07-20 14:52:09 PDT
Thanks Kyle for tracking down this tricky sandboxing issue that was affecting the fence implementation!
Comment 18 EWS 2021-07-20 16:38:01 PDT
Committed r280114 (239831@main): <https://commits.webkit.org/239831@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433839 [details].
Comment 19 Jasper St. Pierre 2021-07-22 17:11:48 PDT
Should I open a separate bug for the landscape vs. portrait issue, or should this be reopened to investigate that?
Comment 20 Kyle Piddington 2021-07-22 17:39:34 PDT
(In reply to Jasper St. Pierre from comment #19)
> Should I open a separate bug for the landscape vs. portrait issue, or should
> this be reopened to investigate that?

Let's open a separate bug. 
Can you confirm with your user that the scenes are identical on iOS14 and iOS 15?
In addition, could you check the render target sizes on Landscape vs Portrait? I suspect we're rendering more in Portrait mode.
Comment 21 Jasper St. Pierre 2021-07-22 17:41:25 PDT
I don't know any way to easily check the render target sizes. Would that be the drawingBufferWidth / drawingBufferHeight on the WebGL render context? I attempt to resize the canvas to fit the window content.
Comment 22 Jasper St. Pierre 2021-07-22 17:43:49 PDT
Also, I filed https://bugs.webkit.org/show_bug.cgi?id=228208 for the portrait vs. landscape issue.
Comment 23 Kenneth Russell 2021-07-22 17:56:59 PDT
Yes, drawingBufferWidth/drawingBufferHeight are the real pixel sizes of WebGL's back buffer.