Commit 281d3a85 authored by Brandon Jones's avatar Brandon Jones Committed by Commit Bot

Ensure WebXR value changes take effect the following frame.

Ensures that the developer can't alter viewports or projection matrices
mid-frame. Covers:

XRSession.depthNear
XRSession.depthFar
XRWebGLLayer.requestViewportScaling()

Bug: 828113
Change-Id: If61262fafaf497a2231b9a57eefa4dd6dbb481eb
Reviewed-on: https://chromium-review.googlesource.com/990719Reviewed-by: default avatarBrian Sheedy <bsheedy@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547547}
parent 565f5a3f
...@@ -23,9 +23,6 @@ xr_session_promise_test( (session) => { ...@@ -23,9 +23,6 @@ xr_session_promise_test( (session) => {
let counter = 0; let counter = 0;
function onFrame(time, xrFrame) { function onFrame(time, xrFrame) {
let leftView = xrFrame.views[0];
let rightView = xrFrame.views[1];
if (counter == 0) { if (counter == 0) {
session.requestAnimationFrame(onFrame); session.requestAnimationFrame(onFrame);
...@@ -34,21 +31,21 @@ xr_session_promise_test( (session) => { ...@@ -34,21 +31,21 @@ xr_session_promise_test( (session) => {
let expectedRightProjection = perspectiveFromFieldOfView( let expectedRightProjection = perspectiveFromFieldOfView(
displayRightEye.fieldOfView, session.depthNear, session.depthFar); displayRightEye.fieldOfView, session.depthNear, session.depthFar);
assert_matrices_approx_equal(leftView.projectionMatrix, expectedLeftProjection); assert_matrices_approx_equal(xrFrame.views[0].projectionMatrix, expectedLeftProjection);
assert_matrices_approx_equal(rightView.projectionMatrix, expectedRightProjection); assert_matrices_approx_equal(xrFrame.views[1].projectionMatrix, expectedRightProjection);
// Update the near and far depths for the session. // Update the near and far depths for the session.
session.depthNear = 1.0; session.depthNear = 1.0;
session.depthFar = 100.0; session.depthFar = 10.0;
// The projection matrices the views report should not take into // The projection matrices the views report should not take into
// account the new session depth values this frame. // account the new session depth values this frame.
assert_matrices_approx_equal(leftView.projectionMatrix, expectedLeftProjection); assert_matrices_approx_equal(xrFrame.views[0].projectionMatrix, expectedLeftProjection);
assert_matrices_approx_equal(rightView.projectionMatrix, expectedRightProjection); assert_matrices_approx_equal(xrFrame.views[1].projectionMatrix, expectedRightProjection);
} else { } else {
// New depth values should be retained between frames. // New depth values should be retained between frames.
assert_equals(session.depthNear, 1.0); assert_equals(session.depthNear, 1.0);
assert_equals(session.depthFar, 100.0); assert_equals(session.depthFar, 10.0);
let expectedLeftProjection = perspectiveFromFieldOfView( let expectedLeftProjection = perspectiveFromFieldOfView(
displayLeftEye.fieldOfView, session.depthNear, session.depthFar); displayLeftEye.fieldOfView, session.depthNear, session.depthFar);
...@@ -56,8 +53,8 @@ xr_session_promise_test( (session) => { ...@@ -56,8 +53,8 @@ xr_session_promise_test( (session) => {
displayRightEye.fieldOfView, session.depthNear, session.depthFar); displayRightEye.fieldOfView, session.depthNear, session.depthFar);
// Projection matricies should now reflect the new depth values // Projection matricies should now reflect the new depth values
assert_matrices_approx_equal(leftView.projectionMatrix, expectedLeftProjection); assert_matrices_approx_equal(xrFrame.views[0].projectionMatrix, expectedLeftProjection);
assert_matrices_approx_equal(rightView.projectionMatrix, expectedRightProjection); assert_matrices_approx_equal(xrFrame.views[1].projectionMatrix, expectedRightProjection);
resolve(); resolve();
} }
counter++; counter++;
......
<!DOCTYPE html>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<script src="file:///gen/layout_test_data/mojo/public/js/mojo_bindings.js"></script>
<script src="file:///gen/device/vr/public/mojom/vr_service.mojom.js"></script>
<script src="../xr/resources/xr-device-mocking.js"></script>
<script src="../xr/resources/xr-test-utils.js"></script>
<script src="../xr/resources/test-constants.js"></script>
<canvas id="webgl-canvas"></canvas>
<script>
let fakeDevices = fakeXRDevices();
xr_session_promise_test( (session, t) => new Promise((resolve, reject) => {
// Session must have a baseLayer or else frame requests will be ignored.
let webglLayer = new XRWebGLLayer(session, gl);
session.baseLayer = webglLayer;
let full_viewport;
function onFirstFrame(time, xrFrame) {
full_viewport = webglLayer.getViewport(xrFrame.views[0]);
webglLayer.requestViewportScaling(0.5);
let new_viewport = webglLayer.getViewport(xrFrame.views[0]);
t.step(() => {
// Viewport should not change within this frame!
assert_equals(full_viewport.x, new_viewport.x);
assert_equals(full_viewport.y, new_viewport.y);
assert_equals(full_viewport.width, new_viewport.width);
assert_equals(full_viewport.height, new_viewport.height);
});
// This should be clamped to some non-zero lower bounds.
webglLayer.requestViewportScaling(0.0);
session.requestAnimationFrame(onSecondFrame);
}
function onSecondFrame(time, xrFrame) {
let new_viewport = webglLayer.getViewport(xrFrame.views[0]);
t.step(() => {
// Viewport should never be zero width or height.
assert_greater_than(new_viewport.width, 0);
assert_greater_than(new_viewport.height, 0);
});
// This should be clamped to 1.0
webglLayer.requestViewportScaling(10.0);
session.requestAnimationFrame(onFinalFrame);
}
function onFinalFrame(time, xrFrame) {
let new_viewport = webglLayer.getViewport(xrFrame.views[0]);
t.step(() => {
// Should be the full viewport again.
assert_equals(full_viewport.x, new_viewport.x);
assert_equals(full_viewport.y, new_viewport.y);
assert_equals(full_viewport.width, new_viewport.width);
assert_equals(full_viewport.height, new_viewport.height);
});
resolve();
}
session.requestAnimationFrame(onFirstFrame);
}), fakeDevices["FakeGooglePixelPhone"], [{ exclusive: true }],
"Ensure viewport scale changes only take effect on the next frame and are properly clamped.");
</script>
...@@ -111,14 +111,14 @@ XRSession::XRSession(XRDevice* device, ...@@ -111,14 +111,14 @@ XRSession::XRSession(XRDevice* device,
void XRSession::setDepthNear(double value) { void XRSession::setDepthNear(double value) {
if (depth_near_ != value) { if (depth_near_ != value) {
views_dirty_ = true; update_views_next_frame_ = true;
depth_near_ = value; depth_near_ = value;
} }
} }
void XRSession::setDepthFar(double value) { void XRSession::setDepthFar(double value) {
if (depth_far_ != value) { if (depth_far_ != value) {
views_dirty_ = true; update_views_next_frame_ = true;
depth_far_ = value; depth_far_ = value;
} }
} }
...@@ -313,6 +313,12 @@ void XRSession::OnFrame( ...@@ -313,6 +313,12 @@ void XRSession::OnFrame(
if (pending_frame_) { if (pending_frame_) {
pending_frame_ = false; pending_frame_ = false;
// Make sure that any frame-bounded changed to the views array take effect.
if (update_views_next_frame_) {
views_dirty_ = true;
update_views_next_frame_ = false;
}
// Cache the base layer, since it could change during the frame callback. // Cache the base layer, since it could change during the frame callback.
XRLayer* frame_base_layer = base_layer_; XRLayer* frame_base_layer = base_layer_;
frame_base_layer->OnFrameStart(); frame_base_layer->OnFrameStart();
...@@ -345,7 +351,7 @@ void XRSession::UpdateCanvasDimensions(Element* element) { ...@@ -345,7 +351,7 @@ void XRSession::UpdateCanvasDimensions(Element* element) {
devicePixelRatio = frame->DevicePixelRatio(); devicePixelRatio = frame->DevicePixelRatio();
} }
views_dirty_ = true; update_views_next_frame_ = true;
output_width_ = element->OffsetWidth() * devicePixelRatio; output_width_ = element->OffsetWidth() * devicePixelRatio;
output_height_ = element->OffsetHeight() * devicePixelRatio; output_height_ = element->OffsetHeight() * devicePixelRatio;
......
...@@ -145,6 +145,7 @@ class XRSession final : public EventTargetWithInlineData { ...@@ -145,6 +145,7 @@ class XRSession final : public EventTargetWithInlineData {
bool ended_ = false; bool ended_ = false;
bool pending_frame_ = false; bool pending_frame_ = false;
bool resolving_frame_ = false; bool resolving_frame_ = false;
bool update_views_next_frame_ = false;
bool views_dirty_ = true; bool views_dirty_ = true;
// Dimensions of the output canvas. // Dimensions of the output canvas.
......
...@@ -173,10 +173,9 @@ void XRWebGLLayer::requestViewportScaling(double scale_factor) { ...@@ -173,10 +173,9 @@ void XRWebGLLayer::requestViewportScaling(double scale_factor) {
ClampToRange(scale_factor, kViewportMinScale, kViewportMaxScale); ClampToRange(scale_factor, kViewportMinScale, kViewportMaxScale);
} }
if (viewport_scale_ != scale_factor) { // Don't set this as the viewport_scale_ directly, since that would allow the
viewport_scale_ = scale_factor; // viewports to change mid-frame.
viewports_dirty_ = true; requested_viewport_scale_ = scale_factor;
}
} }
void XRWebGLLayer::UpdateViewports() { void XRWebGLLayer::UpdateViewports() {
...@@ -239,6 +238,12 @@ void XRWebGLLayer::UpdateViewports() { ...@@ -239,6 +238,12 @@ void XRWebGLLayer::UpdateViewports() {
} }
void XRWebGLLayer::OnFrameStart() { void XRWebGLLayer::OnFrameStart() {
// If the requested scale has changed since the last from, update it now.
if (viewport_scale_ != requested_viewport_scale_) {
viewport_scale_ = requested_viewport_scale_;
viewports_dirty_ = true;
}
framebuffer_->MarkOpaqueBufferComplete(true); framebuffer_->MarkOpaqueBufferComplete(true);
framebuffer_->SetContentsChanged(false); framebuffer_->SetContentsChanged(false);
} }
......
...@@ -96,6 +96,7 @@ class XRWebGLLayer final : public XRLayer, ...@@ -96,6 +96,7 @@ class XRWebGLLayer final : public XRLayer,
std::unique_ptr<viz::SingleReleaseCallback> mirror_release_callback_; std::unique_ptr<viz::SingleReleaseCallback> mirror_release_callback_;
double framebuffer_scale_ = 1.0; double framebuffer_scale_ = 1.0;
double requested_viewport_scale_ = 1.0;
double viewport_scale_ = 1.0; double viewport_scale_ = 1.0;
bool viewports_dirty_ = true; bool viewports_dirty_ = true;
}; };
......
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