Issue metadata
Sign in to add a comment
|
blue screen for ~ 1 second or more when entering WebVR presentation |
||||||||||||||||||||||||||
Issue descriptionChrome Version: 61.0.3163.57 OS: Android N Device: Pixel XL VrCore: 1.8.163477258 What steps will reproduce the problem? (1) go to https://webvr.info/samples/ (2) Select any webVR presentation (3) Press enter VR button <---- Notice a blue screen for a brief moment here. (Didn't use to appear) (4) Follow DON flow to start presentation. <--- Notice blue screen right after syncing controller before presentation starts. (5) Exit presentation (5) Immediately after exiting, re-enter presentation (which skips the DON flow) <--- Notice a longer blue screen here (~ 2 seconds) before presentation starts. What is the expected result? There didn't used to be a blue screen. What happens instead? Blue screen is part of the flow now when entering and exiting presentations.
,
Aug 22 2017
,
Aug 22 2017
I am seeing it too on a version which I synced this morning. This seem a p1 issue.
,
Aug 23 2017
I'll take this and start by figuring out when it was introduced.
,
Aug 24 2017
I don't see the issue after syncing later in the day. Still working to repro this.
,
Aug 24 2017
bisect result: [68156e68656b00d9c963c5b30463a27b9627d14d] Enable surface references by default. Excerpts from CL description: This CL switches from using surface sequences to surface references for surface lifetime management. This entails adding temporary references and using the surface reference graph starting at the top-level root surface and not adding surface sequences everywhere. ...
,
Aug 24 2017
We know when this issue started. Michael is currently the best equipped to investigate why, as time allows.
,
Aug 24 2017
cc kylechar, is this expected behavior from your change? Do blue flashes indicate we're doing something wrong?
,
Aug 24 2017
It sounds like a viz::Surface is getting deleted and whatever is behind the SurfaceDrawQuad (maybe nothing) is showing instead of the viz::Surface contents. I changed how we track the lifetime of viz::Surfaces in that CL, basically I disabled using SurfaceSequences. I don't know much about WebVR unfortunately. A couple questions: 1. Where do you generate / submit CompositorFrames for the WebVR content? 2. Were you explicitly adding SurfaceSequences anywhere? 3. Were you registering frame sink heirarchy anywhere? If you can point me at the code related to this, I'll take a look!
,
Aug 24 2017
I'm not seeing any blue flashes when entering VR, only when exiting VR the content window shows solid blue (and then only maybe 1/5 times). So something appears to be going wrong when switching between the VrCompositor and the Android CompositorView...
,
Aug 25 2017
kylechar is looking into this. I couldn't figure out what was going wrong :P
,
Aug 28 2017
I'm not seeing this on Canary, can anybody repro this on Canary/Dev? If not, then we can probable lower the priority and drop the M-62 target.
,
Aug 30 2017
I've spent some time digging into this. There is a race, related to viz::Surface lifetime, when switching from WebVR mode back to the browser mode. The race is solvable, although it's maybe the symptom of a bigger problem. I added a bunch of log statements to debug what was happening on 6P when visiting https://webvr.info/samples/03-vr-presentation.html, here is a quick summary: 1. Initially in browser mode with WebVR sample website loaded. The renderer is submitting CompositorFrames with dsf=3.5 and SurfaceId S1. The browser embeds S1. 2. Tap the "Enter VR" button and enter WebVR mode. 3. The renderer stops submitting CompositorFrames here. WebVR mode draws without submitting CompositorFrames. 4. The renderer gets an IPC to resize with dsf=1.9326. 5. Tap the X button and switch back to browser mode. 6. CompositorImpl reinitializes itself. 7. The renderer gets a signal to start submitting CompositorFrames again. 8. The browser embeds the last renderer surface which is SurfaceId S1. 9. The renderer submits a CompositorFrame with dsf=1.9326 now. This CompositorFrames has SurfaceId S2. 10. The browser embeds SurfaceId S2 with the wrong dsf. 11. The renderer gets an IPC to resize with dsf=3.5. 12. The renderer submits a CompositorFrame with dsf=3.5 now. These CompositorFrames have SurfaceId S3. 13. The browser embeds SurfaceId S3 with the right dsf. There is a race around steps 7-9 where the renderer can submit a CompositorFrame before the browser embeds S1. If the timing is right, browser still embeds S1 but it has been deleted and the background color shows through. The render probably shouldn't submit SurfaceId S2 at all. It's not the correct dsf, so it won't look right, and it delays showing the right contents on screen. Since the dsf never changes (as far as CompositorFrames are concerned), the renderer will continue submitting SurfaceId S1 when exiting from WebVR mode.
,
Sep 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ca224d466239318c18a99fcebe64b6937bbd51aa commit ca224d466239318c18a99fcebe64b6937bbd51aa Author: kylechar <kylechar@chromium.org> Date: Thu Sep 14 01:22:10 2017 Switch default back to surface sequences on Android. This CL switches Android to use surface sequences instead of surface references by default. This is due to an increase in the number of missing surfaces. The --enable-surface-references flag turns on surface references if necessary. This CL is temporary and will be reverted after it is verified on canary and merged to M62. Bug: 764460 , 757569 Change-Id: Ic0782dd09fbdc2b7064ee3b1d2d4287ab5c73005 Reviewed-on: https://chromium-review.googlesource.com/665697 Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/heads/master@{#501828} [modify] https://crrev.com/ca224d466239318c18a99fcebe64b6937bbd51aa/content/browser/renderer_host/compositor_impl_android.cc
,
Sep 15 2017
Should this be flagged as needing a merge, Kyle?
,
Sep 15 2017
verified does not repro in 63.0.3214.0
,
Sep 15 2017
Also verified with 63.0.3215.0
,
Sep 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/822008d69631063a5580041123f5ed07273cf2bb commit 822008d69631063a5580041123f5ed07273cf2bb Author: kylechar <kylechar@chromium.org> Date: Mon Sep 18 20:57:17 2017 Switch default back to surface sequences on Android. This CL switches Android to use surface sequences instead of surface references by default. This is due to an increase in the number of missing surfaces. The --enable-surface-references flag turns on surface references if necessary. Bug: 764460 , 757569 Change-Id: Ic0782dd09fbdc2b7064ee3b1d2d4287ab5c73005 Reviewed-on: https://chromium-review.googlesource.com/665697 Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: kylechar <kylechar@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#501828}(cherry picked from commit ca224d466239318c18a99fcebe64b6937bbd51aa) Reviewed-on: https://chromium-review.googlesource.com/671823 Reviewed-by: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#304} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/822008d69631063a5580041123f5ed07273cf2bb/content/browser/renderer_host/compositor_impl_android.cc
,
Sep 18 2017
Sorry about the delay in getting this merged. Should be fixed in M62 and I'll make sure that there is no missing surfaces with WebVR when I turn surface references back on for Android. |
|||||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||||
Comment 1 by dbbrooks@chromium.org
, Aug 21 2017