ChristophHaag / SteamVR-OpenHMD

SteamVR plugin for using OpenHMD drivers in SteamVR
Boost Software License 1.0
195 stars 32 forks source link

use system openhmd instead of submodule #82

Open sl1pkn07 opened 1 year ago

sl1pkn07 commented 1 year ago

Hi

is possible use and link with system openhmd instead of pull and build again from submodule?

greetings

thaytan commented 1 year ago

If you're using the meson build, you can comment out this line and it will use an external OpenHMD:

openhmd_subproject = subproject('openhmd', default_options: ['default_library=static'])

Make it:

# openhmd_subproject = subproject('openhmd', default_options: ['default_library=static'])
eli-schwartz commented 1 year ago

I fail to see how that would conceptually work -- the very next line references openhmd_subproject.get_variable().

This is trivial to solve, but it would require following the documentation printed out twice as sandbox violation warnings in the build.

thaytan commented 1 year ago

Sorry - I was looking at my fork and forgot that I didn't merge some changes back here.

https://github.com/thaytan/SteamVR-OpenHMD/commit/087aa4c94ad5e47994e41b62f947f35a48fac423 changes the OpenHMD subproject setup

eli-schwartz commented 1 year ago

That's a good start, but please delete the line:

openhmd_subproject = subproject('openhmd', default_options: ['default_library=static'])

and move default_options: to the dependency().

Using a call to subproject() to force using the subproject by default is a wrong approach, please set project(default_options: ['wrap_mode=forcefallback']) instead (or really, just let Meson autodetect the system version).

thaytan commented 1 year ago

Thanks, that sounds cleaner. I guess I missed that I could pass default_options to the dependency() call at the time

ChristophHaag commented 1 year ago

This was a quick hack I put in a long, long time ago. You can also look at how how sway handles the wlroots subproject for a more decent example

eli-schwartz commented 1 year ago

Right, I discussed the swaywm handling with them too. :D

In their case, the two are developed together and also different wayland projects apparently require different ABI editions of wlroots :scream: and it's not the ecosystem custom :scream: to maintain support for multiple versions via #ifdef so they wanted to compel people to use the bundled version by default.

sl1pkn07 commented 1 year ago

something like

project(
    'steamvr-openhmd', 'cpp',
    default_options : 'cpp_std=c++11',
    version : '0.0.1',
    meson_version:  '>=0.54'
)

sources = [
    'driverlog.cpp',
    'driverlog.h',
    'driver_openhmd.cpp'
]

host_system = host_machine.system()
if host_system == 'windows'
        dep_openhmd = dependency('openhmd', static: true)
else
        dep_openhmd = dependency('openhmd')
endif

deps = [
        dependency('openvr').partial_dependency(includes: true, compile_args: true),
        dep_openhmd,
    dependency('threads')
]

steamvr_openhmd_lib = library(
    'driver_openhmd', sources,
    dependencies : deps,
    install : true,
    version : meson.project_version(),
    name_prefix : ''
)

?

use also system openvr (only use the headers and not link with library). and use openhmd as shared in linux, and static in windows

ChristophHaag commented 1 year ago

do you want to make a pull request?

sl1pkn07 commented 1 year ago

then is ok?

thaytan commented 1 year ago

Does the dependency('openvr').partial_dependency(includes: true, compile_args: true), part actually work? The OpenVR directory in the subprojects isn't really set up as a wrap, it's just 1 header file

eli-schwartz commented 1 year ago

Well, it finds the system version which works... you'd need to set the subproject up as a wrap obviously, in order to do fallback.

thaytan commented 1 year ago

That's what I thought. There's no such thing as a system version of the openvr header - exactly the bundled version needs to be pulled in. It should probably just be moved out of the subprojects directory so meson stops being shouty about it

eli-schwartz commented 1 year ago

That's what I thought. There's no such thing as a system version of the openvr header

What an interesting claim to make. I have one, clearly there is such thing as it.

exactly the bundled version needs to be pulled in.

Requiring an exact version is a different story, of course. But you can do that with Meson too, see the version: kwarg to dependency() (and fall back on the correct version via a subproject wrap). I guess that would matter more if you weren't forced to check it into git because the openvr source code is 147mb...

It should probably just be moved out of the subprojects directory so meson stops being shouty about it

But it is literally another project developed and shipped separately, or at least the subset of that project's release snapshots that you actually make use of. Why shouldn't it semantically be a subproject?

thaytan commented 1 year ago

That's what I thought. There's no such thing as a system version of the openvr header

What an interesting claim to make. I have one, clearly there is such thing as it.

Interesting. Arch must be the only distro that ships one.

exactly the bundled version needs to be pulled in.

Requiring an exact version is a different story, of course. But you can do that with Meson too, see the version: kwarg to dependency() (and fall back on the correct version via a subproject wrap). I guess that would matter more if you weren't forced to check it into git because the openvr source code is 147mb...

Yes, it's a big waste of space and downloads when we need exactly one header, and exactly the version of it that the driver is currently targetted at.

It should probably just be moved out of the subprojects directory so meson stops being shouty about it

But it is literally another project developed and shipped separately, or at least the subset of that project's release snapshots that you actually make use of. Why shouldn't it semantically be a be a subproject?

As above.

eli-schwartz commented 1 year ago

Interesting. Arch must be the only distro that ships one.

Debian ships one too, as does OpenSUSE.

As above.

Making a single-file subproject wrap for a single-file check-your-dependencies-into-git seems perfectly reasonable and not at all related to the space and downloads.

project('openvr-driver-header', version: 'X.Y')

openvr_dep = declare_dependency(include_directories: '.')

This ensures that vendored code is clearly delineated, does not get lost inside the project sources, and has a clear provenance for e.g. the version. It also means that you can isolate upstream changes into an interface export, and handle syncing that separately from your own project sources in the event that upstream does internal refactoring.

thaytan commented 1 year ago

Interesting. Arch must be the only distro that ships one.

Debian ships one too, as does OpenSUSE.

Interesting - I obviously didn't realise distros were doing that. How does it work? The apps compile against OpenVR but can't use it until the SteamVR runtime is installed?

As above.

Making a single-file subproject wrap for a single-file check-your-dependencies-into-git seems perfectly reasonable and not at all related to the space and downloads.

project('openvr-driver-header', version: 'X.Y')

openvr_dep = declare_dependency(include_directories: '.')

This ensures that vendored code is clearly delineated, does not get lost inside the project sources, and has a clear provenance for e.g. the version. It also means that you can isolate upstream changes into an interface export, and handle syncing that separately from your own project sources in the event that upstream does internal refactoring.

OK, that makes sense.