1073224 - deviceorientation returns incorrect data

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[andy]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:32.0) Gecko/20100101 Firefox/32.0
Build ID: 20140831201947

Steps to reproduce:

I wrote a webpage that uses an eventlistener for the deviceorientation event


Actual results:

The deviceAlpha, deviceBeta, and deviceGamma values appear to be bogus, or at least non-compliant with the spec in a way that makes it difficult to understand what's happening.


Expected results:

It should work the same way that Chromium and Safari work.

The code that does *not* work is here:

https://github.com/amluto/eleVR-Web-Player/blob/28a87637e35a385e8b11cc255dd2ef95cbf4b86e/js/phonevr.js

the same code works fine on other mobile browsers.  A fully-functional demo is at http://player.elevr.com.  (Note: that demo is currently rather buggy on iOS 8, and it's completely nonfunctional on recent Chrome for Android versions due to an unrelated origin bug in Chromium.)

Comment 1

[Aaron Train [:aaronmt]]

Can you try out Nightly for Android (http://nightly.mozilla.org) and report back if that works for you?

Comment 2

[andy]

Nightly has the same problem.

Playing with http://www.html5rocks.com/en/tutorials/device/orientation/deviceorientationsample.html, it looks like, as long as either beta or gamma is zero, Firefox and Chrome agree.  If, however, I hold the phone vertically in portrait mode (so beta ~ 90 degrees) and rotate it in the plane of the screen a little, Chrome shows both alpha and gamma changing, whereas Firefox shows gamma changing and alpha remaining roughly constant.

To me, this suggests that Firefox has the wrong Euler angles (they're premuted, the initial orientation is wrong, or both).  I haven't figured out the transformation that fixes it yet, though.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Aren't you basically running into gimbal lock, if you're seeing this at beta == pi/2?

Comment 4

[andy]

There's always a discontinuity somewhere due to gimbal lock.  Chromium has it, too.  But I'm getting visibly wrong results even 45 degrees from flat on the table.

The gimbal lock shouldn't matter, anyway.  The very first thing I do with the deviceorientation data is to turn it into a quaternion representation, so as long as the Euler angles are correct (even if they're discontinuous), it should still work.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Doug, can you look into this?  If not, who can?

Comment 6

[andy]

I spent some more time playing with this.  The draft spec is here:

http://w3c.github.io/deviceorientation/spec-source-orientation.html

Here's the experiment I did.  I held my phone in a position in which none of the angles were close to a multiple of pi/2.  I locked the phone into portrait mode.

In Chrome, if I rotate the phone about its long axis (i.e. y), only gamma changes.  This is consistent with my reading of the spec.

In Firefox, if I rotate the phone about its long axis, alpha and gamma change.  On the other hand, if I rotate about the medium axis (i.e. x), only beta changes.

In both browsers, if I hold the phone such that none of the angles are close to a multiple of pi/2 and turn about my own vertical axis, only alpha changes.

This leads me to believe that Firefox is using the wrong Tait-Bryan convention.  Specifically, I think that it has the relative order of beta and gamma reversed.

Comment 7

[Doug Turner (:dougt)]

maybe jdm would want to look.

confirming bug so that it shows up in my queries.

Comment 8

[Josh Matthews [:jdm]]

I know nothing about the math involved here. I suspect someone else will be more effective at dealing with this.

Comment 9

[andy]

If someone can point me at the relevant code, I can try it.  So far I've just pretended that I have an intuitive understanding of the non-commutative nature of SO(3).  Hi, Boris! :)

*bracing myself for possibly contributing to yet another open source project* :)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Hmm.  The immediate entry point into the JS is nsDeviceSensors::Notify in dom/system/nsDeviceSensors.cpp which calls  FireDOMOrientationEvent.  It's basically passing through the values, as far as I can tell.

Where that's getting called from on Android..  Probably widget/android/nsAppShell.cpp where it's doing things like "case AndroidGeckoEvent::SENSOR_EVENT" in nsAppShell::ProcessNextNativeEvent.

Comment 11

[Josh Matthews [:jdm]]

The original Android event likely comes from http://mxr.mozilla.org/mozilla-central/source/widget/android/AndroidJavaWrappers.cpp#500 .

Comment 12

[andy]

And that comes from mobile/android/base/GeckoEvent.java, which is using the (deprecated, AFAICT) TYPE_ORIENTATION sensor event, which uses a coordinate system that is insufficiently documented.  (And the helpers they suggest using sound like they're not just gimbal locked but actually singular at the poles.)

If I can find some time this week, I'll try switching to TYPE_ROTATION_VECTOR, which uses a quaternion representation.  I'll have extra fun getting the portrait/landscape part right -- Android, sensibly, describes the rotation of the device, whereas HTML5 seems to describe the rotation of the logical screen.

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

FWIW -- I landed HAL support for ROTATION_VECTOR/GAME_ROTATION_VECTOR yesterday in bug 1144674, so it should be possible to easily use these in the DOM Orientation Event case.  We don't have a way of asking our HAL if a particular sensor is supported though, which is going to cause some issues.

Comment 14

[mwiechmann]

The Bug is still present in recent nightly vesions for Android. 
https://github.com/mrdoob/three.js/issues/5947 <-- a bug report for three.js, its DeviceOrientationControl.js library has the same issues with landscape mode in Firefox for Android. Every other browser works as desired, but Firefox...

Comment 15

[rich.tibbett]

Please also read my comment on the three.js issue @ https://github.com/mrdoob/three.js/issues/5947#issuecomment-157638445.

"Firefox is still using the Android ORIENTATION sensor data to compute deviceorientation values. That sensor was officially deprecated in Android Level 8 and has never returned values that matched the W3C spec conventions. Mozilla should really be computing deviceorientation data from e.g. Android's ROTATION_VECTOR."

The following code shows how this should be implemented using rotation vector data instead: https://code.google.com/p/chromium/codesearch#chromium/src/content/public/android/java/src/org/chromium/content/browser/DeviceSensors.java&q=devicesen&sq=package:chromium&type=cs&l=416-436.

Comment 16

[rich.tibbett]

FWIW, you can also convert ROTATION_VECTOR or GAME_ROTATION_VECTOR data to the exact Euler values defined in the W3C spec as follows: https://github.com/adtile/Full-Tilt/blob/master/src/Euler.js#L91-L148.

Comment 17

[James Willcox (:snorp) ([e-mail]) (he/him)]

Eugen, I choose you!

Comment 18

[Eugen Sawin [:esawin]]

(In reply to andy from comment #12)
> And that comes from mobile/android/base/GeckoEvent.java, which is using the
> (deprecated, AFAICT) TYPE_ORIENTATION sensor event, which uses a coordinate
> system that is insufficiently documented.  (And the helpers they suggest
> using sound like they're not just gimbal locked but actually singular at the
> poles.)

I've got a WIP APK [1] which uses TYPE_ROTATION_VECTOR instead. If you want to try it, I would be happy to hear if it behaves the way you would expect.

>                                                             ... I'll have
> extra fun getting the portrait/landscape part right -- Android, sensibly,
> describes the rotation of the device, whereas HTML5 seems to describe the
> rotation of the logical screen.

I haven't addressed this explicitly in [1] yet, but will verify against the spec when finalizing (feedback is welcome, of course).

[1] http://archive.mozilla.org/pub/mobile/try-builds/[e-mail]-6b2160708fdfebbb0b52bc6aafe485c9809ac679/try-android-api-11/

Comment 19

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1073224-1.1-Use-SENSOR_ROTATION_VECTOR-for-DOM-D.patch

This defaults to SENSOR_ROTATION_VECTOR for DOM DeviceOrientation events on Android (behind build flags).
Patch 3 makes us fallback to SENSOR_ORIENTATION on Android, should we fail to get the sensor on a device.

Comment 20

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1073224-2.1-Accept-rotation-vector-device-sensor.patch

Adjusting tests to accept all sensors for device orientation data.

Comment 21

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1073224-3.1-Add-fallback-mechanics-for-device-or.patch

Adds sensor fallback on Android. This could lead to unexpected behavior, since we would potentially enable a different device than requested.

Comment 22

[Eugen Sawin [:esawin]]

s/device/sensor in comment 21.

Comment 23

[Eugen Sawin [:esawin]]

We might want to remove the Android-only build flag in patch 1 and get rid of SENSOR_ORIENTATION on all platforms, but I don't know the exact situation for SENSOR_ROTATION_VECTOR support on Gonk.

Comment 24

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 8699146 [details] [diff] [review]
0001-Bug-1073224-1.1-Use-SENSOR_ROTATION_VECTOR-for-DOM-D.patch

>   double x = len > 0 ? values[0] : 0.0;
>   double y = len > 1 ? values[1] : 0.0;
>   double z = len > 2 ? values[2] : 0.0;
>+  double w = len > 3 ? values[3] : 0.0;

Looks fine to me, but one note -- "values[3], originally optional, will always be present from SDK Level 18 onwards".  Are we requiring/guaranteeing >= level 18?  Otherwise we have to compute the w value.

Comment 25

[Eugen Sawin [:esawin]]

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #24)
> Looks fine to me, but one note -- "values[3], originally optional, will
> always be present from SDK Level 18 onwards".  Are we requiring/guaranteeing
> >= level 18?  Otherwise we have to compute the w value.

We're not requiring level 18, but we (you) already compute the w value in GeckoEvent (bug 1144674).

Comment 26

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1073224-3.2-Add-fallback-mechanics-for-device-or.patch

Sorry, I've missed to mirror the fallback mechanics in disableSensor, updated the patch to do so.

Comment 27

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1073224-1.2-Use-SENSOR_ROTATION_VECTOR-for-DOM-D.patch

Updated UUID for nsIDeviceSensorData.

Comment 28

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/11ffe8491f61
https://hg.mozilla.org/integration/mozilla-inbound/rev/9684dd76a803
https://hg.mozilla.org/integration/mozilla-inbound/rev/3be17e370720

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/11ffe8491f61
https://hg.mozilla.org/mozilla-central/rev/9684dd76a803
https://hg.mozilla.org/mozilla-central/rev/3be17e370720

Comment 30

[Daniel]

Hi guys I have commented on this three.js ticket. There is no version 46 available in Android yet. Firefox nightly I have confirmed the problems have been fixed in both WebVR cardboard sensor and device orientation controls. Any ideas when it might make the playstore as Firefox beta for easy user install ? 

https://github.com/mrdoob/three.js/issues/5947

Comment 31

[Kevin Brosnan [Ex-Mozilla]]

9h or 10th of March it will be available on Firefox Beta. ~6 weeks after that it will be in the release channel. The only thing that could interrupt that would be if this patch is shown to cause a major regression (quite unlikely). https://wiki.mozilla.org/RapidRelease/Calendar

Comment 32

[Daniel]

No problems thankyou.

Comment 33

[Daniel]

Hi I have 49.0.2 stable loaded on Android. And instead of these changes going live the feature is broken completely. I am getting zero values now. It was working before.

Comment 34

[Eugen Sawin [:esawin]]

Daniel, can you please create a new bug and post details to reproduce the issue you're seeing?

Comment 35

[Daniel]

https://threejs.org/examples/#misc_controls_deviceorientation this was working and now does not possible regression or a change of event ? i'll create another ticket then.

Comment 36

[Eugen Sawin [:esawin]]

Daniel, it works for me on the Nexus 6P, so please also note your device and Android version in the new bug.

Comment 37

[Daniel]

I had someone check an S7 and the values are working. It's strange. I'll debug further no problems.

Comment 38

[Daniel]

It's not the wrong values as such it's that all deviceorientation events have stopped working it seems. It's fine on Chrome. I'll make another ticket once I figure it out.

Comment 39

[Daniel]

I made a new report here, the event doesn't get dispatched at all. I have found other people complaining too. It is fine on Chrome. 

https://bugzilla.mozilla.org/show_bug.cgi?id=1318293