Closed Bug 1351048 Opened 3 years ago Closed 3 years ago

SteamVR and Oculus Share Store launch along Firefox

Categories

(Core :: WebVR, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: dmarcos, Assigned: kip)

References

Details

Attachments

(1 file)

If WebVR is enabled each time that Firefox starts it launches the SteamVR and the Oculus Share store apps even though a headset is not connected or VR mode is not engaged.
I am investigating now.
Assignee: nobody → kgilbert
Mozregression points towards Bug 1316279
(In reply to :kip (Kearwood Gilbert) from comment #2)
> Mozregression points towards Bug 1316279

It's possible that this is not the actual cause, as the patch in Bug 1316279 was correcting an issue where WebVR was not initializing at all.

I was bisecting using an Oculus on the latest (v1.13) runtime.  It's possible that something in the runtime has changed the conditions of when to start Oculus home.

Calling InitializeOculusCAPI is not causing Oculus Home to launch.
VRSystemManagerOculus::Init() appears to trigger the launch of Oculus Home.

We may be able to delay calling VRSystemManagerOculus::Init() until GetHMDs() is called.
Although it wasn't possible to automatically return the user to Oculus home without shutting down the browser's GPU process, I have made some fixes to make Firefox play nicer with OpenVR and Oculus home:

- I have refactored the Oculus and OpenVR interfaces in gfx/vr
  so that initialization of the VR libraries only happens once
  a WebVR site is detected.
- The Oculus interface has been cleaned up and updated to unload the Oculus
  runtime library when not in use.
- The browser can now re-connect to Oculus home if it was restarted, without
  restarting the browser.
- We no longer submit a black frame at the end of VR presentation, as this
  appears to be handled by the latest Oculus runtime automatically.
- As we only hold on to the Oculus runtime when needed, this should
  reduce the likelihood of the GPU process being killed by the Oculus
  software updater.
Attachment #8852479 - Flags: review?(dmu)
Comment on attachment 8852479 [details]
Bug 1351048 - Do not load VR libraries until necessary, Oculus cleanup

https://reviewboard.mozilla.org/r/124688/#review127462

r=me, please figure out if you wanna remove ovr_SubmitFrame at stopPresentation()? I think you should keep it. And please do more verification to avoid producing regression bugs before landing. Thanks!

::: gfx/vr/gfxVROculus.cpp:194
(Diff revision 1)
>  #else
> -    libSearchPaths.AppendElement(nsCString("/usr/local/lib"));
> +#error "Unsupported platform!"
> -    libSearchPaths.AppendElement(nsCString("/usr/lib"));
> -    libName.AppendPrintf("libOVRRT%d_%d.so.%d", BUILD_BITS, OVR_PRODUCT_VERSION, OVR_MAJOR_VERSION);
>  #endif
>      

Please remove these redundant spaces.

::: gfx/vr/gfxVROculus.cpp:658
(Diff revision 1)
>    if (!mIsPresenting) {
>      return;
>    }
>    mIsPresenting = false;
>  
> -  ovr_SubmitFrame(mSession, 0, nullptr, nullptr, 0);
> +  // ovr_SubmitFrame(mSession, 0, nullptr, nullptr, 0);

You wanna keep ovr_SubmitFrame() here or remove it? If you remove it, does it work normally for stopping the presentation?
Attachment #8852479 - Flags: review?(dmu) → review+
The try push showed a crashtest being triggered.  I will reproduce and make adjustments to the patch.
(In reply to Daosheng Mu[:daoshengmu] from comment #6)
> > -  ovr_SubmitFrame(mSession, 0, nullptr, nullptr, 0);
> > +  // ovr_SubmitFrame(mSession, 0, nullptr, nullptr, 0);
> 
> You wanna keep ovr_SubmitFrame() here or remove it? If you remove it, does
> it work normally for stopping the presentation?

I intend to remove this.  I will remove the commented version as well to avoid confusion.

The Oculus SDK examples do not call ovr_SubmitFrame with 0 layers.  Originally, this line was added in order to blank out the display when exiting VR presentation.  In my tests with the latest Oculus Runtime, the call to ovr_DestroyTextureSwapChain is blanking the display on its own.

Calling ovr_SubmitFrame when called from the destructor is also a source of crashes (partially fixes Bug 1342430).
(In reply to :kip (Kearwood Gilbert) from comment #7)
> The try push showed a crashtest being triggered.  I will reproduce and make
> adjustments to the patch.

kip, could you share me the link that the crashtest being trigger? I am interested in what crash you meet. AFAIK, our tests are not good at detecting crash at our vr backends like openvr and oculus.
(In reply to Daosheng Mu[:daoshengmu] from comment #9)
> (In reply to :kip (Kearwood Gilbert) from comment #7)
> > The try push showed a crashtest being triggered.  I will reproduce and make
> > adjustments to the patch.
> 
> kip, could you share me the link that the crashtest being trigger? I am
> interested in what crash you meet. AFAIK, our tests are not good at
> detecting crash at our vr backends like openvr and oculus.

This crashtest, which I added earlier was triggering:

gfx/tests/crashtests/1343666.html

I did some experiments today, but haven't yet been able to reproduce on my local machine.

Just in case the test was failing due to unrelated changes, I did another try push earlier today, but saw the same result.

I'll investigate further tomorrow.
(In reply to :kip (Kearwood Gilbert) from comment #11)
> Comment on attachment 8852479 [details]
> Bug 1351048 - Do not load VR libraries until necessary, Oculus cleanup
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/124688/diff/1-2/

Rebased
(In reply to :kip (Kearwood Gilbert) from comment #12)
> Comment on attachment 8852479 [details]
> Bug 1351048 - Do not load VR libraries until necessary, Oculus cleanup
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/124688/diff/2-3/

Removed calls to unused Oculus related preferences that were causing debug builds to crash with an assert.
I have found the cause of the crashtest failure in my patch.  I'm doing another try push to be sure it fixes it, then will be bouncing it back to @daoshengmu for a quick review of the changes

Essentially, there were a couple of prefs being read, which don't actually exist any more.  The function they are in can now be called outside the main thread, causing an assert.

I've simply removed the calls to read the prefs, as they are no longer present anyways

We dynamically load and unload the oculus lib in the compositor thread, but we are also only using the lib from that thread so it is coordinated safely.
I've found the cause of the test_vrDisplay_getFrameData.html test failure when my patch for Bug 1351048 was applied.

The test VR devices were effectively being destroyed before the test was run, and then re-created when getVRDisplays was called.

Since the linearAcceleration was set prior to getVRDisplays, this was reset when the test displays were re-recreated

I've added some code to ensure that RequestVRServiceTest behaves the same as GetVRDisplays, keeping the objects alive.
(Sorry I am not sure how to re-request review without creating a new patch)

I have made some fixes since your r+ to fix test failures found in the try run.

Could you please review interdiff from rev 3 to current (rev 4)?
Flags: needinfo?(dmu)
(In reply to :kip (Kearwood Gilbert) from comment #18)
> (Sorry I am not sure how to re-request review without creating a new patch)
> 
> I have made some fixes since your r+ to fix test failures found in the try
> run.
> 
> Could you please review interdiff from rev 3 to current (rev 4)?

It looks good to me for now. I am interested in what actually happens in VRServiceTest. I will keep investigating by myself.

Thanks.
Flags: needinfo?(dmu)
Blocks: 1341423
Pushed by kgilbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/394603558371
Do not load VR libraries until necessary, Oculus cleanup r=daoshengmu
https://hg.mozilla.org/mozilla-central/rev/394603558371
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
New to bugzilla and not sure whether this fix to Firefox 55 would have applied to Firefox 55.0b3. On a Windows 7 system with SteamVR and Firefox 55.0b3 installed, I encountered issues where some part of SteamVR automatically launches every time I open Firefox, and SteamVR would automatically reopen itself when killed as long as Firefox was open. This in turn caused GPU usage to fly up to over 80% - checked in GPU-Z. I tried tweaking SteamVR settings, though it didn't make a difference despite various suggestions, so I then went into about:config to set dom.vr.openvr.enabled to false, which fixed the issue. SteamVR no longer opened non-stop, GPU usage went back down to <5%, and Firefox's memory usage also dropped from 1.5GB to 500MB.
I started and updated Nightly (56.0a1 (2017-07-04) and created a new profile.
Whenever I visit https://tweakers.net/ SteamVR starts ending in a "Ready" state with the red text "(unresponsive) firefox.exe".

I've disabled it with the above mentioned config entry for now.
(In reply to Alex Haan from comment #23)
> I started and updated Nightly (56.0a1 (2017-07-04) and created a new profile.
> Whenever I visit https://tweakers.net/ SteamVR starts ending in a "Ready"
> state with the red text "(unresponsive) firefox.exe".
> 
> I've disabled it with the above mentioned config entry for now.

It doesn't do this anymore for above url. But a new issue has been opened where I'll report a new URL; https://bugzilla.mozilla.org/show_bug.cgi?id=1389220 .
You need to log in before you can comment on or make changes to this bug.