immersive-web / webvr-polyfill

Use WebVR today, without requiring a special browser build.
http://immersive-web.github.io/webvr-polyfill/examples/
Apache License 2.0
1.4k stars 323 forks source link

Initial orientation correction in hotfix (chrome m65) #310

Open env3d opened 6 years ago

env3d commented 6 years ago
Description:

In Chrome m65, after the latest hotfix patch, the starting orientation is off by 45 degrees. To reproduce, simply visit the example page https://immersive-web.github.io/webvr-polyfill/examples/ with an affected browser. You will see the cube flashed and then the orientation is turned at 45 degree angle.

I patched the cardboard-vr-display with the following, which seems to fix the issue:

diff --git a/src/sensor-fusion/fusion-pose-sensor.js b/src/sensor-fusion/fusion-pose-sensor.js
index 43bd398..7b2dec9 100644
--- a/src/sensor-fusion/fusion-pose-sensor.js
+++ b/src/sensor-fusion/fusion-pose-sensor.js
@@ -91,10 +91,10 @@ FusionPoseSensor.prototype.getOrientation = function() {
       const z = new MathUtil.Quaternion().setFromAxisAngle(new MathUtil.Vector3(0, 0, -1), 0);
       const y = new MathUtil.Quaternion()

-      if (window.orientation === -90) {
-        y.setFromAxisAngle(new MathUtil.Vector3(0, 1, 0), Math.PI / -2);
+      if (window.orientation === 90) {
+        y.setFromAxisAngle(new MathUtil.Vector3(0, 1, 0), Math.PI + (Math.PI / 4));
       } else {
-        y.setFromAxisAngle(new MathUtil.Vector3(0, 1, 0), Math.PI / 2);
+        y.setFromAxisAngle(new MathUtil.Vector3(0, 1, 0), Math.PI / -4);
       }

       return z.multiply(y);
Additional Information:
jsantell commented 6 years ago

Thanks for this! On my Pixel, it seems the orientation is off only by 10 degrees or so, you're seeing 45 degrees? I haven't tried this patch yet but I'll give this a go later

jsantell commented 6 years ago

I tried that patch and starting from all 3 orientations, after the 'snap' from receiving data, they're all facing away from the cube (Pixel, same Chrome m65 build).

Without the patch there's a snap, but that's always the case going from default looking straight down <0,0,-1>, can see on a Canary build, but pretty accurate:

screenshot_20180317-082729 1

With this patch, I'm looking in a different direction at the start (different depending on initial orientation):

screenshot_20180317-082631

env3d commented 6 years ago

This is interesting, I also tried out the patch on a older phone I have lying around (a LG G3). When the patch is applied, the cube is no longer centered. To summarize, I have tried 2 phones:

LG G3 - No patch is needed MotoZ Play - Patch is needed

I'm thinking this may have something to do with the initial deviceorientation reading? I haven't had time to setup a test case to compare the 2 phones I have yet. Any ideas?

jsantell commented 6 years ago

There are some devices that have different directions for the devicemotion event (see this line of code), but if this is only happening in Chrome m65 with this patch, that could mean the devicemotion (what polyfill usually uses) rotation and the deviceorientation (what the m65 fallback uses) rotation are possibly different units -- does the MotoZ work fine with Canary (and the latest polyfill)? Or Firefox?

env3d commented 6 years ago

So just tried it on firefox and the behavior is the same as Chrome M65. I need to apply the patch to have the start orientation center properly. So this may be something specific to the MotoZ? Incidentally, the MotoZ does not seem to suffer from the low refresh rate, but the LG G3 does.

I'll check out the code and maybe setup a test to examine the numbers coming from deviceorientation and devicemotion from those phones.

jsantell commented 6 years ago

FWIW, I've seen the refresh rate issue sometimes work just fine in Chrome m65 and other time 10hz on the same device, and can recreate outside the polyfill (demo).

I haven't tried the MotoZ but thoght I've heard people having used it. It sounds like all A-Frame/Polyfill experiences wouldn't work correctly if one of the axes are different than expected. You can enable Generic Sensors API on Chrome m63+, and the polyfill will attempt to use the Sensor API, which doesn't require the deviceorientation workarounds for m65, and may have the correct coordinate system (you may have to check vrDisplay.poseSensor_.mode in the example to see what ends up getting used, devicemotion or sensors)? Check out these demos using the Sensor API (with Sensors enabled), I wonder if those sensors work, and if they're the same as the LG G3 or if they use the same system as the devicemotion/orientation it currently uses.

env3d commented 6 years ago

This is great! I'll do some investigation later on and report back. Thanks for all your help on this!

jsantell commented 6 years ago

@env3d thanks for your research and reports! Looking forward to hearing what you find

env3d commented 6 years ago

Ok, so I ran a couple of tests, with interesting results. All tests are conducted using chrome m65 on Android. I have 3 phones: a MotoZ Play, a LG G3, and an Acer Zest Plus.

On the first test, I attach a deviceorientation event and print out the alpha, beta, and gamma values for each. I have the following results:

20180317_125522

As can be seen, the alpha value reported (which translates to the 'look' orientation, rotateY if you will of the head), is all different on all 3 phones.

For the second test, I have 2 event handlers, top is deviceorientation and bottom is devicemotion.

20180317_131221

Noticed for 2 phones (motoZ and LG G3), they are showing null for all values, while the Acer has nulls on the deviceorientation but starting reporting on the devicemotion, but only in the acceleration with gravity field.

Hopefully this will give us more insights into what is happening.

Thanks

env3d commented 6 years ago

More more thing to note on the MotoZ. When I downgrade the browser (m57), all fields in the devcemotion api start reporting proper values.

jsantell commented 6 years ago

devicemotion not reporting rotationRate is one of the issues exclusively in Chrome m65, hence using deviceorientation as a fallback since that does have rotationRate. I wonder if the the values are fixed in Chrome Canary m67? Note that they will be in radians rather than degrees in m66 and onward.

env3d commented 6 years ago

Ok, I tried the following patch. Basically I recorded the initial reported degree in deviceorientation and use it as an offset to return the position to 0 degrees. I also need to adjust the getOrientation angle when in landscape. This code should only affect the fallback path. Do you see any issues with this approach?

diff --git a/src/sensor-fusion/fusion-pose-sensor.js b/src/sensor-fusion/fusion-pose-sensor.js
index 43bd398..01bc79d 100644
--- a/src/sensor-fusion/fusion-pose-sensor.js
+++ b/src/sensor-fusion/fusion-pose-sensor.js
@@ -91,10 +91,10 @@ FusionPoseSensor.prototype.getOrientation = function() {
       const z = new MathUtil.Quaternion().setFromAxisAngle(new MathUtil.Vector3(0, 0, -1), 0);
       const y = new MathUtil.Quaternion()

-      if (window.orientation === -90) {
-        y.setFromAxisAngle(new MathUtil.Vector3(0, 1, 0), Math.PI / -2);
-      } else {
-        y.setFromAxisAngle(new MathUtil.Vector3(0, 1, 0), Math.PI / 2);
+      if (window.orientation === 90) {
+        y.setFromAxisAngle(new MathUtil.Vector3(0, 1, 0), Math.PI/2);
+      } else if (window.orientation === -90) {
+        y.setFromAxisAngle(new MathUtil.Vector3(0, 1, 0), -Math.PI/2);
       }

       return z.multiply(y);
@@ -180,7 +180,8 @@ FusionPoseSensor.prototype.resetPose = function() {
 FusionPoseSensor.prototype.onDeviceOrientation_ = function(e) {
   this._deviceOrientationQ = this._deviceOrientationQ || new MathUtil.Quaternion();
   let { alpha, beta, gamma } = e;
-  alpha = (alpha || 0) * Math.PI / 180;
+  this.init_alpha = this.init_alpha || alpha;
+  alpha = ((alpha-this.init_alpha) || 0) * Math.PI / 180;
   beta = (beta || 0) * Math.PI / 180;
   gamma = (gamma || 0) * Math.PI / 180;
   this._deviceOrientationQ.setFromEulerYXZ(beta, alpha, -gamma);
jsantell commented 6 years ago

@env3d does that work on all the devices you have? Getting the initial alpha value as a base scares me a bit since it could be rotated in any direction upon load, but you're correct in that this keeps it scoped within the m65 fallback

env3d commented 6 years ago

@jsantell, yes it works for all my devices running m65. I totally understand the apprehension but it seems to be the best compromise at this point, at least for my use case.

jsantell commented 6 years ago

Is this still an issue in Chrome m66+?

homerjam commented 6 years ago

This is an issue for me (described here). Some way to apply a "global" offset would be ideal...

// crude example
alpha = ((alpha || 0) * Math.PI / 180) + window.ALPHA_OFFSET;
jsantell commented 6 years ago

@homerjam as this is a device specific issue, I'm not sure how to fix it on this end. The polyfill projects (cardboard-vr-display, webvr-polyfill, webxr-polyfill) all polyfill the WebVR/WebXR APIs where ultimately some pose as position/quaternion, or matrix is returned. At that point, you could manipulate the pose anyway you'd like. As a rough/untested example using THREE:

// Rotate orientation by 90 degrees on the Y
const ROTATION_FIX = new THREE.Quaternion().setFromAxisAngle(new THREE.Vector3(0, 1, 0), Math.PI/2);

const q = new THREE.Quaternion().fromArray(vrFrameData.orientation);
if (DEVICE_NEEDS_OFFSET) {
  q.multiply(ROTATION_FIX);
}

If there's not a consistent rule for what sensors are incorrect/unstandard, it'll be hard to land this in the polyfill. If altering the final quaternion is sufficient (e.g., you don't need the individual XYZ values reported in devicemotion event, where some vendors incorrectly swap and negate the values around), then this should be added to whatever is using the polyfill, either in the code including it, or AFrame for example (or at least a way to modify the values reported from the polyfill, which I think is already possible in A-Frame by bumping the camera components' orientation on tick)