Bug 228208

Summary: WebGL 2 performance differs in landscape and portrait mode
Product: WebKit Reporter: Jasper St. Pierre <jstpierre>
Component: WebGLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, kbr, kkinnunen, kpiddington
Priority: P2    
Version: Safari Technology Preview   
Hardware: iPhone / iPad   
OS: iOS 14   

Jasper St. Pierre
Reported 2021-07-22 17:43:16 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 poorly in portrait mode, while it runs fine in landscape mode. The user was on an iPhone 12 Pro.
Attachments
Jasper St. Pierre
Comment 1 2021-07-22 17:43:33 PDT
Kenneth Russell
Comment 2 2021-07-22 17:58:04 PDT
Just to get the OS versions correct - is this the Safari 15 beta, on iOS 15?
Jasper St. Pierre
Comment 3 2021-07-22 18:00:31 PDT
Yes, this user was on the iOS 15 Beta. The OS selection field does not list iOS 15 as an option, so I selected the closest equivalent.
Kyle Piddington
Comment 4 2021-07-22 18:45:50 PDT
I did take a look with ToT Safari, which includes some important fixes . I do see the a landscape vs portrait performance regression. I suspect the render target size is larger in landscape mode than it is in portrait.
Kenneth Russell
Comment 5 2021-07-22 19:07:06 PDT
Thanks for confirming. Kyle, would it be possible for you to add logging to your iOS build when the WebGL rendering context resizes and tell us what drawingBufferWidth / drawingBufferHeight are in portrait and landscape mode for these examples?
Jasper St. Pierre
Comment 6 2021-07-22 19:12:17 PDT
I have updated noclip.website so that the "Statistics" panel on the left shows the GL drawing buffer width and height for easy verification. If you would like any more information on display, I can provide that to you as long as it's provided through some web-accessible API.
Kyle Piddington
Comment 7 2021-07-23 10:34:33 PDT
(In reply to Kenneth Russell from comment #5) > Thanks for confirming. Kyle, would it be possible for you to add logging to > your iOS build when the WebGL rendering context resizes and tell us what > drawingBufferWidth / drawingBufferHeight are in portrait and landscape mode > for these examples? Thanks to Jasper's updates, these were pretty simple to get! Portrait 2948x5682 Landscape: 2940x1506 Portrait mode is doing almost 3x the rendering work. For reference, this was captured on an iPhone XS Max, with an effective screen resolution of 1242x2688 (via https://support.apple.com/kb/SP780?locale=en_US).
Jasper St. Pierre
Comment 8 2021-07-23 11:25:19 PDT
Do you suspect this is an issue with my content, or a bug on the WebKit side? I do take devicePixelRatio into account when resizing the canvas, and I currently use window.innerWidth / window.innerHeight, which is possibly the wrong measurement. https://github.com/magcius/noclip.website/blob/3101e0babfa134a854635f16700c4775fc0e5666/src/viewer.ts#L74-L83 https://github.com/magcius/noclip.website/blob/3101e0babfa134a854635f16700c4775fc0e5666/src/main.ts#L474-L476 If there's a better way to handle this, please let me know.
Kenneth Russell
Comment 9 2021-07-23 12:23:18 PDT
I'm not sure but have you configured the viewport properly for mobile? https://developer.apple.com/library/archive/documentation/AppleApplications/Reference/SafariWebContent/UsingtheViewport/UsingtheViewport.html#//apple_ref/doc/uid/TP40006509-SW1 also see https://webkit.org/blog/7929/designing-websites-for-iphone-x/ Maybe because the initial-scale isn't configured, or similar, then the window's receiving a very large size in portrait orientation.
Kyle Piddington
Comment 10 2021-07-23 15:58:31 PDT
I'm in agreement with Kenneth here. Via the webkit inspector, it looks like your canvas size is by default (on my iPhone XS Max), 982x1894. This is almost double the screen size. Something's not quite right with how you're measuring, or creating the initial canvas size.
Kyle Piddington
Comment 11 2021-07-23 15:59:56 PDT
http://mydevice.io seems to report the closer-to-correct CSS screen size of 414x896.
Jasper St. Pierre
Comment 12 2021-07-23 16:09:48 PDT
Interesting. I'll try and make some adjustments to how the viewport and see if that improves things. I don't currently have a device to test with so I'll be stabbing a bit in the dark here. Thanks for all the help!
Kyle Piddington
Comment 13 2021-07-23 16:17:07 PDT
(In reply to Jasper St. Pierre from comment #12) > Interesting. I'll try and make some adjustments to how the viewport and see > if that improves things. I don't currently have a device to test with so > I'll be stabbing a bit in the dark here. Thanks for all the help! Best I can tell from the minified JS, it looks like you insert your canvas programmatically. I'd check there as a starting point for what's going wonky. Whatever you do seems to work on desktop correctly, which is probably how we missed this in the first place.
Jasper St. Pierre
Comment 14 2021-07-23 16:27:14 PDT
Correct. I create a canvas dynamically, and effectively resize it with: canvas.width = window.innerWidth * window.devicePixelRatio; canvas.height = window.innerHeight * window.devicePixelRatio; canvas.style.width = `${window.innerWidth}px`; canvas.style.height = `${window.innerHeight}px`; So I think the question is how the window's inner width and height change for portrait vs. landscape. I just inserted a <meta viewport> tag which might improve the sizes of things, at the expense of probably making the UI unusable. The core of the issue is probably that without a viewport specification, portrait was rendering a "zoomed-out" view of the page, which has a large canvas. Curious that the old WebGL implementation didn't seem to have this issue though.
Kyle Piddington
Comment 15 2021-07-23 16:38:48 PDT
(In reply to Jasper St. Pierre from comment #14) > Correct. I create a canvas dynamically, and effectively resize it with: > > canvas.width = window.innerWidth * window.devicePixelRatio; > canvas.height = window.innerHeight * window.devicePixelRatio; > canvas.style.width = `${window.innerWidth}px`; > canvas.style.height = `${window.innerHeight}px`; > > So I think the question is how the window's inner width and height change > for portrait vs. landscape. I just inserted a <meta viewport> tag which > might improve the sizes of things, at the expense of probably making the UI > unusable. > > The core of the issue is probably that without a viewport specification, > portrait was rendering a "zoomed-out" view of the page, which has a large > canvas. Curious that the old WebGL implementation didn't seem to have this > issue though. For what it's worth, I did run this demo on iOS 14 with the old WebGL implementation by enabling WebGL2 experimental mode. The render size was the same, but textures for most standard geometry were loaded. I suspect we were just performing less rendering work on these scenes.
Kyle Piddington
Comment 16 2021-07-23 16:50:28 PDT
With your new <Viewport> tag, we're running at a smooth frame rate! You are correct that the UI is a little shot, but it's not entirely bad. Text is now readable, for instance, where before it was rendering at such a small resolution that I had to take a screenshot and zoom in. With your permission, can I mark this as resolved?
Jasper St. Pierre
Comment 17 2021-07-23 16:55:57 PDT
OK, I'll trust you on that! Thank you for all the help! I'll report a new bug if users report any other issues.
Kyle Piddington
Comment 18 2021-07-23 17:02:33 PDT
(In reply to Jasper St. Pierre from comment #17) > OK, I'll trust you on that! Thank you for all the help! I'll report a new > bug if users report any other issues. Thanks Jasper. As a heads up, I am hitting a page crash with the Sen's Fortress scene, where it seems that we're loading enough data to hit the high-memory watermark on iOS Safari. I suspect we'll see a few scenes like that for the more modern games.
Jasper St. Pierre
Comment 19 2021-07-23 17:13:07 PDT
Ah, that's likely because I have to decompress the BC1 textures and all mips to RGBA8 which takes up quite a lot of additional texture memory, since I don't believe BC1 is supported on iOS. I remember crashing on some Dark Souls maps on PC before I added support for WEBGL_compressed_texture_s3tc. I might be able to do some memory improvements to the BC1 decoder right now as well; thank you for the heads up, and I'll add it to my list of things to investigate.
Kenneth Russell
Comment 20 2021-07-23 17:33:26 PDT
Great work Kyle and Jasper figuring this out! Jasper, this might be a great time to try Basis Universal. :D You'd get portable assets that would transcode to ASTC on iOS' new WebGL implementation.
Note You need to log in before you can comment on or make changes to this bug.