New issue
Advanced search Search tips

Issue 793364 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug
Proj-XR
Proj-XR-VR



Sign in to add a comment

OpenVR and Oculus devices should be more obvious that they are satisfying thread safety for VR APIs

Project Member Reported by billorr@chromium.org, Dec 8 2017

Issue description

Both OpenVR and Oculus have thread safety requirements, such as running all rendering APIs on one thread.  We should ensure we satisfy these.


We can wrap API usage in an object that DCHECKS if things are run on incorrect threads.

 
Status: Assigned (was: Untriaged)
Components: Internals>XR
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/582b854a42002ff2b085b5ea794da8e0e5c2cd56

commit 582b854a42002ff2b085b5ea794da8e0e5c2cd56
Author: Bill Orr <billorr@chromium.org>
Date: Fri Jul 06 22:29:57 2018

Clean up OvrSession lifetime for Oculus device.

For querying basic hardware information, or supporting magic window,
we shouldn't initialize the Oculus runtime for presenting.  We were
using the Oculus APIs on multiple threads in a way that isn't thread
safe.  The multithreaded usage of ovrSessions also prevented correctly
handling errors like device-lost.

This change makes it so we have different OvrSessions for presenting or
non-presenting, and we only have one of these sessions initialized at a time.
The gamepad data now comes from the OculusRenderLoop, so we aren't using the
Oculus API on 3 separate threads.

We aren't passing the sessions around between threads, so this also makes it more
clear that we are using the API in a threadsafe way.

BUG= 852456 ,  838433 ,  793364 

Change-Id: I4f21ce4d59c081f0e1730b52cc12d1819369a436
Reviewed-on: https://chromium-review.googlesource.com/1116241
Commit-Queue: Bill Orr <billorr@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573093}
[modify] https://crrev.com/582b854a42002ff2b085b5ea794da8e0e5c2cd56/device/vr/oculus/oculus_device.cc
[modify] https://crrev.com/582b854a42002ff2b085b5ea794da8e0e5c2cd56/device/vr/oculus/oculus_device.h
[modify] https://crrev.com/582b854a42002ff2b085b5ea794da8e0e5c2cd56/device/vr/oculus/oculus_device_provider.cc
[modify] https://crrev.com/582b854a42002ff2b085b5ea794da8e0e5c2cd56/device/vr/oculus/oculus_device_provider.h
[modify] https://crrev.com/582b854a42002ff2b085b5ea794da8e0e5c2cd56/device/vr/oculus/oculus_gamepad_data_fetcher.cc
[modify] https://crrev.com/582b854a42002ff2b085b5ea794da8e0e5c2cd56/device/vr/oculus/oculus_gamepad_data_fetcher.h
[modify] https://crrev.com/582b854a42002ff2b085b5ea794da8e0e5c2cd56/device/vr/oculus/oculus_render_loop.cc
[modify] https://crrev.com/582b854a42002ff2b085b5ea794da8e0e5c2cd56/device/vr/oculus/oculus_render_loop.h
[modify] https://crrev.com/582b854a42002ff2b085b5ea794da8e0e5c2cd56/device/vr/windows/d3d11_texture_helper.cc
[modify] https://crrev.com/582b854a42002ff2b085b5ea794da8e0e5c2cd56/device/vr/windows/d3d11_texture_helper.h

Labels: VR-Desktop
Components: -Internals>XR Internals>XR>VR
Labels: -Pri-3 Pri-2
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9

commit 405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9
Author: Bill Orr <billorr@chromium.org>
Date: Fri Jul 13 21:19:11 2018

Fix up OpenVR threading and session lifetimes

This is similar to a change made for Oculus devices.  OpenVR is
initialized when we have a magic window session, and can only be used on
the UI thread, or it can be initialized on the render-loop thread, and
we will only use it on that thread.

Gamepad data is now updated per-frame by the render loop, with data
passed over to the gamepad fetcher and read in its polling thread.

BUG= 820288 ,  793364 

Change-Id: I543487f823f7ac08e9b0f01889d227f455d0482c
Reviewed-on: https://chromium-review.googlesource.com/1136056
Commit-Queue: Bill Orr <billorr@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575070}
[modify] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/BUILD.gn
[add] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/openvr_api_wrapper.cc
[add] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/openvr_api_wrapper.h
[modify] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/openvr_device.cc
[modify] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/openvr_device.h
[modify] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/openvr_device_provider.cc
[modify] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/openvr_device_provider.h
[modify] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/openvr_gamepad_data_fetcher.cc
[modify] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/openvr_gamepad_data_fetcher.h
[modify] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/openvr_render_loop.cc
[modify] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/openvr_render_loop.h
[modify] https://crrev.com/405f1053fe8a90e4ed859d430ea6ab2f8f1a2db9/device/vr/openvr/test/fake_openvr_impl_api.cc

Status: Fixed (was: Assigned)

Sign in to add a comment