1391181 - deviceorientation on android is very precise but flips *often* on semi predictable moments

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[jan klopper]

Attachment: mozilla.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170612122310

Steps to reproduce:

I' working on a VR 'game' in which we track the rotation of all axis by tracking the deviceorientation events and storing them.

When having the device in landscape mode look straight forward as if it would be in a vr goggle / cardboard.
* Look to the left
* look to the right
* Look up
* Look down
* centered again Tilt head left
* centered again Tilt head right

We use three.js for our graphical vr experience, but run a test script outside of three.js which shows the same issues.

We tried various devices, running various android versions and various firefox versions up to the newest stable releases of both.

To reproduce, load the attachment, hold your phone as if in vr / landscape mode in front of you and play around the horizon, looking around, and tilting.
Once you see a flip in the upper graph you might see it fixed in the lower graph.
Pausing the measurements can be done by tapping either one of the graphs, same with continuing the measurements.

When running the same code in the three.js deviceorientation handler the whole world flips up side down regularly and left becomes right, and right becomes left.

Chrome seems to not have these flips. But has the worst repeatability when it comes to returning any degrees at all.


Actual results:

What we see if that especially when looking almost straight ahead tilting the head+phone over to the left or right will dramatically flip the alpha and beta axis if we also look slightly up and down and thus crossing the horizon.

We took a few hours to try and normalize and recognize these flips but there's some underlying math issue which I'm not familiar enough with.


Expected results:

We'd expect the rotation coming out of the deviceorientation to be stable, and not flip at various seemingly unpredictable points.

Comment 1

[jan klopper]

An online version of the attached test is available here:

https://fysiovr.nl/test

Comment 2

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

We simply forward the data we get from Android. Do other browsers exhibit similar behavior?

Comment 3

[jan klopper]

James, Thanks for responding.

I've tested Chrome, but I shall test others and report back if need be.

Chrome does not exhibits the flips and shifts (half pi, quart pi, inverse, inverse + half pi) that Firefox does, but gives completely worthless data on the same device.

A Firefox test would pretty accurately (within 1deg) return rotation data, but will exhibit flips halfway through.
A chrome test will non-deterministically return anywhere between 20% and 70% of an error on any given rotation.

If you open the url in my second comment on both browsers you should see what I mean pretty quick by looking around.
My guess is that Chrome probably does some smoothing of the sensor data that goes horribly wrong if you need actual accurate datapoints, firefox probably ignores some Math which should have been applied to the raw sensor data.

Comment 4

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

I can reproduce the weird flips on my Pixel. Dylan, can you look?

Comment 5

[Dylan Roeh (:droeh) (he/him)]

Yeah, this definitely looks like something is not quite right in the quaternion -> euler angles conversion. Eugen, it looks like you wrote this (RotationVectorToOrientation in nsDeviceSensors.cpp), do you have a link to wherever you found this conversion originally? Otherwise I'll probably just rewrite it from scratch, trying to sort out exactly what condition is being mishandled here is a pain, since this doesn't resemble the quaternion->euler conversion I'm familiar with.

Comment 6

[jan klopper]

Thanks guys, If you can get this going in the right direction, ill pitch in and create a javascript shim to try and do the same calculations for older firefox versions. Ill make sure Three.js also accepts that in their orientationcontroller class so VR starts working for more people.

In the test I've already tried to fix some of the issues by 'detecting' the issues and inversing the angles etc, but I'm not familiar with most of that Math.

Comment 7

[Eugen Sawin [:esawin]]

(In reply to Dylan Roeh (:droeh) from comment #5)
> Yeah, this definitely looks like something is not quite right in the
> quaternion -> euler angles conversion. Eugen, it looks like you wrote this
> (RotationVectorToOrientation in nsDeviceSensors.cpp), do you have a link to
> wherever you found this conversion originally? Otherwise I'll probably just
> rewrite it from scratch, trying to sort out exactly what condition is being
> mishandled here is a pain, since this doesn't resemble the quaternion->euler
> conversion I'm familiar with.

It should be the "standard" conversion but with reduced operations and reusable intermediates. I don't remember the original source, although I remember further optimizing it, which may have introduced these artifacts.

If the flips originate from the conversion, it would probably be best to rewrite the conversion and test against it.

Comment 8

[Dylan Roeh (:droeh) (he/him)]

Attachment: Rewrite quaternion conversion code

So there was definitely something we were doing incorrectly with how we are computing beta -- it's stuck in [-90, 90] and changes directions within that interval mid-rotation.

I ended up just rewriting the quaternion conversion code to address that, and it should fix any erroneous jumps I believe. There are still discontinuities present in the test file, but every one I can produce is a jump between two sets of angles that represent the same rotation in 3d.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8411ec11c95e
Reimplement the quaternion to Tait-Bryan conversion for device orientation events. r=snorp

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8411ec11c95e

Comment 11

[jan klopper]

I've downloaded Firefox 57.0a1 nightly for 2017-09-03, but It looks like I still have similar flips and issues that I had earlier. They occur specifically when holding the device in landscape mode, back the horizon, and then rotating the device by moving the short edges up and down.

Does that version include the new code, or are these the discontinuities that Dylan mentions? 

Specifically:
I see the Alpha move up or down by 180deg.
I see the gamma move down by180deg.
I see beta flip over 90deg and thus inversing.

Comment 12

[Dylan Roeh (:droeh) (he/him)]

(In reply to jan klopper from comment #11)
> I've downloaded Firefox 57.0a1 nightly for 2017-09-03, but It looks like I
> still have similar flips and issues that I had earlier. They occur
> specifically when holding the device in landscape mode, back the horizon,
> and then rotating the device by moving the short edges up and down.
> 
> Does that version include the new code, or are these the discontinuities
> that Dylan mentions? 
> 
> Specifically:
> I see the Alpha move up or down by 180deg.
> I see the gamma move down by180deg.
> I see beta flip over 90deg and thus inversing.

In general, I think (alpha, beta, 90) and (alpha+180, 180-beta, -90) represent the same angle (in degrees, of course). The arithmetic is modulo the appropriate range ([0,360) for alpha, [-180,180) for beta, and [-90,90) for gamma). So, for example, (90, 45, 90) and (270, 135, -90) represent the same rotation with a 90 degree difference in beta. It may be helpful to refer to the spec[0] when visualizing this.

[0]: https://w3c.github.io/deviceorientation/spec-source-orientation.html#deviceorientation

Comment 13

[Dylan Roeh (:droeh) (he/him)]

Comment on attachment 8903291 [details] [diff] [review]
Rewrite quaternion conversion code

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: Incorrect device orientation events
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No
[Why is the change risky/not risky?]: This simply fixes the way we calculate the rotation angles from a quaternion representation.
[String changes made/needed]:

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Dylan, is this crucial to get into the 56 release?  Can it ride the trains, and wait till 57 beta?
Would we be risking breaking how rotation works for other applications or the general FxA population?

Comment 15

[Dylan Roeh (:droeh) (he/him)]

Well, device orientation events are broken without this patch: they give a beta angle that in many cases is incorrect. So there is a risk that there's some breakage in my patch that I missed, of course, but that should be weighed against the certain breakage in existing code. As for whether or not it's crucial to get this into 56... I'm not sure crucial is the word I'd use, at the moment it seems like apps using device orientation events are pretty few and far between. I'm in favor of getting this in ASAP so that we do not start seeing developers trying to workaround our old incorrect behavior.

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8903291 [details] [diff] [review]
Rewrite quaternion conversion code

OK, thanks for the further explanation! 
That makes sense, let's give this a try on beta 10.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/42c4fe4f6b40