Closed Bug 1375693 Opened 8 years ago Closed 8 years ago

gfx/vr/openvr/src/envvartools_public.cpp:29:2: error: "Unsupported Platform"

Categories

(Core :: WebVR, defect)

Product:
Core
Component:
WebVR
Platform:
Unspecified
FreeBSD
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla56
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed

People

(Reporter: jbeich, Assigned: jbeich)

References

Details

(Keywords: regression)

Attachments

(1 file)

Bug 1375693 - Don't build OpenVR on Tier3 due to lack of open source runtime.
59 bytes, text/x-review-board-request
kip
: review+
Details
$ echo "ac_add_options --enable-warnings-as-errors" >>.mozconfig $ ./mach build [...] In file included from gfx/vr/openvr/src/dirtools_public.cpp:4: gfx/vr/openvr/src/pathtools_public.h:127:2: error: "Unknown platform for PLATSUBDIR" [-Werror,-W#warnings] #warning "Unknown platform for PLATSUBDIR" ^ 1 error generated. [...] gfx/vr/openvr/src/envvartools_public.cpp:29:2: error: "Unsupported Platform" #error "Unsupported Platform" ^ gfx/vr/openvr/src/envvartools_public.cpp:44:2: error: "Unsupported Platform" #error "Unsupported Platform" ^ 2 errors generated.
Probably any ELF platform can use Linux code. /proc/self/exe can be replaced by getexecname() on Solaris, KERN_PROC_PATHNAME on DragonFly, FreeBSD, NetBSD and /proc/curproc/file on OpenBSD. http://searchfox.org/mozilla-central/rev/3291398f10dc/gfx/vr/openvr/src/pathtools_public.cpp#56
Assignee: nobody → kgilbert
getexecname() looks like an alternative spelling for getprogname(), __progname, program_invocation_name, or argv[0]. It doesn't qualify for /proc/self/exe replacement unlike /proc/self/path/a.out.
Comment on attachment 8880642 [details] Bug 1375693 - Don't build OpenVR on Tier3 due to lack of open source runtime. https://reviewboard.mozilla.org/r/151976/#review156972 This LGTM; thanks for patching it. It would have also been an option to disable openvr on platforms that it doesn't support. (This is 3rd party code) You have effectively ported OpenVR to the new platforms, which may be of value beyond the Firefox codebase. Perhaps we could consider submitting these changes upstream: https://github.com/ValveSoftware/openvr If not accepted upstream, we should update the gfx/vr/openvr/README.mozilla.md file to ensure that these changes are not overwritten in later OpenVR updates (eg. Bug 1371845, which will land soon) These things can happen later; however, and can get this landed first.
Attachment #8880642 - Flags: review?(kgilbert) → review+
Blocks: 1371845
(In reply to :kip (Kearwood Gilbert) from comment #4) > It would have also been an option to disable openvr on platforms that it > doesn't support. (This is 3rd party code) Who is going to maintain such a code path once Android catches on? Lack of bug 1371159 is already PITA enough. > Perhaps we could consider submitting these changes upstream: Done and added a reference in the commit message for the ping-back via mozilla/gecko-dev. > If not accepted upstream, we should update the > gfx/vr/openvr/README.mozilla.md file to ensure that these changes are not > overwritten in later OpenVR updates (eg. Bug 1371845, which will land soon) Anyone else who updates OpenVR can forget about this bug. Should I really wait for upstream to reject/accept?
Flags: needinfo?(kgilbert)
Comment on attachment 8880642 [details] Bug 1375693 - Don't build OpenVR on Tier3 due to lack of open source runtime. Existing runtimes seem closed, so exposing OpenVR is kinda pointless. Let's revert to the previous behavior for now.
Flags: needinfo?(kgilbert)
Attachment #8880642 - Flags: review+ → review?(kgilbert)
(In reply to Jan Beich from comment #1) > Probably any ELF platform can use Linux code. /proc/self/exe can be replaced > by getexecname() on Solaris, KERN_PROC_PATHNAME on DragonFly, FreeBSD, > NetBSD and /proc/curproc/file on OpenBSD. There's no /proc on openbsd, nor a reliable way to get the full path to a process, and that's by design. I second jan here, if there's no opensource runtime available for us poor third-party platforms, then there's no point in trying to support that monstrosity. But then, we'll get new bugs when m-c starts thinking that openvr is available on all platforms, and its the profiler stuff all over again.
Although anyone can write an open source driver for a headset and controllers for OpenVR, this hasn't materialized yet outside of Linux, Mac, and Windows. OpenVR support in Firefox outside of these three is probably only useful for platform developers currently. On Android, OpenVR is not yet in use. Those platforms are targeted with either the "Oculus Mobile Software Development Kit" for Samsung Gear VR or Google Daydream API's. OSVR is fully open source, including the hardware design, schematics, drivers, and runtimes. It is feasible to port this to more platforms; however, work would need to be done outside of the browser before it could provide benefit.
Comment on attachment 8880642 [details] Bug 1375693 - Don't build OpenVR on Tier3 due to lack of open source runtime. https://reviewboard.mozilla.org/r/151976/#review157346 This LGTM, Thanks!
Attachment #8880642 - Flags: review?(kgilbert) → review+
Keywords: checkin-needed
Assignee: kgilbert → jbeich
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/84467f2e7e67 Don't build OpenVR on Tier3 due to lack of open source runtime. r=kip
Keywords: checkin-needed
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
https://hg.mozilla.org/mozilla-central/rev/84467f2e7e67
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Blocks: 1473881
Depends on: 1497379
No longer depends on: 1497379
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: