Commit 28e0c070 authored by Klaus Weidner's avatar Klaus Weidner Committed by Commit Bot

Avoid null poses in WebVR magic window mode

There's a race condition where the first magic window VSync could arrive before
ever getting a pose. Add a check and deferred execution to avoid this.

BUG=787196

Change-Id: If804e4d6acc3da7b903ba36108f23c00b1c8fe5b
Reviewed-on: https://chromium-review.googlesource.com/783887
Commit-Queue: Klaus Weidner <klausw@chromium.org>
Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Reviewed-by: default avatarBill Orr <billorr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523657}
parent 982457e6
...@@ -17,7 +17,6 @@ vr_test( (t, mock_service) => { ...@@ -17,7 +17,6 @@ vr_test( (t, mock_service) => {
var counter = 0; var counter = 0;
function onFrame() { function onFrame() {
display.requestAnimationFrame(onFrame);
if (counter == 0) { if (counter == 0) {
t.step( () => { t.step( () => {
assert_false(display.getFrameData(fd)); assert_false(display.getFrameData(fd));
...@@ -27,14 +26,6 @@ vr_test( (t, mock_service) => { ...@@ -27,14 +26,6 @@ vr_test( (t, mock_service) => {
assert_false(display.getFrameData(fd)); assert_false(display.getFrameData(fd));
}, "Does not update within the same frame"); }, "Does not update within the same frame");
} else { } else {
// TODO(mthiesse): This is a workaround for crbug.com/787196. Either
// remove the workaround once fixed or remove this todo.
// Let several animation frames get triggered so we're sure to have a
// pose
if (counter <= 2) {
counter++;
return;
}
t.step( () => { t.step( () => {
assert_true(display.getFrameData(fd)); assert_true(display.getFrameData(fd));
assert_not_equals(fd.pose, null); assert_not_equals(fd.pose, null);
...@@ -49,6 +40,10 @@ vr_test( (t, mock_service) => { ...@@ -49,6 +40,10 @@ vr_test( (t, mock_service) => {
t.done(); t.done();
} }
counter++; counter++;
// Use rAF late so that the mock's setPose above can supply its data
// before RequestVSync calls GetPose. This is an artifact of the mocking
// framework, the real implementation doesn't have this restriction.
display.requestAnimationFrame(onFrame);
} }
display.requestAnimationFrame(onFrame); display.requestAnimationFrame(onFrame);
......
...@@ -16,16 +16,9 @@ vr_test( (t, mock_service) => { ...@@ -16,16 +16,9 @@ vr_test( (t, mock_service) => {
var fd1 = new VRFrameData(); var fd1 = new VRFrameData();
var fd2 = new VRFrameData(); var fd2 = new VRFrameData();
mock_service.mockVRDisplays_[0].setPose(expected_pose); mock_service.mockVRDisplays_[0].setPose(expected_pose);
var numFrames = 0;
function onFrame() { function onFrame() {
display.requestAnimationFrame(onFrame); display.requestAnimationFrame(onFrame);
numFrames++;
// Let one rAF run before checking in order to ensure we actually get
// frame data
if (numFrames == 1) {
return;
}
t.step( () => { t.step( () => {
assert_true(display.getFrameData(fd1)); assert_true(display.getFrameData(fd1));
......
...@@ -47,6 +47,12 @@ namespace blink { ...@@ -47,6 +47,12 @@ namespace blink {
namespace { namespace {
// Threshold for rejecting stored magic window poses as being too old.
// If it's exceeded, defer magic window rAF callback execution until
// a fresh pose is received.
constexpr WTF::TimeDelta kMagicWindowPoseAgeThreshold =
WTF::TimeDelta::FromMilliseconds(250);
VREye StringToVREye(const String& which_eye) { VREye StringToVREye(const String& which_eye) {
if (which_eye == "left") if (which_eye == "left")
return kVREyeLeft; return kVREyeLeft;
...@@ -220,6 +226,7 @@ void VRDisplay::RequestVSync() { ...@@ -220,6 +226,7 @@ void VRDisplay::RequestVSync() {
return; return;
if (!is_presenting_) { if (!is_presenting_) {
magic_window_vsync_waiting_for_pose_.Reset();
magic_window_provider_->GetPose( magic_window_provider_->GetPose(
WTF::Bind(&VRDisplay::OnMagicWindowPose, WrapWeakPersistent(this))); WTF::Bind(&VRDisplay::OnMagicWindowPose, WrapWeakPersistent(this)));
pending_magic_window_vsync_ = true; pending_magic_window_vsync_ = true;
...@@ -1044,15 +1051,31 @@ void VRDisplay::OnMagicWindowVSync(double timestamp) { ...@@ -1044,15 +1051,31 @@ void VRDisplay::OnMagicWindowVSync(double timestamp) {
pending_magic_window_vsync_ = false; pending_magic_window_vsync_ = false;
pending_magic_window_vsync_id_ = -1; pending_magic_window_vsync_id_ = -1;
vr_frame_id_ = -1; vr_frame_id_ = -1;
ProcessScheduledAnimations(timestamp); WTF::TimeDelta pose_age = WTF::TimeTicks::Now() - magic_window_pose_time_;
if (pose_age < kMagicWindowPoseAgeThreshold) {
ProcessScheduledAnimations(timestamp);
} else {
// The VSync got triggered before ever getting a pose, or the pose is
// stale. Defer the animation until a pose arrives to avoid passing null
// poses to the application.
magic_window_vsync_waiting_for_pose_ =
WTF::Bind(&VRDisplay::ProcessScheduledAnimations,
WrapWeakPersistent(this), timestamp);
}
} }
void VRDisplay::OnMagicWindowPose(device::mojom::blink::VRPosePtr pose) { void VRDisplay::OnMagicWindowPose(device::mojom::blink::VRPosePtr pose) {
magic_window_pose_time_ = WTF::TimeTicks::Now();
if (!in_animation_frame_) { if (!in_animation_frame_) {
frame_pose_ = std::move(pose); frame_pose_ = std::move(pose);
} else { } else {
pending_pose_ = std::move(pose); pending_pose_ = std::move(pose);
} }
if (magic_window_vsync_waiting_for_pose_) {
// We have a vsync waiting for a pose, run it now.
std::move(magic_window_vsync_waiting_for_pose_).Run();
magic_window_vsync_waiting_for_pose_.Reset();
}
} }
void VRDisplay::OnPresentationProviderConnectionError() { void VRDisplay::OnPresentationProviderConnectionError() {
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "platform/Timer.h" #include "platform/Timer.h"
#include "platform/heap/Handle.h" #include "platform/heap/Handle.h"
#include "platform/wtf/Forward.h" #include "platform/wtf/Forward.h"
#include "platform/wtf/Functional.h"
#include "platform/wtf/text/WTFString.h" #include "platform/wtf/text/WTFString.h"
#include "public/platform/WebGraphicsContext3DProvider.h" #include "public/platform/WebGraphicsContext3DProvider.h"
...@@ -246,6 +247,8 @@ class VRDisplay final : public EventTargetWithInlineData, ...@@ -246,6 +247,8 @@ class VRDisplay final : public EventTargetWithInlineData,
bool pending_presenting_vsync_ = false; bool pending_presenting_vsync_ = false;
bool pending_magic_window_vsync_ = false; bool pending_magic_window_vsync_ = false;
int pending_magic_window_vsync_id_ = -1; int pending_magic_window_vsync_id_ = -1;
base::OnceClosure magic_window_vsync_waiting_for_pose_;
WTF::TimeTicks magic_window_pose_time_;
bool in_animation_frame_ = false; bool in_animation_frame_ = false;
bool did_submit_this_frame_ = false; bool did_submit_this_frame_ = false;
bool display_blurred_ = false; bool display_blurred_ = false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment