ValveSoftware / openvr

OpenVR SDK
http://steamvr.com
BSD 3-Clause "New" or "Revised" License
6.07k stars 1.28k forks source link

[BUG]: Broken `libopenvr` build #1594

Open okawo80085 opened 2 years ago

okawo80085 commented 2 years ago

Issue

Since last release, libopenvr does not build!

In the release include paths got changed:

- #include "%header_name%.h"
+ #include <vrcore/%header_name%.h>

But the directory structure did not, the headers remained in src/vrcommon/, even tho its added as an include directory in cmake, its not possible to get the headers from using those include paths

However that's not where the story ends, there is more.... missing headers! in src/vrcommon/strtools_public.cpp other then incorrect include paths, the newly added header assert.h is not present in the source tree

//========= Copyright Valve Corporation ============//
- #include "strtools_public.h"
+ #include <vrcore/strtools_public.h>
  #include <string.h>
  #include <stdio.h>
  #include <stdlib.h>
@@ -9,11 +9,30 @@
  #include <functional>
  #include <locale>
  #include <codecvt>
+ #include <vrcore/assert.h>

...

Which if added to src/vrcommon/ will break the std assert.h include

Proposed resolution

Fixing the include paths is easy enough, either change vrcore to vrcommon or rename the src/vrcommon directory to src/vrcore (and also change the paths for it in cmake)

Fixing the absence of assert.h though... From what i can see in src/vrcommon/strtools_public.cpp the only function that's supposed to be defined in assert.h is the following

void AssertMsg(bool, const char* msg);
// or
void AssertMsg(bool, const std::string msg);

So a simple header can be made where a single inline assert function is defined, either using std assert and io or anything else. Or getting the actual header that had to be there would be nice, because there is no way in hell we'll be able to guess what Valve intended that header to be!

Oh and also this header needs to be renamed to literally anything but assert.h (e.g. vr_assert.h or vrassert.h), it will break the include of std assert.h, and since this is done in a library, it may potentially also break the same include in this library's user projects! So change the god damn name!

dignakov commented 2 years ago

I'm seeing the same problems. The repo does not build as it is currently.

okawo80085 commented 2 years ago

An interesting finding from #425, turns out that libopenvr_api is not meant to be built by us... the shipped with this repo version has more symbols then we have source code

Which is plain wrong! If there are proprietary components why are they not in their own lib(as pointed out in #425)? This practically cuts the ability for us to maintain this library, no wonder the build is missing sources, most of the code crammed into this library is not present here :/

ChristophHaag commented 2 years ago

All the code you need to use the documented API is in the library. so you can build it yourself for your own VR applications.

The proprietary extensions are only used by internal SteamVR tools like vrdashboard (possibly SteamVR Home?)

okawo80085 commented 2 years ago

All the code you need to use the documented API is in the library. so you can build it yourself for your own VR applications.

Well yes, but actually no, as i initially said the current master branch does not build, its missing 1 source file and most include paths are broken

While yes you can fix it yourself and build it, not everyone can or is willing to do that, i hope its not crazy to expect the sources in this repo to be buildable...

okawo80085 commented 2 years ago

So there might be a way to split the internal part of the library from the open source part... I doubt it's even possible, but hey I can at least try, the basic idea is to obj dump the pre-built version and the freshly built open source version, get a diff of the two, obj copy the diff into a libopenvr_interenal.o (for example) then when compiling the open source version just statically link libopenvr_internal.o in the end an vualá open source part rebuilt with the internal part un-touched.

Many ways this can go wrong, or not work at all, worth a shot IMO, considering how little alternatives there are :/

nightst4r commented 2 years ago

So OpenVR has been broken for over a year and no one cared? thats.. what? Is this not a serious project? Is there something new we should be using instead?

ZNixian commented 2 years ago

If it's of any use to anyone, I've written a couple of tiny that can be used to fix this, without modifying any files inside the openvr repo itself (assuming you're using your own buildscript) - this is useful if you're trying to build the openvr repo as a submodule, for example.

https://gitlab.com/znixian/openvr-tests/-/tree/master/src/openvrfixes

ZNixian commented 2 years ago

So OpenVR has been broken for over a year and no one cared? thats.. what? Is this not a serious project? Is there something new we should be using instead?

Indeed it has. It's just that almost nobody actually compiles this from source, they just take the DLLs that are checked into this repo, which contain a bunch of Valve's proprietary bits and pieces.

It's worth noting that all the important stuff is in SteamVR itself (or that one incomplete 3rd-party implementation I made), and this repo is mostly just a couple of (relatively poorly specified) API headers.

ManaMarkTea commented 2 years ago

Oh my. Okay good to know. Should I be using StreamVR or the binaries in the repo, I was compiling it, but if I don't have to.. I could skip that step.

ZNixian commented 2 years ago

Should I be using StreamVR or the binaries in the repo

The whole OpenVR/SteamVR combination works as two parts:

So you're going to be using SteamVR either way, you can just either compile the code that loads SteamVR into your programme or get the pre-compiled version. If you're just trying to make a game I'd suggest grabbing the binaries from this repo since that's what everyone else uses (for example, they're the ones distributed with the OpenVR Unity plugin).

ManaMarkTea commented 2 years ago

Cool, sounds good thanks =)