1316279 - [webvr] WebVR in out-of-process compositing mode.

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Daosheng Mu[:daoshengmu]]

That is a regression bug related to GPU process works (Bug 1264543). We will see “WebVR supported, but no VRDisplays found." from https://webvr.info/samples/04-simple-mirroring.html. If we disable e10s, it will be good.

That is because mVRDisplays and mControllerManagers in VRManager are not initialized successfully when VR manager is running at an individual GPU process.

Comment 1

[Daosheng Mu[:daoshengmu]]

mVRDisplays and mControllerManagers of VRManager are not initialized from VRManager::ManagerInit()

We suppose VRManager::ManagerInit() would be called at GPUParent::Init() when we are launching GPU process. But, it seems to not happen so...

Comment 2

[Daosheng Mu[:daoshengmu]]

I think I find out the root cause. This is because we use gfxPrefs::VREnabled(), !gfxPrefs::VROpenVREnabled for checking if we enable some configs. But gfxPrefs seems to not work in OOP compositing. Do we have any approach to fix this kind of problem?

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

gfxPrefs does work in the GPU process, but Preferences does not... could stuff like this not be working?

http://searchfox.org/mozilla-central/source/gfx/vr/gfxVROpenVR.cpp#103

There could be other XPCOM things missing for VR too, but Preferences is the most obvious. Maybe we should add nsCString support to gfxPrefs and move VR to that.

Comment 4

[Daosheng Mu[:daoshengmu]]

(In reply to David Anderson [:dvander] from comment #3)
> gfxPrefs does work in the GPU process, but Preferences does not... could
> stuff like this not be working?
> 
> http://searchfox.org/mozilla-central/source/gfx/vr/gfxVROpenVR.cpp#103
> 
> There could be other XPCOM things missing for VR too, but Preferences is the
> most obvious. Maybe we should add nsCString support to gfxPrefs and move VR
> to that.

I am not sure why gfxPrefs::VREnabled() and gfxPrefs::VROpenVREnabled are not true because I have adjusted them in about:config, and non-e10s mode is good.

nsCString in Preferences is an another issue. Like you said, I can't get my dll path from http://searchfox.org/mozilla-central/rev/4b6cab91f93c73ae591dafaea40fd5704b41810e/gfx/vr/gfxVROpenVR.cpp#103.

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Daosheng Mu[:daoshengmu] from comment #4)
> (In reply to David Anderson [:dvander] from comment #3)
> > gfxPrefs does work in the GPU process, but Preferences does not... could
> > stuff like this not be working?
> > 
> > http://searchfox.org/mozilla-central/source/gfx/vr/gfxVROpenVR.cpp#103
> > 
> > There could be other XPCOM things missing for VR too, but Preferences is the
> > most obvious. Maybe we should add nsCString support to gfxPrefs and move VR
> > to that.
> 
> I am not sure why gfxPrefs::VREnabled() and gfxPrefs::VROpenVREnabled are
> not true because I have adjusted them in about:config, and non-e10s mode is
> good.
> 
> nsCString in Preferences is an another issue. Like you said, I can't get my
> dll path from
> http://searchfox.org/mozilla-central/rev/
> 4b6cab91f93c73ae591dafaea40fd5704b41810e/gfx/vr/gfxVROpenVR.cpp#103.

Ah, the reason might be something silly. We initialize VRManager here:

http://searchfox.org/mozilla-central/rev/886d5186f0598ab2f8a9953eb5a4dab9750ef834/gfx/ipc/GPUParent.cpp#108

But this occurs before we actually get the gfxPrefs blob, here:

http://searchfox.org/mozilla-central/rev/886d5186f0598ab2f8a9953eb5a4dab9750ef834/gfx/ipc/GPUParent.cpp#147

So I think we need to move VRManager initialization to the end of RecvInit. We still need to fix places that use Preferences, though.

Comment 6

[Daosheng Mu[:daoshengmu]]

(In reply to David Anderson [:dvander] from comment #5)
> (In reply to Daosheng Mu[:daoshengmu] from comment #4)
> > (In reply to David Anderson [:dvander] from comment #3)
> > > gfxPrefs does work in the GPU process, but Preferences does not... could
> > > stuff like this not be working?
> > > 
> > > http://searchfox.org/mozilla-central/source/gfx/vr/gfxVROpenVR.cpp#103
> > > 
> > > There could be other XPCOM things missing for VR too, but Preferences is the
> > > most obvious. Maybe we should add nsCString support to gfxPrefs and move VR
> > > to that.
> > 
> > I am not sure why gfxPrefs::VREnabled() and gfxPrefs::VROpenVREnabled are
> > not true because I have adjusted them in about:config, and non-e10s mode is
> > good.
> > 
> > nsCString in Preferences is an another issue. Like you said, I can't get my
> > dll path from
> > http://searchfox.org/mozilla-central/rev/
> > 4b6cab91f93c73ae591dafaea40fd5704b41810e/gfx/vr/gfxVROpenVR.cpp#103.
> 
> Ah, the reason might be something silly. We initialize VRManager here:
> 
> http://searchfox.org/mozilla-central/rev/
> 886d5186f0598ab2f8a9953eb5a4dab9750ef834/gfx/ipc/GPUParent.cpp#108
> 
> But this occurs before we actually get the gfxPrefs blob, here:
> 
> http://searchfox.org/mozilla-central/rev/
> 886d5186f0598ab2f8a9953eb5a4dab9750ef834/gfx/ipc/GPUParent.cpp#147
> 
> So I think we need to move VRManager initialization to the end of RecvInit.
> We still need to fix places that use Preferences, though.

After moving VRManager initialization to the end of RecvInit, we can use gfxPrefs::VREnabled() and gfxPrefs::VROpenVREnabled. Just remaining the nsCString support of Preferences for getting the dll path.

Comment 7

[Christopher Van Wiemeersch [:cvan]]

What's the expected ETA of fixing this? WebVR is not working for users anymore in Firefox Nightly (unless they know to and aware of having to disable multiprocessing).

Comment 8

[Daosheng Mu[:daoshengmu]]

(In reply to Christopher Van Wiemeersch [:cvan] from comment #7)
> What's the expected ETA of fixing this? WebVR is not working for users
> anymore in Firefox Nightly (unless they know to and aware of having to
> disable multiprocessing).

I just file Bug 1318618 to trace it, and I am going to jump in to fix it. I hope it would be resolved next week.

Comment 9

[Daosheng Mu[:daoshengmu]]

I have confirmed that we have no other issues besides Bug 1318618. After Bug 1318618 is resolved, this bug can be resolved as well.

Comment 10

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1316279 - Part 1: Fix gfxPrefs support for WebVR in out-of-process compositing;

Review commit: https://reviewboard.mozilla.org/r/94902/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/94902/

Comment 11

[Daosheng Mu[:daoshengmu]]

(In reply to Daosheng Mu[:daoshengmu] from comment #10)
> Created attachment 8813487 [details]
> Bug 1316279 - Fix gfxPrefs support for WebVR in out-of-process compositing;
> 
> Review commit: https://reviewboard.mozilla.org/r/94902/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/94902/

Kip, Should I need to modify the way you get ovr_lib in gfxVROculus?, https://dxr.mozilla.org/mozilla-central/rev/0534254e9a40b4bade2577c631fe4cfa0b5db41d/gfx/vr/gfxVROculus.cpp#167.

I am not sure if "dom.vr.ovr_lib_path" and "dom.vr.ovr_lib_name" are unused now? Because I don't see you define it in all.js.

Comment 12

[:kip (Kearwood Gilbert)]

(In reply to Daosheng Mu[:daoshengmu] from comment #11)
> (In reply to Daosheng Mu[:daoshengmu] from comment #10)
> > Created attachment 8813487 [details]
> > Bug 1316279 - Fix gfxPrefs support for WebVR in out-of-process compositing;
> > 
> > Review commit: https://reviewboard.mozilla.org/r/94902/diff/#index_header
> > See other reviews: https://reviewboard.mozilla.org/r/94902/
> 
> Kip, Should I need to modify the way you get ovr_lib in gfxVROculus?,
> https://dxr.mozilla.org/mozilla-central/rev/
> 0534254e9a40b4bade2577c631fe4cfa0b5db41d/gfx/vr/gfxVROculus.cpp#167.
> 
> I am not sure if "dom.vr.ovr_lib_path" and "dom.vr.ovr_lib_name" are unused
> now? Because I don't see you define it in all.js.

We can remove dom.vr.ovr_lib_path and dom.vr.ovr_lib_name.  I don't believe anyone has needed to set these to get Oculus running with the current Oculus runtimes.

Comment 13

[:kip (Kearwood Gilbert)]

(In reply to :kip (Kearwood Gilbert) from comment #12)
> (In reply to Daosheng Mu[:daoshengmu] from comment #11)
> > (In reply to Daosheng Mu[:daoshengmu] from comment #10)
> > > Created attachment 8813487 [details]
> > > Bug 1316279 - Fix gfxPrefs support for WebVR in out-of-process compositing;
> > > 
> > > Review commit: https://reviewboard.mozilla.org/r/94902/diff/#index_header
> > > See other reviews: https://reviewboard.mozilla.org/r/94902/
> > 
> > Kip, Should I need to modify the way you get ovr_lib in gfxVROculus?,
> > https://dxr.mozilla.org/mozilla-central/rev/
> > 0534254e9a40b4bade2577c631fe4cfa0b5db41d/gfx/vr/gfxVROculus.cpp#167.
> > 
> > I am not sure if "dom.vr.ovr_lib_path" and "dom.vr.ovr_lib_name" are unused
> > now? Because I don't see you define it in all.js.
> 
> We can remove dom.vr.ovr_lib_path and dom.vr.ovr_lib_name.  I don't believe
> anyone has needed to set these to get Oculus running with the current Oculus
> runtimes.

We will require one string preference for HTC Vive / OpenVR, "gfx.vr.openvr-runtime":

https://dxr.mozilla.org/mozilla-central/source/gfx/vr/gfxVROpenVR.cpp#103

Comment 14

[:kip (Kearwood Gilbert)]

Comment on attachment 8813487 [details]
Bug 1316279 - Part 1: Fix gfxPrefs support for WebVR in out-of-process compositing;

https://reviewboard.mozilla.org/r/94902/#review95370

r=me with the small nit on alphabetical order in gfxPrefs.h

::: gfx/thebes/gfxPrefs.h:323
(Diff revision 1)
>    DECL_GFX_PREF(Live, "dom.meta-viewport.enabled",             MetaViewportEnabled, bool, false);
>    DECL_GFX_PREF(Once, "dom.vr.enabled",                        VREnabled, bool, false);
>    DECL_GFX_PREF(Once, "dom.vr.oculus.enabled",                 VROculusEnabled, bool, true);
>    DECL_GFX_PREF(Once, "dom.vr.openvr.enabled",                 VROpenVREnabled, bool, false);
>    DECL_GFX_PREF(Once, "dom.vr.osvr.enabled",                   VROSVREnabled, bool, false);
> +  DECL_GFX_PREF(Once, "gfx.vr.openvr-runtime",                 VROpenVRRuntime, std::string, "");

nit: Move openvr-runtime pref up one line to maintain alphabetical order.

Comment 15

[philippeback]

The Oculus CV1 with firmware 78/34a904e8da and the software from Oculus downloaded and installed 23/11 do not work with Nightly 53.0a1 (2016-11-23) (64-bit)

Very frustrating as no WebVR can be tested in the headset. Chromium has the same symptom.

Headset works perfectly in Oculus and SteamVR.

Head tracking works.

Comment 16

[philippeback]

(In reply to philippeback from comment #15)
> The Oculus CV1 with firmware 78/34a904e8da and the software from Oculus
> downloaded and installed 23/11 do not work with Nightly 53.0a1 (2016-11-23)
> (64-bit)
> 
> Very frustrating as no WebVR can be tested in the headset. Chromium has the
> same symptom.
> 
> Headset works perfectly in Oculus and SteamVR.
> 
> Head tracking works.

Dreaded WebVR Supported but no display found.
Come on, that cannot be like this, how are U guys testing this thing?

Comment 17

[:kip (Kearwood Gilbert)]

(In reply to philippeback from comment #16)
> (In reply to philippeback from comment #15)
> > The Oculus CV1 with firmware 78/34a904e8da and the software from Oculus
> > downloaded and installed 23/11 do not work with Nightly 53.0a1 (2016-11-23)
> > (64-bit)
> > 
> > Very frustrating as no WebVR can be tested in the headset. Chromium has the
> > same symptom.
> > 
> > Headset works perfectly in Oculus and SteamVR.
> > 
> > Head tracking works.
> 
> Dreaded WebVR Supported but no display found.
> Come on, that cannot be like this, how are U guys testing this thing?

Automated testing to reduce chances of regressions like this is a high priority in the next quarter.  For now, may I suggest trying Firefox Dev Edition which is a bit farther from the bleeding edge to reduce your exposure?  WebVR and Oculus support are enabled by default in Dev Edition as well.

Comment 18

[philippeback]

Attachment: 24-11-16 09-49-40.png

That's where the current Firefox Dev Edition I downloaded this morning crashes in the panorama WebVR sample.

The no display message is gone but same "Firefox takes a long time to load".

Comment 19

[philippeback]

Ok loaded Dev Edition, loaded the WebVR panorama sample. No more No Display but "Firefox takes a long time to load" in the headset.

Also, see https://bug1316279.bmoattachments.org/attachment.cgi?id=8814006 for a screenshot in VS2015 whith symbols I tried to add from mozilla. But it still had an issue there b/c of mozalloc_abort.cpp not being found or something.

I dug into the disassembly, there seems to be something usable at the bottom.

Comment 20

[philippeback]

Attachment: FFX Crash in Entering VR

Comment 21

[:kip (Kearwood Gilbert)]

(In reply to philippeback from comment #20)
> Created attachment 8814007 [details]
> FFX Crash in Entering VR

Thanks for reporting the crashes.  Could we move this to a separate bug so it can be tracked once this bug with e10s compatibility has been closed?

Comment 22

[philippeback]

It now works in Nightly with:

about:config

browser.tabs.remote.autostart.2 set to false.
openvr.enabled set to false


Enable multi process Nightly unchecked

Then I was able to use all samples, including stuff from aframe.io

Life is great now. Hope to get no further regressions.

Ah yes, spatial sound didn't sound so spatial. Is there a more comprehensive test I can run?

Comment 23

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8813487 [details]
Bug 1316279 - Part 1: Fix gfxPrefs support for WebVR in out-of-process compositing;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/94902/diff/1-2/

Comment 24

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1316279 - Part 2: Fix gfxPrefs::PrefGet string when using default value;

Review commit: https://reviewboard.mozilla.org/r/95608/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/95608/

Comment 25

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8813487 [details]
Bug 1316279 - Part 1: Fix gfxPrefs support for WebVR in out-of-process compositing;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/94902/diff/2-3/

Comment 26

[Daosheng Mu[:daoshengmu]]

(In reply to Daosheng Mu[:daoshengmu] from comment #24)
> Created attachment 8814334 [details]
> Bug 1316279 - Part 2: Fix gfxPrefs::PrefGet string when using default value;
> 
> Review commit: https://reviewboard.mozilla.org/r/95608/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/95608/

dvander, I find a bug at gfxPrefs::PrefGet string when users didn't set the value. Please help me review it. thanks.

Comment 27

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8814334 [details]
Bug 1316279 - Part 2: Fix gfxPrefs::PrefGet string when using default value;

https://reviewboard.mozilla.org/r/95608/#review96148

::: gfx/thebes/gfxPrefs.cpp:200
(Diff revision 1)
>    MOZ_ASSERT(IsPrefsServiceAvailable());
>  
>    nsAdoptingCString result;
>    Preferences::GetCString(aPref, &result);
> +
> +  if (result.IsEmpty())

nit: need braces here

Comment 28

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8814334 [details]
Bug 1316279 - Part 2: Fix gfxPrefs::PrefGet string when using default value;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/95608/diff/1-2/

Comment 29

[Daosheng Mu[:daoshengmu]]

Follow Comment 28 to add a brace at if(result.IsEmpty())

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48164669baec
Part 1: Fix gfxPrefs support for WebVR in out-of-process compositing; r=kip
https://hg.mozilla.org/integration/autoland/rev/211067e97639
Part 2: Fix gfxPrefs::PrefGet string when using default value; r=dvander

Comment 31

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/48164669baec
https://hg.mozilla.org/mozilla-central/rev/211067e97639

Comment 32

[philippeback]

Question: Are the following settings

about:config

browser.tabs.remote.autostart.2 set to false.
openvr.enabled set to false

Enable multi process Nightly unchecked

Still to be put that way?

Basically, how can I test in Nightly with a fresh set of settings?

Comment 33

[Daosheng Mu[:daoshengmu]]

(In reply to philippeback from comment #32)
> Question: Are the following settings
> 
> about:config
> 
> browser.tabs.remote.autostart.2 set to false.
> openvr.enabled set to false
> 
> Enable multi process Nightly unchecked
> 
> Still to be put that way?
> 
> Basically, how can I test in Nightly with a fresh set of settings?

Hi,

You can help us test it by

browser.tabs.remote.autostart.2 set to false.
openvr.enabled set to true

Enable multi process Nightly checked

Please feel free to give us feedback.

Comment 34

[philippeback]

Interestingly,the Enable multi process Nightly is grayed out in the Nightly I have.

53.0a1 (2016-11-30) (64-bit)

Comment 35

[philippeback]

I can enter VR and leave it without trouble with these settings:

browser.tabs.remote.autostart.2 set to false.
openvr.enabled set to true


But cannot check the Enable multi process Nighly checkbox, sorry.