Remove size_x and size_z from VRStageParameters

The notion of a simple "size" for Stage Parameters/bounded was a WebVr
concept that did not carry over to WebXr.  To simplify the mojom, move
the "conversion" of square stages (for those runtimes that only support
giving size x/z) down into a utility function in the device process.

Bug: 1017843
Change-Id: Ib050a7ea3655a3816f6008e82434435ecb2c4426
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1918037
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718364}

chrome/browser/vr/service/browser_xr_runtime.cc

@@ -122,7 +122,6 @@ device::mojom::VRDisplayInfoPtr ValidateVRDisplayInfo(
       IsValidTransform(info->stage_parameters->standing_transform, 1000000)) {
     ret->stage_parameters = device::mojom::VRStageParameters::New(
         info->stage_parameters->standing_transform,
-        info->stage_parameters->size_x, info->stage_parameters->size_z,
         info->stage_parameters->bounds);
   }
 

device/BUILD.gn

@@ -329,6 +329,7 @@ test("device_unittests") {
       "vr/orientation/orientation_device_unittest.cc",
       "vr/util/fps_meter_unittest.cc",
       "vr/util/sliding_average_unittest.cc",
+      "vr/util/stage_utils_unittest.cc",
       "vr/vr_device_base_unittest.cc",
     ]
 

device/vr/BUILD.gn

@@ -28,6 +28,8 @@ if (enable_vr) {
       "util/sample_queue.h",
       "util/sliding_average.cc",
       "util/sliding_average.h",
+      "util/stage_utils.cc",
+      "util/stage_utils.h",
       "util/transform_utils.cc",
       "util/transform_utils.h",
       "util/xr_standard_gamepad_builder.cc",

device/vr/oculus/oculus_device.cc

@@ -15,6 +15,7 @@
 #include "build/build_config.h"
 #include "device/vr/oculus/oculus_render_loop.h"
 #include "device/vr/oculus/oculus_type_converters.h"
+#include "device/vr/util/stage_utils.h"
 #include "device/vr/util/transform_utils.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "third_party/libovr/src/Include/OVR_CAPI.h"
@@ -78,8 +79,8 @@ mojom::VRDisplayInfoPtr CreateVRDisplayInfo(mojom::XRDeviceId id,
 
   ovrVector3f boundary_size;
   ovr_GetBoundaryDimensions(session, ovrBoundary_PlayArea, &boundary_size);
-  display_info->stage_parameters->size_x = boundary_size.x;
-  display_info->stage_parameters->size_z = boundary_size.z;
+  display_info->stage_parameters->bounds =
+      vr_utils::GetStageBoundsFromSize(boundary_size.x, boundary_size.z);
 
   return display_info;
 }

device/vr/openvr/openvr_device.cc

@@ -16,6 +16,7 @@
 #include "build/build_config.h"
 #include "device/vr/openvr/openvr_render_loop.h"
 #include "device/vr/openvr/openvr_type_converters.h"
+#include "device/vr/util/stage_utils.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "third_party/openvr/src/headers/openvr.h"
 #include "ui/gfx/geometry/angle_conversions.h"
@@ -99,13 +100,12 @@ mojom::VRDisplayInfoPtr CreateVRDisplayInfo(vr::IVRSystem* vr_system,
 
   vr::IVRChaperone* chaperone = vr::VRChaperone();
   if (chaperone) {
-    chaperone->GetPlayAreaSize(&display_info->stage_parameters->size_x,
-                               &display_info->stage_parameters->size_z);
-  } else {
-    display_info->stage_parameters->size_x = 0.0f;
-    display_info->stage_parameters->size_z = 0.0f;
+    float size_x = 0;
+    float size_z = 0;
+    chaperone->GetPlayAreaSize(&size_x, &size_z);
+    display_info->stage_parameters->bounds =
+        vr_utils::GetStageBoundsFromSize(size_x, size_z);
   }
-
   return display_info;
 }
 

device/vr/openxr/openxr_device.cc

@@ -21,8 +21,6 @@ constexpr float kFov = 45.0f;
 constexpr unsigned int kRenderWidth = 1024;
 constexpr unsigned int kRenderHeight = 1024;
 
-constexpr float kStageSizeX = 0.0f;
-constexpr float kStageSizeZ = 0.0f;
 // OpenXR doesn't give out display info until you start a session.
 // However our mojo interface expects display info right away to support WebVR.
 // We create a fake display info to use, then notify the client that the display
@@ -50,11 +48,6 @@ mojom::VRDisplayInfoPtr CreateFakeVRDisplayInfo(device::mojom::XRDeviceId id) {
   display_info->right_eye->render_width = kRenderWidth;
   display_info->right_eye->render_height = kRenderHeight;
 
-  display_info->stage_parameters = mojom::VRStageParameters::New();
-  display_info->stage_parameters->standing_transform = gfx::Transform();
-  display_info->stage_parameters->size_x = kStageSizeX;
-  display_info->stage_parameters->size_z = kStageSizeZ;
-
   return display_info;
 }
 

device/vr/openxr/openxr_render_loop.cc

@@ -6,6 +6,7 @@
 
 #include "device/vr/openxr/openxr_api_wrapper.h"
 #include "device/vr/openxr/openxr_input_helper.h"
+#include "device/vr/util/stage_utils.h"
 #include "device/vr/util/transform_utils.h"
 #include "ui/gfx/geometry/angle_conversions.h"
 #include "ui/gfx/transform.h"
@@ -234,11 +235,11 @@ bool OpenXrRenderLoop::UpdateStageParameters() {
       changed = true;
     }
 
-    if (current_display_info_->stage_parameters->size_x != stage_bounds.width ||
-        current_display_info_->stage_parameters->size_z !=
-            stage_bounds.height) {
-      current_display_info_->stage_parameters->size_x = stage_bounds.width;
-      current_display_info_->stage_parameters->size_z = stage_bounds.height;
+    if (current_stage_bounds_.width != stage_bounds.width ||
+        current_stage_bounds_.height != stage_bounds.height) {
+      current_display_info_->stage_parameters->bounds =
+          vr_utils::GetStageBoundsFromSize(stage_bounds.width,
+                                           stage_bounds.height);
       changed = true;
     }
 

device/vr/openxr/openxr_render_loop.h

@@ -45,6 +45,7 @@ class OpenXrRenderLoop : public XRCompositorCommon {
 
   std::unique_ptr<OpenXrApiWrapper> openxr_;
   std::unique_ptr<OpenXRInputHelper> input_helper_;
+  XrExtent2Df current_stage_bounds_;
 
   base::RepeatingCallback<void(mojom::VRDisplayInfoPtr)>
       on_display_info_changed_;

device/vr/public/mojom/vr_service.mojom

@@ -240,14 +240,12 @@ struct VREyeParameters {
 
 struct VRStageParameters {
   gfx.mojom.Transform standing_transform;
-  float size_x;
-  float size_z;
 
-  // If present, indicates that the device supports a bounded reference space.
+  // If present, indicates that the device supports a bounded reference space
+  // (https://immersive-web.github.io/webxr/#xrboundedreferencespace-interface).
   // The points are expected to be in the "standing" space (i.e. there should be
   // no need to transform them by the accompanying standing transform) and
   // in a clockwise winding order.
-  // This should supersede size_x/size_z if they are set.
   array<gfx.mojom.Point3F>? bounds;
 };
 

device/vr/util/stage_utils.cc

@@ -0,0 +1,28 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "device/vr/util/stage_utils.h"
+
+#include "base/logging.h"
+#include "ui/gfx/geometry/point3_f.h"
+
+namespace device {
+namespace vr_utils {
+
+std::vector<gfx::Point3F> GetStageBoundsFromSize(float size_x, float size_z) {
+  if (size_x <= 0.0 || size_z <= 0.0)
+    return {};
+
+  double hx = size_x * 0.5;
+  double hz = size_z * 0.5;
+
+  std::vector<gfx::Point3F> bounds = {
+      gfx::Point3F(hx, 0.0, -hz), gfx::Point3F(hx, 0.0, hz),
+      gfx::Point3F(-hx, 0.0, hz), gfx::Point3F(-hx, 0.0, -hz)};
+
+  return bounds;
+}
+
+}  // namespace vr_utils
+}  // namespace device

device/vr/util/stage_utils.h

@@ -0,0 +1,29 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DEVICE_VR_UTIL_STAGE_UTILS_H_
+#define DEVICE_VR_UTIL_STAGE_UTILS_H_
+
+#include <vector>
+
+#include "device/vr/vr_export.h"
+
+namespace gfx {
+class Point3F;
+}  // namespace gfx
+
+namespace device {
+namespace vr_utils {
+
+// convenience method for runtimes that only support rectangular play areas to
+// convert to an array of bounds (which the mojom/WebXr API expects). Returns
+// a vector of points representing the four corners of a rectangle centered at
+// 0,0 with length and width determined by size_x and size_z respectively.
+std::vector<gfx::Point3F> DEVICE_VR_EXPORT GetStageBoundsFromSize(float size_x,
+                                                                  float size_z);
+
+}  // namespace vr_utils
+}  // namespace device
+
+#endif  // DEVICE_VR_UTIL_STAGE_UTILS_H_

device/vr/util/stage_utils_unittest.cc

@@ -0,0 +1,63 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "device/vr/util/stage_utils.h"
+
+#include "ui/gfx/geometry/point3_f.h"
+#include "ui/gfx/geometry/vector3d_f.h"
+#include "ui/gfx/transform.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+constexpr size_t kExpectedBoundsSize = 4;
+}
+
+namespace device {
+
+TEST(StageUtils, Basic) {
+  auto bounds = vr_utils::GetStageBoundsFromSize(2.0, 2.0);
+
+  // The Expected bounds (without transform) are:
+  //  (1, 0, -1)
+  //  (1, 0, 1)
+  //  (-1, 0, 1)
+  //  (-1, 0, -1)
+  // This represents a box 2x2 box, centered around 0.
+  // Note that the choice of which point starts is (unfortunately) an
+  // implementation detail.  The order however, should always be
+  // counter-clockwise.
+  std::vector<gfx::Point3F> expected_bounds = {
+      gfx::Point3F(1, 0, -1), gfx::Point3F(1, 0, 1), gfx::Point3F(-1, 0, 1),
+      gfx::Point3F(-1, 0, -1)};
+
+  ASSERT_EQ(bounds.size(), kExpectedBoundsSize);
+  for (size_t i = 0; i < kExpectedBoundsSize; i++) {
+    EXPECT_FLOAT_EQ(bounds[i].x(), expected_bounds[i].x());
+    EXPECT_FLOAT_EQ(bounds[i].y(), expected_bounds[i].y());
+    EXPECT_FLOAT_EQ(bounds[i].z(), expected_bounds[i].z());
+  }
+}
+
+TEST(StageUtils, InvalidBounds) {
+  // clang-format off
+  std::vector<std::pair<float, float>> test_data{
+    { 0.0,   0.0 },
+    { 0.0,   1.0 },
+    { 1.0,   0.0 },
+    { 1.0,  -1.0 },
+    { -1.0,  1.0 },
+    {-1.0,  -1.0 }
+  };
+  // clang-format on
+
+  // This avoids any compiler issues with 0 not being deduced to the right type.
+  constexpr size_t kEmptyBoundsSize = 0;
+  for (const auto& data : test_data) {
+    auto bounds = vr_utils::GetStageBoundsFromSize(data.first, data.second);
+    ASSERT_EQ(bounds.size(), kEmptyBoundsSize);
+  }
+}
+
+}  // namespace device

third_party/blink/renderer/modules/xr/xr_bounded_reference_space.cc

@@ -54,32 +54,16 @@ void XRBoundedReferenceSpace::EnsureUpdated() {
     // in the boundsGeometry accessor.
     TransformationMatrix bounds_transform = InverseOriginOffsetMatrix();
 
+    // We may not have bounds if we've lost tracking after being created.
+    // Whether we have them or not, we need to clear the existing bounds.
+    bounds_geometry_.clear();
     if (display_info->stage_parameters->bounds) {
-      bounds_geometry_.clear();
-
       for (const auto& bound : *(display_info->stage_parameters->bounds)) {
         FloatPoint3D p =
             bounds_transform.MapPoint(FloatPoint3D(bound.X(), 0.0, bound.Z()));
         bounds_geometry_.push_back(
             DOMPointReadOnly::Create(p.X(), p.Y(), p.Z(), 1.0));
       }
-    } else {
-      double hx = display_info->stage_parameters->size_x * 0.5;
-      double hz = display_info->stage_parameters->size_z * 0.5;
-      FloatPoint3D a = bounds_transform.MapPoint(FloatPoint3D(hx, 0.0, -hz));
-      FloatPoint3D b = bounds_transform.MapPoint(FloatPoint3D(hx, 0.0, hz));
-      FloatPoint3D c = bounds_transform.MapPoint(FloatPoint3D(-hx, 0.0, hz));
-      FloatPoint3D d = bounds_transform.MapPoint(FloatPoint3D(-hx, 0.0, -hz));
-
-      bounds_geometry_.clear();
-      bounds_geometry_.push_back(
-          DOMPointReadOnly::Create(a.X(), a.Y(), a.Z(), 1.0));
-      bounds_geometry_.push_back(
-          DOMPointReadOnly::Create(b.X(), b.Y(), b.Z(), 1.0));
-      bounds_geometry_.push_back(
-          DOMPointReadOnly::Create(c.X(), c.Y(), c.Z(), 1.0));
-      bounds_geometry_.push_back(
-          DOMPointReadOnly::Create(d.X(), d.Y(), d.Z(), 1.0));
     }
   } else {
     // If stage parameters aren't available set the transform to null, which

third_party/blink/renderer/modules/xr/xr_session.cc

@@ -92,6 +92,8 @@ const char kHitTestSubscriptionFailed[] = "Hit test subscription failed.";
 
 const double kDegToRad = M_PI / 180.0;
 
+constexpr wtf_size_t kMinNumberOfBounds = 2;
+
 // Indices into the views array.
 const unsigned int kMonoOrStereoLeftView = 0;
 const unsigned int kStereoRightView = 1;
@@ -402,15 +404,10 @@ ScriptPromise XRSession::requestReferenceSpace(
           MakeGarbageCollected<XRReferenceSpace>(this, requested_type);
       break;
     case XRReferenceSpace::Type::kTypeBoundedFloor: {
-      bool supports_bounded = false;
-      if (immersive() && display_info_->stage_parameters) {
-        if (display_info_->stage_parameters->bounds) {
-          supports_bounded = true;
-        } else if (display_info_->stage_parameters->size_x > 0 &&
-                   display_info_->stage_parameters->size_z > 0) {
-          supports_bounded = true;
-        }
-      }
+      bool supports_bounded =
+          immersive() && display_info_->stage_parameters &&
+          display_info_->stage_parameters->bounds &&
+          display_info_->stage_parameters->bounds->size() > kMinNumberOfBounds;
 
       if (supports_bounded) {
         reference_space = MakeGarbageCollected<XRBoundedReferenceSpace>(this);

third_party/blink/web_tests/external/wpt/resources/chromium/webxr-test.js

@@ -11,8 +11,6 @@ default_standing.matrix = [1, 0, 0, 0,
                            0, 1.65, 0, 1];
 const default_stage_parameters = {
   standingTransform: default_standing,
-  sizeX: 0,
-  sizeZ: 0,
   bounds: null
 };
 

third_party/blink/web_tests/wpt_internal/webxr/resources/xr-internal-device-mocking.js

@@ -19,17 +19,6 @@ MockRuntime.prototype.requestHitTest = function(ray) {
   return Promise.resolve(hit_results);
 };
 
-MockRuntime.prototype.setStageSize = function(x, z) {
-  if (!this.displayInfo_.stageParameters) {
-    this.displayInfo_.stageParameters = default_stage_parameters;
-  }
-
-  this.displayInfo_.stageParameters.sizeX = x;
-  this.displayInfo_.stageParameters.sizeZ = z;
-
-  this.sessionClient_.onChanged(this.displayInfo_);
-};
-
 MockRuntime.prototype.getSubmitFrameCount = function() {
   return this.presentation_provider_.submit_frame_count_;
 };