Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple VR devices to be used simultaneously. #6286

Merged
merged 1 commit into from
Mar 24, 2015
Merged

Conversation

borismus
Copy link
Contributor

This enables applications such as this one: https://googledrive.com/host/0B4Nj-yDXjBs_fmNpbDdKMGlvLWg4RU05eWdKWURSZWs0d0ZzYURCemtodTdKeVprYy0ySFE.

Generally, worth thinking about what the right way of supporting multiple position sensors is, we should sync about a full rewrite of VREffect and VRControls and WebVR boilerplate.

@mrdoob mrdoob merged commit fe169d3 into mrdoob:dev Mar 24, 2015
mrdoob added a commit that referenced this pull request Mar 24, 2015
@mrdoob
Copy link
Owner

mrdoob commented Mar 24, 2015

Thanks!

@dmarcos
Copy link
Contributor

dmarcos commented Apr 2, 2015

@mrdoob @borismus In this PR we iterate through the list of vr input devices and copy the orientation and position into the camera quaternion and position for each one. The values of the last sensor in the list will prevail. Is this intentional?

@borismus
Copy link
Contributor Author

borismus commented Apr 3, 2015

It's not a great solution, but at least allows for the scenario where there
are two separate devices, with one providing only orientation and one
providing only position. To solve it right we need more sophistication in
VRControls. Basically we could keep track of the previous position and
orientation from each device, calculate deltas and then apply a combined
delta.

On Thu, Apr 2, 2015 at 12:35 AM, Diego Marcos notifications@github.com
wrote:

@mrdoob https://github.com/mrdoob @borismus
https://github.com/borismus In this PR we iterate through the list of
vr input devices and copy the orientation and position into the camera
quaternion for each one. The orientation and position of the last one will
prevail. Is this intentional?


Reply to this email directly or view it on GitHub
#6286 (comment).

@dmarcos
Copy link
Contributor

dmarcos commented Apr 5, 2015

@borismus @cyee @vvuk @jcarpenter These changes are not compatible with how things work on Firefox Nightly at the moment. The webVR API always returns a Cardboard device besides other devices that might be connected. On desktop, the Cardboard device is reported on second place and always returns a zero quaternion (the value is never null). When iterating over the list the Cardboard orientation overrides the Rift's value.

We have to improve threejs to better handle multiple vr inputs without relying on the order of the devices list returned by the API. We have to allow the application to decide which device is going to be used for orientation and position (via Hardware ID?)

@dmarcos dmarcos mentioned this pull request Apr 6, 2015
@dmarcos
Copy link
Contributor

dmarcos commented Apr 6, 2015

@borismus @mrdoob Do you mind if we revert this change until we come up with a better API to handle multiple input devices? We have people relying on VRcontrols reporting that Firefox Nightly is broken.

@brianpeiris
Copy link
Contributor

@dmarcos IMO the real problem is that Firefox Nightly reports a phantom Cardboard sensor. The sensible fix is for Firefox to remove that device on desktops. Is that going to happen anytime soon?

If not, I can tweak #6340 to ignore the cardboard device (by looking for a specific deviceId) if it finds an Oculus device as well.

@dmarcos
Copy link
Contributor

dmarcos commented Apr 6, 2015

@brianpeiris My understanding is that the phantom Cardboard sensor is something temporary. @vvuk can provide more info on the rational behind it.

This strange behavior of Firefox Nightly is showing the kind of problems that this patch is introducing when dealing with multiple inputs. It iterates over all the reported devices and sets the camera orientation and position for each of them. The last in the list wins and there's no guaranteed order in which VR inputs are reported. It is going to confuse people. We need to give control to the developer on what input or combination of inputs are going to be used to modify the camera.

@brianpeiris
Copy link
Contributor

To the larger discussion; VRControls makes a lot of assumptions about position/orientation sensors right now. I think fusing orientation and position from two separate devices is a good idea. The headtrackr implementation is a good example. I can see a similar solution being used for other combinations such as @mkeblx's goggle-paper fiducial tracking library for adding positional tracking to cardboard devices or Sixense's STEM/Hydra to add positional tracking for DK1 or Gear VR.

I don't know if it makes sense to combine orientations from multiple devices or positions from multiple devices though. How do you decide which one is the "base" position/orientation?

Furthermore, with the current Razer Hydra and STEM systems and in the near future, with the Vive headset and maybe even with Rift CV1, sensor devices are going to represent more than just the user's head position and orientation. They are going to represent hands and arms and torsos. Maybe even fingers. These assumptions will not hold for long but I don't think we need to account for everything right now.

@brianpeiris
Copy link
Contributor

We need to give control to the developer on what input or combination of inputs are going to be used to modify the camera.

@dmarcos From the point of view of a WebVR API user, I'd like to think that the APIs take care of this for me automatically since there are probably going to be a typical set of configurations. Each dev shouldn't have to repeat the setup code in their applications. Perhaps the API spec should include a sensorLocation property that can be used to differentiate between a "head-orientation" sensor, "head-position" sensor, and a "hand" sensor, etc., so that VRControls can just filter to find the ones that should control the camera.

@vvuk
Copy link

vvuk commented Apr 6, 2015

The Cardboard device showing up on desktop is temporary, but multiple devices showing up is not -- these issues should be resolved early on before we have content out there that's blindly picking the first device in the list (or letting the last one win). Whatever HMD is used for output should have its associated position sensor(s) used (via matching hardwareUnitId). Any fusing between different devices/IDs will likely need to be done on a webapp level with specific understanding of the sensors involved (with helper libraries, certainly), instead of just taking all the provided ones.

If there are multiple devices, then it's reasonable to pick the first presented HMD, or provide the user with a list to choose from, and then use its associated sensor(s). Or try to choose a more sensible default based on device name (not ID -- the IDs are not constant in any way and will change for every page, essentially).

In the future, you may be able to drive two HMDs (for two users) from one browser, so choosing explicitly will be come important for those applications.

@brianpeiris
Copy link
Contributor

Any fusing between different devices/IDs will likely need to be done on a webapp level with specific understanding of the sensors involved

@borismus Maybe the polyfill should fuse the mouse sensor and the headtrackr sensor before instead of VRControls, since the polyfill is already aware of their details.

Or try to choose a more sensible default based on device name.

@vvuk So is deviceName guaranteed to be a constant for each device going forward? Should it be enumerated in the spec?

Or try to choose a more sensible default based on device name (not ID -- the IDs are not constant in any way and will change for every page, essentially).

Doesn't this contradict the spec?:

deviceId: An identifier for this distinct sensor/device on a physical hardware device. This shouldn’t change across browser restarts, allowing configuration data to be saved based on it.

@dmarcos
Copy link
Contributor

dmarcos commented Apr 6, 2015

@mrdoob @borismus What do you think about reverting this patch and giving a bit more thought to the multiple VR inputs API?

@mrdoob
Copy link
Owner

mrdoob commented Apr 6, 2015

@dmarcos this hasn't reached to the master branch yet. we still have some time to come up with the right approach.

@dmarcos
Copy link
Contributor

dmarcos commented Apr 6, 2015

@mrdoob Neither dev or master branches work at the moment with Firefox Nightly and we are getting reports of Firefox being broken. I want to revert VRControls to a functional state and then come up with a good solution for multiple inputs. It's going to take a few days to come up with something palatable. Should we patch VRControls with @brianpeiris patch in the meantime?

@mrdoob
Copy link
Owner

mrdoob commented Apr 6, 2015

Wait, I can't revert this. There have been other commits after this. Could you guys do a new PR that makes it work on Firefox Nightly again?

@borismus
Copy link
Contributor Author

borismus commented Apr 6, 2015

I'm fine with reverting this. Main objective of the patch was to kick off a broader discussion, so mission accomplished :)

First, as far as I can tell, PositionSensors are designed to be general enough to correspond to various objects: the head, an input controller, etc. It's pretty clear to me that PositionSensors corresponding to the head are a necessity, and they should somehow be identifiable as such. To solve this, we can have an enum in the PositionSensor corresponding to body part. To start, we can start with options of HEAD, and OTHER. (cc: @vvuk)

Next, the question is whether or not the WebVR spec should even support multiple simultaneous PositionSensors for each body part. One design choice is to say that there can only be one PositionSensor active at a time. If we go with this direct mapping, and want to implement the rift default of being able to HMD-look and mouse-look simultaneously, we can special-case the mouse-look and keyboard input and not try to polyfill those.

Otherwise, we allow multiple simultaneous PositionSensors, which is my preferred option. If we are keeping the spec as is, the right way to do this is to keep track of cumulative positionDeltas & orientationDeltas that are affected by each PositionSensor. I'll work on a patch unless someone objects.

@vvuk
Copy link

vvuk commented Apr 7, 2015

Any fusing between different devices/IDs will likely need to be done on a webapp level with specific understanding of the sensors involved

@borismus Maybe the polyfill should fuse the mouse sensor and the headtrackr sensor before instead of VRControls, since the polyfill is already aware of their details.

Yes, that would make sense to me.

Or try to choose a more sensible default based on device name.

@vvuk So is deviceName guaranteed to be a constant for each device going forward? Should it be enumerated in the spec?

It's not -- it's just a user-readable name. You could put the name directly in a dropdown UI to allow the user to select between different devices.

Or try to choose a more sensible default based on device name (not ID -- the IDs are not constant in any way and will change for every page, essentially).

Doesn't this contradict the spec?:

deviceId: An identifier for this distinct sensor/device on a physical hardware device. This shouldn’t change across browser restarts, allowing configuration data to be saved based on it.

Yeah, the spec needs to change; I'll do that today. Without the change, this accidentally introduces a UUID for the Web, letting any content track you across any website, thus throwing out all sorts of privacy guidelines. Whoops!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants