Closed Bug 1287944 Opened 4 years ago Closed 3 years ago
[webvr] Should return to Oculus home after exit
59 bytes, text/x-review-board-request
48.95 KB, patch
|Details | Diff | Splinter Review|
The browser should return control back to Oculus home after exitPresent(). The HMD displays black until browser is completely shutdown. This is an issue when users want to continue to browser regular 2D content. Using try builds: http://email@example.com/try-win64/firefox-50.0a1.en-US.win64.zip
After exitPresent(), the browser renders a black frame to clear the headset. In the future, with backwards compatibility, we could render a projection of the 2d view of the page when it is not presenting WebVR content. Unfortunately, the only way to release the headset and return to Oculus home is to shut down the process that uses the Oculus API or for another VR application to take control from us. The user may also exit using the Oculus remote. We have some options here, which may be combined for the best experience: - Implement backwards compatibility for non-WebVR content, which would be presented when WebVR content is not active - Present UX on the main display indicating that the browser is controlling the display and how to return to Oculus Home. - Move the Gecko VR Compositor functions and Oculus API calls into a separate process that can be shut down without closing the browser. - Launch Firefox from Oculus Home and shut down when the Oculus platform asks us to. (We can still launch this process from regular desktop Firefox)
Component: General → WebVR
Product: Firefox → Core
Hi Casey! Is this still happening for you? If so, I'll take a deeper look.
I have reproduced this on the current Nightly. Something has regressed, preventing ovr_shutdown() from being called when all WebVR content has been closed. Also see Bug 1260937 where we will improve the session management to interoperate better with Oculus Home.
With this patch, I am now able to close and re-start Oculus home even during an active WebVR session. Firefox will respond to the ShouldQuit flag and will completely unload the Oculus libraries. Additionally, it is now possible to return to Oculus Home using the in-vr system menu and then re-start a WebVR site. Now that we aren't so "sticky" in holding on to the Oculus OVR lib, we should hopefully no longer have our process killed when Oculus runtimes are being updated. A couple of prefs have been added to control the interaction with Oculus Home: // Minimum number of milliseconds after content has stopped VR presentation // before the Oculus session is re-initialized to an invisible / tracking // only mode. If this value is too high, users will need to wait longer // after stopping WebVR presentation before automaticaclly returning to the // Oculus home interface. (They can immediately return to the Oculus Home // interface through the Oculus HUD without waiting this duration) // If this value is too low, the Oculus Home interface may be visible // momentarily during VR link navigation. pref("dom.vr.oculus.present.timeout", 10000); // Minimum number of milliseconds that the browser will wait before // reloading the Oculus OVR library after seeing a "ShouldQuit" flag set. // Oculus requests that we shut down and unload the OVR library, by setting // a "ShouldQuit" flag. To ensure that we don't interfere with // Oculus software auto-updates, we will not attempt to re-load the // OVR library until this timeout has elapsed. pref("dom.vr.oculus.quit.timeout", 30000);
Comment on attachment 8880657 [details] Bug 1287944 - Improve interaction with Oculus Home https://reviewboard.mozilla.org/r/152004/#review159826 LGTM. Just need to fix some nits. Good Job! ::: gfx/vr/gfxVROculus.h:52 (Diff revision 6) > + void StopPresentation(); > + void StopTracking(); > + bool IsQuitTimeoutActive(); > + already_AddRefed<layers::CompositingRenderTargetD3D11> GetNextRenderTarget(); > + ovrTextureSwapChain GetSwapChain(); > + Please remove this space. ::: gfx/vr/gfxVROculus.h:66 (Diff revision 6) > + RefPtr<ID3D11Device> mDevice; > + // The timestamp of the last time Oculus set ShouldQuit to true. > + TimeStamp mLastShouldQuit; > + // The timestamp of the last ending presentation > + TimeStamp mLastPresentationEnd; > + Please remove this space. ::: gfx/vr/gfxVROculus.h:68 (Diff revision 6) > + TimeStamp mLastShouldQuit; > + // The timestamp of the last ending presentation > + TimeStamp mLastPresentationEnd; > + > + ~VROculusSession(); > + same here. Please remove the redundnant space
Attachment #8880657 - Flags: review?(dmu) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/f9b033c61423 Improve interaction with Oculus Home r=daoshengmu
Windows debug builds started failing like https://treeherder.mozilla.org/logviewer.html#?job_id=112393140&repo=autoland Backed out in https://hg.mozilla.org/integration/autoland/rev/9ed45070606fec2a6167e97d5f300a7f22e8b712
Thanks Wes, I will reproduce, correct, and then re-land.
VROculusSession is now allocated within VRSystemManagerOculus::GetHMDs, matching the prior functionality, ensuring the constructor is called on the same thread. Daosheng: Could you please review the interdiff before I re-land?
(In reply to :kip (Kearwood Gilbert) from comment #18) > VROculusSession is now allocated within VRSystemManagerOculus::GetHMDs, > matching the prior functionality, ensuring the constructor is called on the > same thread. > > Daosheng: Could you please review the interdiff before I re-land? r=me. LGTM. thanks.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/ededddfadca7 Improve interaction with Oculus Home r=daoshengmu
Is this something we want on 55 too?
(In reply to Ryan VanderMeulen [:RyanVM] from comment #22) > Is this something we want on 55 too? I believe this is worth uplifting to 55 as well. I'll rebase the patch and add a request here.
Comment on attachment 8888058 [details] [diff] [review] Bug 1287944 - Improve interaction with Oculus Home (Beta Uplift) Approval Request Comment [Feature/Bug causing the regression]: Bug 1250244 (Implement WebVR) [User impact if declined]: When a user is visiting a WebVR site while their Oculus runtime software updates, the update will terminate the Firefox GPU process to unlock its files. This results in crash reports and a bad user experience as they will need to restart the browser to access any other VR content. [Is this code covered by automated tests?]: This fix affects use cases that can not be simulated by our software-only puppet VR test driver. This fix must be verified manually on a computer with Oculus VR hardware attached. [Has the fix been verified in Nightly?]: Yes, the crash no longer occurs in Nightly and the browser releases the headset as needed to access other VR content. [Needs manual test from QE? If yes, steps to reproduce]: Yes, STR: Test 1: - (Ensure that the Oculus hardware and software is configured on the test machine) - Visit https://webvr.info/samples/03-vr-presentation.html - Press the "Enter VR" button on the page - Verify that the WebVR content is visible in the headset. - Close the Oculus software, while staying in VR. - Refresh the web page. - Firefox should not crash. Test 2: - (Ensure that the Oculus hardware and software is configured on the test machine) - Visit https://webvr.info/samples/03-vr-presentation.html - Press the "Enter VR" button on the page - Verify that the WebVR content is visible in the headset. - Do not click "Exit VR", leave it running in the headset - Use the Oculus Touch controllers or an XBox One controller to return to Oculus Home. - Launch a non-webvr app from Oculus Home - Firefox should not crash, and you should be able to access non-webvr apps without shutting down Firefox. [List of other uplifts needed for the feature/fix]: N/A [Is the change risky?]: Medium risk. [Why is the change risky/not risky?]: This fix added and changed many lines of code, but was self contained within code that will only activate with Oculus VR hardware attached after visiting a WebVR site. [String changes made/needed]: No string changes
Attachment #8888058 - Flags: approval-mozilla-beta?
Comment on attachment 8888058 [details] [diff] [review] Bug 1287944 - Improve interaction with Oculus Home (Beta Uplift) webvr/oculus fix, beta55+
Attachment #8888058 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.