Open Bug 1412336 Opened 2 years ago Updated 2 years ago

Device orientation in A-Frame demo doesn't work

Categories

(Core :: DOM: Device Interfaces, defect, P2)

All
Android
defect

Tracking

()

Tracking Status
fennec + ---

People

(Reporter: abovens, Assigned: droeh, NeedInfo)

References

()

Details

Attachments

(1 file, 1 obsolete file)

Steps to reproduce:
1. Open Nightly on Android and go to https://ada-a-frame.glitch.me
2. Change the device orientation by moving the device.

Actual result:
Orientation events are not registered, unlike in other browsers (e.g. Chrome)
tracking-fennec: --- → ?
OS: Unspecified → Android
Hardware: Unspecified → All
Version: Firefox 57 → Trunk
Component: General → DOM: Device Interfaces
Product: Firefox for Android → Core
See Also: → 1356921
Hi Dylan, are you the right person to check this issue? Or do you know who I can ask. I am asking you because you fixed a similar bug 1356921. Thank you.
Flags: needinfo?(droeh)
Yeah, I can look into it. A quick investigation shows that a-frame's device orientation isn't working in general in Nightly, but in Release it works fine provided you use a recent enough version of a-frame (update the version in the link to 0.7.0 and it works, or try the demos on https://aframe.io/examples). 

I'll figure out what's causing the breakage in Nightly.
Flags: needinfo?(droeh)
Priority: -- → P2
Assignee: nobody → droeh
tracking-fennec: ? → +
This is happening because dom.vr.enabled is set to true in Fennec nightly, this patch removes that. Randall, I'm flagging you for review because I'm not sure if there's some good reason why we were doing that in the first place.
Attachment #8930480 - Flags: review?(rbarker)
Comment on attachment 8930480 [details] [diff] [review]
Don't set dom.vr.enabled to true in Fennec nightly

Review of attachment 8930480 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/app/mobile.js
@@ -848,4 @@
>  pref("consoleservice.logcat", true);
>  #endif
>  
> -#ifndef RELEASE_OR_BETA

Probably should be:

#if defined(MOZ_ANDROID_GOOGLE_VR)

::: modules/libpref/init/all.js
@@ +5225,4 @@
>  
>  // WebVR is enabled by default in beta and release for Windows and for all
>  // platforms in nightly and aurora.
> +#if defined(XP_WIN) || defined(XP_MACOSX) || !defined(RELEASE_OR_BETA) && !defined(MOZ_WIDGET_ANDROID)

I believe mobile.js will overwrite this so I think this fine. If you enable it in mobile.js when defined(MOZ_ANDROID_GOOGLE_VR).
Attachment #8930480 - Flags: review?(rbarker)
Thanks, I wasn't aware of MOZ_ANDROID_GOOGLE_VR.
Attachment #8930480 - Attachment is obsolete: true
Attachment #8930724 - Flags: review?(rbarker)
Attachment #8930724 - Flags: review?(rbarker) → review+
Pushed by droeh@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5970c2922ed
Do not set dom.vr.enabled to true in Fennec nightly. r=rbarker
Backed out for failing mochitests dom/tests/mochitest/general/test_interfaces.html and dom/vr/test/mochitest/test_vrDisplay_canvas2d.html (VRDisplay not available):

https://hg.mozilla.org/integration/mozilla-inbound/rev/37e8b667e99722c1c1d2ffc68576e2151eac31b4

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c5970c2922ed8e6bfb1b173b350977dd34a80680&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry

Failure log dom/tests/mochitest/general/test_interfaces.html: https://treeherder.mozilla.org/logviewer.html#?job_id=146801715&repo=mozilla-inbound
dom/tests/mochitest/general/test_interfaces.html | VRDisplay should be defined on the global scope

Failure log dom/vr/test/mochitest/test_vrDisplay_canvas2d.html: https://treeherder.mozilla.org/logviewer.html#?job_id=146801758&repo=mozilla-inbound
dom/vr/test/mochitest/test_vrDisplay_canvas2d.html | Finish running WebVR Canvas2D test. - Finish running WebVR Canvas2D test.: navigator.getVRDisplays is not a function
Flags: needinfo?(droeh)
OK, apparently we don't want to just disable that pref on nightly.

Randall, I was talking to snorp about this and he mentioned you had a hack that provides orientation events through webvr, which is basically what we'd need to do to have aframe use the device orientation with dom.vr.enabled = true. Do you think that's a reasonable approach and can you put the hack up somewhere if so?
Flags: needinfo?(droeh) → needinfo?(rbarker)
(In reply to Dylan Roeh (:droeh) from comment #8)
> OK, apparently we don't want to just disable that pref on nightly.
> 
> Randall, I was talking to snorp about this and he mentioned you had a hack
> that provides orientation events through webvr, which is basically what we'd
> need to do to have aframe use the device orientation with dom.vr.enabled =
> true. Do you think that's a reasonable approach and can you put the hack up
> somewhere if so?

Actually, I think the bug is with the WebVR polyfill that AFrame is using. the polyfill seems to be assuming that if the dom has WebVR components then it will be able to get a display which isn't a great assumption. I would recommend either disabling the test and leaving the vr dom off or wait for the WebVR polyfil to be fixed (not likely since it will take months for it to make it to the AFrame fork if the past is any indication).
Flags: needinfo?(rbarker)
In June when the polyfill broke orientation on Firefox Android, it got pushed through polyfill->aframe->release pretty quickly once all the right folks in those projects were all looking at it.

ni? Diego for how we turned it around last time.
Flags: needinfo?(dmarcos)
(In reply to Randall Barker [:rbarker] from comment #9)
> (In reply to Dylan Roeh (:droeh) from comment #8)
> > OK, apparently we don't want to just disable that pref on nightly.
> > 
> > Randall, I was talking to snorp about this and he mentioned you had a hack
> > that provides orientation events through webvr, which is basically what we'd
> > need to do to have aframe use the device orientation with dom.vr.enabled =
> > true. Do you think that's a reasonable approach and can you put the hack up
> > somewhere if so?
> 
> Actually, I think the bug is with the WebVR polyfill that AFrame is using.
> the polyfill seems to be assuming that if the dom has WebVR components then
> it will be able to get a display which isn't a great assumption. I would
> recommend either disabling the test and leaving the vr dom off or wait for
> the WebVR polyfil to be fixed (not likely since it will take months for it
> to make it to the AFrame fork if the past is any indication).

Right, this is pretty much what I suggested to snorp, and he didn't seem happy with either approach. I guess we can talk about it on Wednesday.
I don't think I realized that the pose comes from the display. I agree that fixing the polyfill seems like the best way forward.
The polyfill has a flag `ALWAYS_APPEND_POLYFILL_DISPLAY` which always fires up the polyfill even when native WebVR support exists, in the event there are no native VR displays. It looks like A-Frame[0] is not currently using this flag. I believe this will fix the issue, but keep in mind it's not on by default due to always clobbering the native implementation when on mobile, so this is a decision each consumer of the polyfill must make. Please file an issue on the polyfill[1] if this is not the case -- thank you!

[0] https://github.com/aframevr/aframe/blob/ba02747ad0f23f98a58f4590b6ff75267d7cda75/src/index.js#L4-L10
[1] https://github.com/googlevr/webvr-polyfill
Thanks, Jordan. I'm guessing A-Frame is not going to want to set that flag given that it will clobber a native implementation -- snorp, what are your thoughts here?
Flags: needinfo?(snorp)
I think I discussed with Dylan just disabling WebVR in Firefox since we don't support any actual displays there currently.
Flags: needinfo?(snorp)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #15)
> I think I discussed with Dylan just disabling WebVR in Firefox since we
> don't support any actual displays there currently.

But it can be used with a viewer like cardboard, right?
@droeh they could, it clobbers in the sense of globals (`navigator.getVRDisplays`) but still will natively use the frame data and viewer, etc., where VR is enabled and provides a VRDisplay (Daydream in Chrome, GearVR in Samsung Internet).

It sounds like disabling it would work as well -- regardless, the polyfill should handle the no viewer situation.

Hope this helps!
You need to log in before you can comment on or make changes to this bug.