Commit b2005f08 authored by Johann Koenig's avatar Johann Koenig Committed by Commit Bot

Revert "Stop sending XRDeviceId with VRDisplayInfo"

This reverts commit 67468f66.

Reason for revert: causes win-chrome builds to fail:
[10171/23886] CXX obj/device/vr/vr/oculus_device.obj
FAILED: obj/device/vr/vr/oculus_device.obj
c:\b\s\w\ir\cache\goma\client\gomacc.exe ..\..\third_party\llvm-build\Release+Asserts\bin\clang-cl.e...(too long)
../../device/vr/oculus/oculus_device.cc(65,17): error: no member named 'id' in 'device::mojom::VRDisplayInfo'
display_info->id = id;
~~~~~~~~~~~~~~^
1 error generated.

https://ci.chromium.org/p/chrome/builders/ci/win-chrome

Original change's description:
> Stop sending XRDeviceId with VRDisplayInfo
>
> This value is not used by Blink, and does not need to be communicated
> over the mojo interface. The XRDeviceId enum should not be removed
> because it is used for usage stats.
>
> Bug: 1017477
> Change-Id: I5e8cfff00c374f48495290c5dfb8911a1824a2de
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424684
> Reviewed-by: Alexander Cooper <alcooper@chromium.org>
> Reviewed-by: Chris Palmer <palmer@chromium.org>
> Auto-Submit: Brandon Jones <bajones@chromium.org>
> Commit-Queue: Brandon Jones <bajones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#810958}

TBR=palmer@chromium.org,bajones@chromium.org,bialpio@chromium.org,alcooper@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1017477
Change-Id: Ic5db77eb272086c7e51c900e8f23f722209dedb3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2434019
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: default avatarJohann Koenig <johannkoenig@google.com>
Cr-Commit-Position: refs/heads/master@{#811183}
parent c8144b5e
...@@ -33,8 +33,10 @@ namespace device { ...@@ -33,8 +33,10 @@ namespace device {
namespace { namespace {
mojom::VRDisplayInfoPtr CreateVRDisplayInfo(const gfx::Size& frame_size) { mojom::VRDisplayInfoPtr CreateVRDisplayInfo(mojom::XRDeviceId device_id,
const gfx::Size& frame_size) {
mojom::VRDisplayInfoPtr device = mojom::VRDisplayInfo::New(); mojom::VRDisplayInfoPtr device = mojom::VRDisplayInfo::New();
device->id = device_id;
device->webxr_default_framebuffer_scale = 1.0; device->webxr_default_framebuffer_scale = 1.0;
device->left_eye = mojom::VREyeParameters::New(); device->left_eye = mojom::VREyeParameters::New();
device->right_eye = nullptr; device->right_eye = nullptr;
...@@ -78,7 +80,7 @@ ArCoreDevice::ArCoreDevice( ...@@ -78,7 +80,7 @@ ArCoreDevice::ArCoreDevice(
// if initialization fails. Use an arbitrary but really low resolution to make // if initialization fails. Use an arbitrary but really low resolution to make
// it obvious if we're using this data instead of the actual values we get // it obvious if we're using this data instead of the actual values we get
// from the output drawing surface. // from the output drawing surface.
SetVRDisplayInfo(CreateVRDisplayInfo({16, 16})); SetVRDisplayInfo(CreateVRDisplayInfo(GetId(), {16, 16}));
} }
ArCoreDevice::ArCoreDevice() ArCoreDevice::ArCoreDevice()
...@@ -175,7 +177,7 @@ void ArCoreDevice::OnDrawingSurfaceReady(gfx::AcceleratedWidget window, ...@@ -175,7 +177,7 @@ void ArCoreDevice::OnDrawingSurfaceReady(gfx::AcceleratedWidget window,
<< frame_size.height() << " rotation=" << static_cast<int>(rotation); << frame_size.height() << " rotation=" << static_cast<int>(rotation);
DCHECK(!session_state_->is_arcore_gl_initialized_); DCHECK(!session_state_->is_arcore_gl_initialized_);
auto display_info = CreateVRDisplayInfo(frame_size); auto display_info = CreateVRDisplayInfo(GetId(), frame_size);
SetVRDisplayInfo(std::move(display_info)); SetVRDisplayInfo(std::move(display_info));
RequestArCoreGlInitialization(window, rotation, frame_size); RequestArCoreGlInitialization(window, rotation, frame_size);
......
...@@ -111,12 +111,17 @@ device::mojom::VREyeParametersPtr ValidateEyeParameters( ...@@ -111,12 +111,17 @@ device::mojom::VREyeParametersPtr ValidateEyeParameters(
} }
device::mojom::VRDisplayInfoPtr ValidateVRDisplayInfo( device::mojom::VRDisplayInfoPtr ValidateVRDisplayInfo(
const device::mojom::VRDisplayInfo* info) { const device::mojom::VRDisplayInfo* info,
device::mojom::XRDeviceId id) {
if (!info) if (!info)
return nullptr; return nullptr;
device::mojom::VRDisplayInfoPtr ret = device::mojom::VRDisplayInfo::New(); device::mojom::VRDisplayInfoPtr ret = device::mojom::VRDisplayInfo::New();
// Rather than just cloning everything, we copy over each field and validate
// individually. This ensures new fields don't bypass validation.
ret->id = id;
// Maximum 1000km translation. // Maximum 1000km translation.
if (info->stage_parameters && if (info->stage_parameters &&
IsValidTransform(info->stage_parameters->mojo_from_floor, 1000000)) { IsValidTransform(info->stage_parameters->mojo_from_floor, 1000000)) {
...@@ -210,7 +215,7 @@ BrowserXRRuntimeImpl::BrowserXRRuntimeImpl( ...@@ -210,7 +215,7 @@ BrowserXRRuntimeImpl::BrowserXRRuntimeImpl(
: id_(id), : id_(id),
device_data_(std::move(device_data)), device_data_(std::move(device_data)),
runtime_(std::move(runtime)), runtime_(std::move(runtime)),
display_info_(ValidateVRDisplayInfo(display_info.get())) { display_info_(ValidateVRDisplayInfo(display_info.get(), id)) {
DVLOG(2) << __func__ << ": id=" << id; DVLOG(2) << __func__ << ": id=" << id;
// Unretained is safe because we are calling through an InterfacePtr we own, // Unretained is safe because we are calling through an InterfacePtr we own,
// so we won't be called after runtime_ is destroyed. // so we won't be called after runtime_ is destroyed.
...@@ -352,7 +357,7 @@ bool BrowserXRRuntimeImpl::SupportsNonEmulatedHeight() const { ...@@ -352,7 +357,7 @@ bool BrowserXRRuntimeImpl::SupportsNonEmulatedHeight() const {
void BrowserXRRuntimeImpl::OnDisplayInfoChanged( void BrowserXRRuntimeImpl::OnDisplayInfoChanged(
device::mojom::VRDisplayInfoPtr vr_device_info) { device::mojom::VRDisplayInfoPtr vr_device_info) {
bool had_display_info = !!display_info_; bool had_display_info = !!display_info_;
display_info_ = ValidateVRDisplayInfo(vr_device_info.get()); display_info_ = ValidateVRDisplayInfo(vr_device_info.get(), id_);
if (had_display_info) { if (had_display_info) {
for (VRServiceImpl* service : services_) { for (VRServiceImpl* service : services_) {
service->OnDisplayInfoChanged(); service->OnDisplayInfoChanged();
......
...@@ -98,11 +98,14 @@ mojom::VREyeParametersPtr CreateEyeParamater( ...@@ -98,11 +98,14 @@ mojom::VREyeParametersPtr CreateEyeParamater(
return eye_params; return eye_params;
} }
mojom::VRDisplayInfoPtr CreateVRDisplayInfo(gvr::GvrApi* gvr_api) { mojom::VRDisplayInfoPtr CreateVRDisplayInfo(gvr::GvrApi* gvr_api,
mojom::XRDeviceId device_id) {
TRACE_EVENT0("input", "GvrDelegate::CreateVRDisplayInfo"); TRACE_EVENT0("input", "GvrDelegate::CreateVRDisplayInfo");
mojom::VRDisplayInfoPtr device = mojom::VRDisplayInfo::New(); mojom::VRDisplayInfoPtr device = mojom::VRDisplayInfo::New();
device->id = device_id;
gvr::BufferViewportList gvr_buffer_viewports = gvr::BufferViewportList gvr_buffer_viewports =
gvr_api->CreateEmptyBufferViewportList(); gvr_api->CreateEmptyBufferViewportList();
gvr_buffer_viewports.SetToRecommendedBufferViewports(); gvr_buffer_viewports.SetToRecommendedBufferViewports();
...@@ -267,7 +270,7 @@ GvrDelegateProvider* GvrDevice::GetGvrDelegateProvider() { ...@@ -267,7 +270,7 @@ GvrDelegateProvider* GvrDevice::GetGvrDelegateProvider() {
void GvrDevice::OnDisplayConfigurationChanged(JNIEnv* env, void GvrDevice::OnDisplayConfigurationChanged(JNIEnv* env,
const JavaRef<jobject>& obj) { const JavaRef<jobject>& obj) {
DCHECK(gvr_api_); DCHECK(gvr_api_);
SetVRDisplayInfo(CreateVRDisplayInfo(gvr_api_.get())); SetVRDisplayInfo(CreateVRDisplayInfo(gvr_api_.get(), GetId()));
} }
void GvrDevice::Init(base::OnceCallback<void(bool)> on_finished) { void GvrDevice::Init(base::OnceCallback<void(bool)> on_finished) {
...@@ -291,7 +294,7 @@ void GvrDevice::CreateNonPresentingContext() { ...@@ -291,7 +294,7 @@ void GvrDevice::CreateNonPresentingContext() {
jlong context = Java_NonPresentingGvrContext_getNativeGvrContext( jlong context = Java_NonPresentingGvrContext_getNativeGvrContext(
env, non_presenting_context_); env, non_presenting_context_);
gvr_api_ = gvr::GvrApi::WrapNonOwned(reinterpret_cast<gvr_context*>(context)); gvr_api_ = gvr::GvrApi::WrapNonOwned(reinterpret_cast<gvr_context*>(context));
SetVRDisplayInfo(CreateVRDisplayInfo(gvr_api_.get())); SetVRDisplayInfo(CreateVRDisplayInfo(gvr_api_.get(), GetId()));
if (paused_) { if (paused_) {
PauseTracking(); PauseTracking();
......
...@@ -27,9 +27,11 @@ constexpr unsigned int kRenderHeight = 1024; ...@@ -27,9 +27,11 @@ constexpr unsigned int kRenderHeight = 1024;
// However our mojo interface expects display info right away to support WebVR. // 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 // We create a fake display info to use, then notify the client that the display
// info changed when we get real data. // info changed when we get real data.
mojom::VRDisplayInfoPtr CreateFakeVRDisplayInfo() { mojom::VRDisplayInfoPtr CreateFakeVRDisplayInfo(device::mojom::XRDeviceId id) {
mojom::VRDisplayInfoPtr display_info = mojom::VRDisplayInfo::New(); mojom::VRDisplayInfoPtr display_info = mojom::VRDisplayInfo::New();
display_info->id = id;
display_info->left_eye = mojom::VREyeParameters::New(); display_info->left_eye = mojom::VREyeParameters::New();
display_info->right_eye = mojom::VREyeParameters::New(); display_info->right_eye = mojom::VREyeParameters::New();
...@@ -59,7 +61,7 @@ OpenXrDevice::OpenXrDevice(OpenXrStatics* openxr_statics) ...@@ -59,7 +61,7 @@ OpenXrDevice::OpenXrDevice(OpenXrStatics* openxr_statics)
: VRDeviceBase(device::mojom::XRDeviceId::OPENXR_DEVICE_ID), : VRDeviceBase(device::mojom::XRDeviceId::OPENXR_DEVICE_ID),
instance_(openxr_statics->GetXrInstance()), instance_(openxr_statics->GetXrInstance()),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
mojom::VRDisplayInfoPtr display_info = CreateFakeVRDisplayInfo(); mojom::VRDisplayInfoPtr display_info = CreateFakeVRDisplayInfo(GetId());
SetVRDisplayInfo(std::move(display_info)); SetVRDisplayInfo(std::move(display_info));
#if defined(OS_WIN) #if defined(OS_WIN)
......
...@@ -168,6 +168,8 @@ void OpenXrRenderLoop::InitializeDisplayInfo() { ...@@ -168,6 +168,8 @@ void OpenXrRenderLoop::InitializeDisplayInfo() {
current_display_info_->left_eye = mojom::VREyeParameters::New(); current_display_info_->left_eye = mojom::VREyeParameters::New();
} }
current_display_info_->id = device::mojom::XRDeviceId::OPENXR_DEVICE_ID;
gfx::Size view_size = openxr_->GetViewSize(); gfx::Size view_size = openxr_->GetViewSize();
current_display_info_->left_eye->render_width = view_size.width(); current_display_info_->left_eye->render_width = view_size.width();
current_display_info_->right_eye->render_width = view_size.width(); current_display_info_->right_eye->render_width = view_size.width();
......
...@@ -27,6 +27,12 @@ using gfx::Vector3dF; ...@@ -27,6 +27,12 @@ using gfx::Vector3dF;
namespace { namespace {
static constexpr int kDefaultPumpFrequencyHz = 60; static constexpr int kDefaultPumpFrequencyHz = 60;
mojom::VRDisplayInfoPtr CreateVRDisplayInfo(mojom::XRDeviceId id) {
mojom::VRDisplayInfoPtr display_info = mojom::VRDisplayInfo::New();
display_info->id = id;
return display_info;
}
display::Display::Rotation GetRotation() { display::Display::Rotation GetRotation() {
display::Screen* screen = display::Screen::GetScreen(); display::Screen* screen = display::Screen::GetScreen();
if (!screen) { if (!screen) {
...@@ -48,7 +54,7 @@ VROrientationDevice::VROrientationDevice(mojom::SensorProvider* sensor_provider, ...@@ -48,7 +54,7 @@ VROrientationDevice::VROrientationDevice(mojom::SensorProvider* sensor_provider,
base::BindOnce(&VROrientationDevice::SensorReady, base::BindOnce(&VROrientationDevice::SensorReady,
base::Unretained(this))); base::Unretained(this)));
SetVRDisplayInfo(mojom::VRDisplayInfo::New()); SetVRDisplayInfo(CreateVRDisplayInfo(GetId()));
} }
VROrientationDevice::~VROrientationDevice() { VROrientationDevice::~VROrientationDevice() {
......
...@@ -298,6 +298,7 @@ struct VRStageParameters { ...@@ -298,6 +298,7 @@ struct VRStageParameters {
}; };
struct VRDisplayInfo { struct VRDisplayInfo {
XRDeviceId id;
VRStageParameters? stage_parameters; VRStageParameters? stage_parameters;
// Parameters required to distort a scene for viewing in a VR headset. Only // Parameters required to distort a scene for viewing in a VR headset. Only
// required for devices which have the can_present capability. // required for devices which have the can_present capability.
......
...@@ -17,6 +17,8 @@ FakeVRDevice::~FakeVRDevice() {} ...@@ -17,6 +17,8 @@ FakeVRDevice::~FakeVRDevice() {}
mojom::VRDisplayInfoPtr FakeVRDevice::InitBasicDevice() { mojom::VRDisplayInfoPtr FakeVRDevice::InitBasicDevice() {
mojom::VRDisplayInfoPtr display_info = mojom::VRDisplayInfo::New(); mojom::VRDisplayInfoPtr display_info = mojom::VRDisplayInfo::New();
display_info->id = GetId();
display_info->left_eye = InitEye(45, -0.03f, 1024); display_info->left_eye = InitEye(45, -0.03f, 1024);
display_info->right_eye = InitEye(45, 0.03f, 1024); display_info->right_eye = InitEye(45, 0.03f, 1024);
return display_info; return display_info;
......
...@@ -66,6 +66,7 @@ void VRDeviceBase::ListenToDeviceChanges( ...@@ -66,6 +66,7 @@ void VRDeviceBase::ListenToDeviceChanges(
void VRDeviceBase::SetVRDisplayInfo(mojom::VRDisplayInfoPtr display_info) { void VRDeviceBase::SetVRDisplayInfo(mojom::VRDisplayInfoPtr display_info) {
DCHECK(display_info); DCHECK(display_info);
DCHECK(display_info->id == id_);
display_info_ = std::move(display_info); display_info_ = std::move(display_info);
if (listener_) if (listener_)
......
...@@ -72,10 +72,16 @@ class VRDeviceTest : public testing::Test { ...@@ -72,10 +72,16 @@ class VRDeviceTest : public testing::Test {
std::unique_ptr<VRDeviceBaseForTesting> MakeVRDevice() { std::unique_ptr<VRDeviceBaseForTesting> MakeVRDevice() {
std::unique_ptr<VRDeviceBaseForTesting> device = std::unique_ptr<VRDeviceBaseForTesting> device =
std::make_unique<VRDeviceBaseForTesting>(); std::make_unique<VRDeviceBaseForTesting>();
device->SetVRDisplayInfoForTest(mojom::VRDisplayInfo::New()); device->SetVRDisplayInfoForTest(MakeVRDisplayInfo(device->GetId()));
return device; return device;
} }
mojom::VRDisplayInfoPtr MakeVRDisplayInfo(mojom::XRDeviceId device_id) {
mojom::VRDisplayInfoPtr display_info = mojom::VRDisplayInfo::New();
display_info->id = device_id;
return display_info;
}
base::test::SingleThreadTaskEnvironment task_environment_; base::test::SingleThreadTaskEnvironment task_environment_;
DISALLOW_COPY_AND_ASSIGN(VRDeviceTest); DISALLOW_COPY_AND_ASSIGN(VRDeviceTest);
...@@ -93,7 +99,7 @@ TEST_F(VRDeviceTest, DeviceChangedDispatched) { ...@@ -93,7 +99,7 @@ TEST_F(VRDeviceTest, DeviceChangedDispatched) {
base::DoNothing()); // TODO: consider getting initial info base::DoNothing()); // TODO: consider getting initial info
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_CALL(listener, DoOnChanged(testing::_)).Times(1); EXPECT_CALL(listener, DoOnChanged(testing::_)).Times(1);
device->SetVRDisplayInfoForTest(mojom::VRDisplayInfo::New()); device->SetVRDisplayInfoForTest(MakeVRDisplayInfo(device->GetId()));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
......
...@@ -27,8 +27,9 @@ namespace { ...@@ -27,8 +27,9 @@ namespace {
// However our mojo interface expects display info right away to support WebVR. // 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 // We create a fake display info to use, then notify the client that the display
// info changed when we get real data. // info changed when we get real data.
mojom::VRDisplayInfoPtr CreateFakeVRDisplayInfo() { mojom::VRDisplayInfoPtr CreateFakeVRDisplayInfo(device::mojom::XRDeviceId id) {
mojom::VRDisplayInfoPtr display_info = mojom::VRDisplayInfo::New(); mojom::VRDisplayInfoPtr display_info = mojom::VRDisplayInfo::New();
display_info->id = id;
display_info->left_eye = mojom::VREyeParameters::New(); display_info->left_eye = mojom::VREyeParameters::New();
display_info->right_eye = mojom::VREyeParameters::New(); display_info->right_eye = mojom::VREyeParameters::New();
...@@ -55,7 +56,7 @@ mojom::VRDisplayInfoPtr CreateFakeVRDisplayInfo() { ...@@ -55,7 +56,7 @@ mojom::VRDisplayInfoPtr CreateFakeVRDisplayInfo() {
MixedRealityDevice::MixedRealityDevice() MixedRealityDevice::MixedRealityDevice()
: VRDeviceBase(device::mojom::XRDeviceId::WINDOWS_MIXED_REALITY_ID) { : VRDeviceBase(device::mojom::XRDeviceId::WINDOWS_MIXED_REALITY_ID) {
SetVRDisplayInfo(CreateFakeVRDisplayInfo()); SetVRDisplayInfo(CreateFakeVRDisplayInfo(GetId()));
} }
MixedRealityDevice::~MixedRealityDevice() { MixedRealityDevice::~MixedRealityDevice() {
......
...@@ -671,6 +671,8 @@ bool MixedRealityRenderLoop::UpdateDisplayInfo() { ...@@ -671,6 +671,8 @@ bool MixedRealityRenderLoop::UpdateDisplayInfo() {
if (!current_display_info_) { if (!current_display_info_) {
current_display_info_ = mojom::VRDisplayInfo::New(); current_display_info_ = mojom::VRDisplayInfo::New();
current_display_info_->id =
device::mojom::XRDeviceId::WINDOWS_MIXED_REALITY_ID;
changed = true; changed = 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