search

immersive-web / cardboard-vr-display

A JavaScript implementation of a WebVR 1.1 VRDisplay
https://immersive-web.github.io/cardboard-vr-display
Apache License 2.0
91 stars 43 forks source link

Handle Chrome m66+'s change of reporting a deviceorientation rotationRate #19

Closed jsantell closed 6 years ago

jsantell commented 6 years ago

Now in degrees rather than radians. Fixes #18.

jsantell commented 6 years ago

@dustinkerstein ping for review, not a fan of the UA parsing but not sure what other indicators we have here. Thinking of browsers that report Chrome in UA, but those should have the same deviceorientation unit change. Seems safe but need to give it more thought/testing.

1jj commented 6 years ago

Note that Chrome 66 just fixed a bug to follow the spec, other browsers did it right. So this logic seems wrong:

if (this.isIOS || this.isFirefoxAndroid || this.isChromeUsingDegrees) {
     this.gyroscope.multiplyScalar(Math.PI / 180);
   }

and should be vice versa, something simple like this:

if (! this.isChromeUsingDegrees) {
     this.gyroscope.multiplyScalar(Math.PI / 180);
   }

(don't know about Edge, though)

jsantell commented 6 years ago

@1jj I agree with you, but hesitant for devices that could have changed behaviour when they're not covered by these conditionals. Before releasing a new version, will be doing a full test on devices with possibly that change if there's no breakage