WebVR: input lag / render pauses due to broken event processing |
||||||||||
Issue descriptionChrome Version: 59.0.3040.0 OS: Pixel (repros on other devices as well) VRCore: 1.3.148407064 happens when using both daydream and cardboard headsets What steps will reproduce the problem? (steps are for cardboard, but daydream has similar result) (1) go to https://webvr.info/samples/03-vr-presentation.html and (2) Click enter VR (3) In binocular mode, touch the screen. What is the expected result? The color of the background should change. What happens instead? The color of the background does not change. And after exiting binocular view, you can see the colors rapidly flicker like all the clicks were stored until exit.
,
Mar 14 2017
If you wait long enough in VR, the queued input events get sent eventually (1-10 seconds later), at least on the 6P.
,
Mar 14 2017
For whatever reason, seems to work find on the LG G3 running 59.0.3041.0.
,
Mar 14 2017
Not reproducible in 59.0.3037.0, which is the last working Canary before Klaus' patch went in, so it's likely related to that.
,
Mar 14 2017
Investigating. FWIW, touching the screen is expected to have no effect while in Daydream View mode, it suppresses touch events due to using capacitive nubs for calibrating screen position. But clicking the controller's touchpad should generate an emulated screen tap.
,
Mar 14 2017
,
Mar 15 2017
Proposed fix in https://codereview.chromium.org/2748293002 : If we call GetVSync inline from a rAF context, we can end up with the next VSync already being ready and scheduled when we exit, and then we'll process several frames in a row without ever yielding back to the main event loop, causing input processing delay.
,
Mar 15 2017
BTW, the broken logic had the side effect of effectively suppressing normal frame lifecycle processing for the compositor, so it made WebVR presentation more efficient. With the fix, it's doing significant work every frame, and we'll really want the compositing path suppression as per issue 698923 .
,
Mar 15 2017
Trace + screenshot of the malfunctioning scheduling attached, note it's running back to back OnVSync frames within a single TaskQueueManager::RunTask without ever yielding execution back to the toplevel.
,
Mar 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ff386cfefd3fe0f9cef72ff28f7c7f09d972608d commit ff386cfefd3fe0f9cef72ff28f7c7f09d972608d Author: klausw <klausw@chromium.org> Date: Wed Mar 15 20:29:19 2017 WebVR: process animations from posted task to yield for other events If we call GetVSync inline from OnVSync, we can end up with the next VSync already being ready and scheduled when we exit, and then we'll process several frames in a row without ever yielding back to the main event loop, causing input processing delay. BUG= 701444 Review-Url: https://codereview.chromium.org/2748293002 Cr-Commit-Position: refs/heads/master@{#457189} [modify] https://crrev.com/ff386cfefd3fe0f9cef72ff28f7c7f09d972608d/chrome/browser/android/vr_shell/vr_shell_gl.cc [modify] https://crrev.com/ff386cfefd3fe0f9cef72ff28f7c7f09d972608d/third_party/WebKit/Source/modules/vr/VRDisplay.cpp [modify] https://crrev.com/ff386cfefd3fe0f9cef72ff28f7c7f09d972608d/third_party/WebKit/Source/modules/vr/VRDisplay.h
,
Mar 16 2017
,
Mar 16 2017
,
Mar 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/efd3d31152e48d1089175b9256c56bccbe6d9792 commit efd3d31152e48d1089175b9256c56bccbe6d9792 Author: klausw <klausw@chromium.org> Date: Fri Mar 17 00:13:12 2017 WebVR: move animation state check to deferred task The animation controller or document may have disappeared in between scheduling the callback and executing it. BUG= 701444 R=bajones, mthiesse Review-Url: https://codereview.chromium.org/2752343002 Cr-Commit-Position: refs/heads/master@{#457618} [modify] https://crrev.com/efd3d31152e48d1089175b9256c56bccbe6d9792/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
,
Mar 23 2017
Reopening since M58 is also affected by this issue. As per OmahaProxy: Commit a316edeb... initially landed in 58.0.2995.0 This also causes a serious performance regression on complex WebVR site such as Sketchfab's Mars One, https://sketchfab.com/models/83ced347037f47aba8473147d65df074. Traces show pauses of hundreds of milliseconds in RealSwapBuffers, apparently as a side effect of event handling being backed up. Traces attached to compare. They are for M56 (as baseline), M58-beta, and a test build for branch-heads/3029 + the two cherry-picked patches from this bug. Requesting merge back to M58 due to the regression. The change has been tested extensively in Canary. Please contact me if you have questions or concerns about this merge.
,
Mar 23 2017
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 23 2017
,
Mar 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/40b2aa9268c6ff790dd6691bac26b5e15799852e commit 40b2aa9268c6ff790dd6691bac26b5e15799852e Author: klausw <klausw@chromium.org> Date: Thu Mar 23 20:55:09 2017 WebVR: process animations from posted task to yield for other events If we call GetVSync inline from OnVSync, we can end up with the next VSync already being ready and scheduled when we exit, and then we'll process several frames in a row without ever yielding back to the main event loop, causing input processing delay. BUG= 701444 NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2748293002 Cr-Commit-Position: refs/heads/master@{#457189} (cherry picked from commit ff386cfefd3fe0f9cef72ff28f7c7f09d972608d) Review-Url: https://codereview.chromium.org/2774673003 Cr-Commit-Position: refs/branch-heads/3029@{#394} Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471} [modify] https://crrev.com/40b2aa9268c6ff790dd6691bac26b5e15799852e/chrome/browser/android/vr_shell/vr_shell_gl.cc [modify] https://crrev.com/40b2aa9268c6ff790dd6691bac26b5e15799852e/third_party/WebKit/Source/modules/vr/VRDisplay.cpp [modify] https://crrev.com/40b2aa9268c6ff790dd6691bac26b5e15799852e/third_party/WebKit/Source/modules/vr/VRDisplay.h
,
Mar 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1598a6bafc5d202e1fe34a2976ea12c37e80214b commit 1598a6bafc5d202e1fe34a2976ea12c37e80214b Author: klausw <klausw@chromium.org> Date: Thu Mar 23 21:01:40 2017 WebVR: move animation state check to deferred task The animation controller or document may have disappeared in between scheduling the callback and executing it. BUG= 701444 R=bajones, mthiesse NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2752343002 Cr-Commit-Position: refs/heads/master@{#457618} (cherry picked from commit efd3d31152e48d1089175b9256c56bccbe6d9792) Review-Url: https://codereview.chromium.org/2771913002 Cr-Commit-Position: refs/branch-heads/3029@{#395} Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471} [modify] https://crrev.com/1598a6bafc5d202e1fe34a2976ea12c37e80214b/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
,
Mar 23 2017
Merged, closing. This issue should not have affected M57, and is fixed for M58 and ToT/M59.
,
Mar 23 2017
To clarify, it'll be fixed for a future M58 build that includes it. Current M58 beta is affected.
,
Mar 24 2017
,
Mar 25 2017
See also issue 705145 where older devices still suffer from >1s pauses in Cardboard mode. It appears that there are two separate issues for M58 and the patches from this bug fixed one of them.
,
Jul 4
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by mthiesse@chromium.org
, Mar 14 2017