Closed Bug 1073224 Opened 5 years ago Closed 4 years ago

deviceorientation returns incorrect data

Categories

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

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed
fennec 46+ ---

People

(Reporter: andy, Assigned: esawin)

References

Details

Attachments

(3 files, 2 obsolete files)

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.)
Component: General → DOM
OS: Linux → Android
Product: Firefox for Android → Core
Hardware: x86_64 → ARM
Version: Firefox 32 → unspecified
Can you try out Nightly for Android (http://nightly.mozilla.org) and report back if that works for you?
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.
Aren't you basically running into gimbal lock, if you're seeing this at beta == pi/2?
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.
Doug, can you look into this?  If not, who can?
Flags: needinfo?(dougt)
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.
maybe jdm would want to look.

confirming bug so that it shows up in my queries.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(dougt) → needinfo?(josh)
I know nothing about the math involved here. I suspect someone else will be more effective at dealing with this.
Flags: needinfo?(josh)
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* :)
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.
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.
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.
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...
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.
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.
Flags: needinfo?(dougt)
Eugen, I choose you!
Assignee: nobody → esawin
tracking-fennec: --- → ?
Flags: needinfo?(dougt)
(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/esawin@mozilla.com-6b2160708fdfebbb0b52bc6aafe485c9809ac679/try-android-api-11/
Flags: needinfo?(andy)
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.
Attachment #8699146 - Flags: review?(vladimir)
Adjusting tests to accept all sensors for device orientation data.
Attachment #8699147 - Flags: review?(vladimir)
Adds sensor fallback on Android. This could lead to unexpected behavior, since we would potentially enable a different device than requested.
Attachment #8699149 - Flags: review?(snorp)
s/device/sensor in comment 21.
Attachment #8699149 - Flags: review?(snorp) → review+
tracking-fennec: ? → 46+
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.
Attachment #8699146 - Flags: review?(vladimir) → review+
(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).
Sorry, I've missed to mirror the fallback mechanics in disableSensor, updated the patch to do so.
Attachment #8699149 - Attachment is obsolete: true
Attachment #8703751 - Flags: review?(snorp)
Attachment #8703751 - Flags: review?(snorp) → review+
Updated UUID for nsIDeviceSensorData.
Attachment #8699146 - Attachment is obsolete: true
Attachment #8705140 - Flags: review+
Blocks: 1205649
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
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
No problems thankyou.
Flags: needinfo?(andy)
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.
Daniel, can you please create a new bug and post details to reproduce the issue you're seeing?
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.
Daniel, it works for me on the Nexus 6P, so please also note your device and Android version in the new bug.
I had someone check an S7 and the values are working. It's strange. I'll debug further no problems.
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.
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
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.