Refactor gfxVROculus.cpp to use gfxVRExternal interface

RESOLVED FIXED in Firefox 64

Status

()

defect
RESOLVED FIXED
a year ago
8 months ago

People

(Reporter: kip, Assigned: kip)

Tracking

59 Branch
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

a year ago
As part of an iterative implementation to move the VR api interfaces to their own process, we will first update the backend's in gfx/vr/... to communicate entirely with the rest of Gecko through the VRExternalShmem struct, defined in gfx/vr/external_api/moz_external_vr.h

This implementation will follow the conversion of gfxVROpenVR.cpp in Bug 1466699, which will lay much of the foundation needed.
Assignee

Updated

a year ago
Blocks: 1362578
Assignee

Updated

11 months ago
Blocks: 1473399
Assignee

Comment 4

8 months ago
Fixed android build failure found in try push.  Updated try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=98ef39faa749b9005fcd0f01e80899f1b4a0481e
Please check it with dom.vr.external;true, dom.vr.service;true, dom.vr.process;true to see if there is any regression.
Assignee

Comment 6

8 months ago
Corrected nits in review feedback
Moved #ifdef to fix macos and linux builds

Updated try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5c7c54f193f7384db16b32b1ccd9d0dbe023080f

I am now doing some testing with physical VR hardware to verify that no regressions have occurred, as per comment 5.
Assignee

Comment 7

8 months ago
Try run found another macOS build failure.  Fixed; updated try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd5dab015c69e217a04a31d2c36746005b991153
Assignee

Comment 8

8 months ago
Fixed an assertion found in a try run that also enabled the VR service thread by default.  Fixed; updated try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6afe0455e4f83fff74161d86af0ac6e6a3b953df

Comment 9

8 months ago
Pushed by kgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f536c1abf26e
Refactor gfxVROculus.cpp to use gfxVRExternal interface, r=daoshengmu

Comment 10

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f536c1abf26e
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64

Comment 11

8 months ago
Backout by dvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32976a484a5e
Backed out changeset f536c1abf26e for causing build bustage at /build/build/src/gfx/vr/service/OculusSession.cpp
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla64 → ---
Depends on: 1498550
Assignee

Comment 12

8 months ago
This was backed out due to MSVC builds returning to Tier 1 and reporting warnings:

10:45:33     INFO -  z:/build/build/src/gfx/vr/service/OculusSession.cpp(189): error C2220: warning treated as error - no 'object' file generated
10:45:33     INFO -  z:/build/build/src/gfx/vr/service/OculusSession.cpp(189): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)
10:45:33     INFO -  z:/build/build/src/gfx/vr/service/OculusSession.cpp(192): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)

Change to fix this:

< +    aControllerState.buttonPressed |= (1 << aButtonIdx);
---
> +    aControllerState.buttonPressed |= ((uint64_t)1 << aButtonIdx);
653c653
< +    aControllerState.buttonTouched |= (1 << aButtonIdx);
---
> +    aControllerState.buttonTouched |= ((uint64_t)1 << aButtonIdx);
Attachment #9016807 - Flags: review?(dmu)
Comment on attachment 9016807 [details] [diff] [review]
Bug 1466700 - Refactor gfxVROculus.cpp to use gfxVRExternal interface (fixed for reland)

LGTM.
Attachment #9016807 - Flags: review?(dmu) → review+
Assignee

Comment 14

8 months ago
The updated patch, "Bug 1466700 - Refactor gfxVROculus.cpp to use gfxVRExternal interface (fixed for reland)" has been updated to correct the MSVC warning and is ready for re-landing.
Flags: needinfo?(ryanvm)

Comment 15

8 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/06020ce3e33a
Refactor gfxVROculus.cpp to use gfxVRExternal interface, r=daoshengmu
I'll keep an eye on the builds on autoland and re-land bug 1473399 once they're green. Thanks for the quick rebase and sorry again for the extra hassle!
Flags: needinfo?(ryanvm)
For posterity, given that the backout and relanding was all contained to the autoland repo (and will end up merging back to m-c in the same push eventually), be aware the the Nightly builds starting on m-c *now* (i.e. the second round of 20181012 Nightlies) will still contain the changes from the merge back in comment 10. In case it comes up in archaeology down the road :)

Comment 18

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/06020ce3e33a
Status: REOPENED → RESOLVED
Last Resolved: 8 months ago8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Depends on: 1498789
You need to log in before you can comment on or make changes to this bug.