Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use system SPIRV and Vulkan headers if new enough #3322

Closed
smcv opened this issue Mar 31, 2023 · 12 comments · Fixed by #3836
Closed

Use system SPIRV and Vulkan headers if new enough #3322

smcv opened this issue Mar 31, 2023 · 12 comments · Fixed by #3836

Comments

@smcv
Copy link
Contributor

smcv commented Mar 31, 2023

When packaging a library like DXVK Native, Linux distributions generally prefer to use system dependencies rather than vendoring a copy into dependent projects, so that if there's a bug in the dependency, it can be fixed in one place. This would also make it easier to build from the source tarballs provided by Github, which don't include git submodules, or from a simple git clone without submodules.

Would it be possible to use something like this (slightly pseudocode, I haven't tested this):

vulkan_dep = dependency('vulkan', '>=1.3.239', required: false)

if vulkan_dep.found()
  vulkan_headers = vulkan_dep.partial_dependency(includes: true)
else
  vulkan_headers = declare_dependency(include_directories: ['./include/vulkan/include'])
endif

spirv_dep = dependency('SPIRV-Headers', '>=1.5.5', required: false)

if not spirv_dep.found()
  spirv_dep = declare_dependency(include_directories: ['./include/spirv/include'])
endif

and then use vulkan_headers and spirv_dep in the dependencies of each target, instead of adding these two directories to dxvk_include_dirs?

If that seems like an OK approach, I'll prepare a merge request.

(Unfortunately, checking for SPIRV-Headers newer than 1.5.5 doesn't currently work, as a result of KhronosGroup/SPIRV-Headers#334.)

@K0bin
Copy link
Collaborator

K0bin commented Mar 31, 2023

Why would Linux distros ship DXVK Native?

@smcv
Copy link
Contributor Author

smcv commented Mar 31, 2023

To compile games (presumably games that were originally for Windows and subsequently ported to Linux) against it? Fedora already has packages, although they're currently out of date.

Even if most mainstream Linux distros don't actually do this, the Steam Runtime behaves a lot like a Linux distro, and we want to be using a single consistent set of Vulkan and SPIRV headers across everything that's included in that. The Steam Runtime's SDK already includes both, at up-to-date versions.

@doitsujin
Copy link
Owner

DXVK currently is not really designed to be shipped by distros. Releases are not necessarily binary-compatible, and we don't want to change sonames all the time either.

Like, yeah, we can address things like the glfw/sdl stuff, but if you absolutely need to package it for whatever reason, then please let us do the necessary reworks first in order to not turn this into an absolute maintenance nightmare for us that's just going to lead to complaints from your side again when things break.

@misyltoad
Copy link
Collaborator

misyltoad commented Mar 31, 2023

fwiw, I am kinda committed to binary compatibility for DXVK Native once we start shipping it in the Steam Runtime

Before we actually get it shipped in the runtime, I would say we can change stuff, I would say.

Like, yeah, we can address things like the glfw/sdl stuff, but if you absolutely need to package it for whatever reason, then please let us do the necessary reworks first in order to not turn this into an absolute maintenance nightmare for us that's just going to lead to complaints from your side again when things break.

I brought up packaging this in the runtime, mainly so I don't have to update like 5 games that use it and someone else is also wanting to use it for a game on Steam.

What reworks did you have in mind @doitsujin?

I think if we have some time to improve this situation, I think I would maybe make the HWND situation nicer, so you pass in a

struct WSIWindow
{
    WSIWindowType type = WSI_WINDOW_TYPE_SDL;
    union
    {
        struct
        {
            SDL_Window *window;
        } sdl2;

        struct
        {
            GLFWwindow *window;
        } glfw;

        struct
        {
            // reserve some whatever amount if we want
            // to add X11/Wayland WSI which would need multiple
            // ptrs in the future.
            uint64_t padding[16];
        } padding;
    }
};

or something.

That aside...

I don't see the point of using system headers, Vulkan ABI is stable, the headers are completely autogenerated from spec xml, using whatever system headers makes no difference. There is nothing to fix ever by definition.

@doitsujin
Copy link
Owner

What reworks did you have in mind @doitsujin?

Mostly just the runtime selection thing and any build system jank that may need sorting out.

I think if we have some time to improve this situation, I think I would maybe make the HWND situation nicer, so you pass in a

What would we typedef HWND to in that case? A WSIWindow*?

I don't see the point of using system headers, Vulkan ABI is stable, using whatever system headers makes no difference. There is nothing to fix ever by definition.

Kind of agree, I don't really see why we need this.

@misyltoad
Copy link
Collaborator

What would we typedef HWND to in that case? A WSIWindow*?

Yeah. I also want to make it possible to switch WSIs at runtime potentially.

@smcv
Copy link
Contributor Author

smcv commented Mar 31, 2023

Even with Vulkan's API/ABI itself being long-term stable, it's entirely possible that its headers will need changes to be able to compile with future compilers, or work correctly on future architectures.

@smcv
Copy link
Contributor Author

smcv commented Mar 31, 2023

DXVK currently is not really designed to be shipped by distros. Releases are not necessarily binary-compatible, and we don't want to change sonames all the time either.

If that's currently the case, then it's premature to include it in Debian and the Steam Runtime for now (and probably also Fedora, but that's not my decision).

However, if it's ever going to be packaged in environments like the Steam Runtime SDK, it'll be good to attempt packaging (but without actually pushing it to distros) before it's declared stable, so that if making it more sensibly packageable requires incompatible changes (like #3324 or #3322 (comment)), it isn't too late for those changes to happen.

@flibitijibibo
Copy link
Contributor

To add to the WSIWindow idea, here's SDL_syswm which does something pretty similar and has historically done a pretty good job re: keeping the ABI stable while also allowing for changes which the runtime can detect:

https://github.com/libsdl-org/SDL/blob/main/include/SDL3/SDL_syswm.h

(Basically it's what Joshua had in mind, but with a version number to make it easier to add to existing structs while knowing what struct members are valid)

Regarding applications, FNA is hoping to incorporate D3D11 Linux support via dxvk and vkd3d-shader; you can see our usage of dxvk here (grep FNA3D_DXVK_NATIVE): https://github.com/FNA-XNA/FNA3D/blob/master/src/FNA3D_Driver_D3D11.c

@frantisekz
Copy link
Contributor

To compile games (presumably games that were originally for Windows and subsequently ported to Linux) against it? Fedora already has packages, although they're currently out of date.

Yeah, we have both wine-dxvk (installed by default together with wine package, maintained by me) and dxvk-native (the original pre-merge release of dxvk-native). There is a plan to upgrade both packages, wine-dxvk bump to 2.1 at least will be a 0day update for Fedora 38. dxvk-native bump probably pending work merging the two packages into the one source package: https://bugzilla.redhat.com/show_bug.cgi?id=2152207

Being able to use system SPIRV/Vulkan headers would be beneficial even for the wine version of the dxvk (it's distribution guideline), currently, To adhere to that guideline, I am currently doing something in the lines of:

# Copy out system include directories for spirv, vulkan and DirectX
mkdir -p ./include/vulkan/include && cp -R /usr/include/vulkan ./include/vulkan/include && cp -R /usr/include/vk_video ./include/vulkan/include
mkdir -p ./include/spirv/include && cp -R /usr/include/spirv ./include/spirv/include

mkdir -p ./include/native/directx && cp %{mingw_sysroot}/mingw/include/d3d*.h ./include/native/directx
cp %{mingw_sysroot}/mingw/include/dx*.h ./include/native/directx

Being able to use the system version would be nice from the packager perspective, but it's not the world's end :)

@pchome
Copy link
Contributor

pchome commented May 31, 2023

@K0bin

Why would Linux distros ship DXVK Native?

https://wiki.gentoo.org/wiki/Project:Prefix

tldr

To bring out the virtues of Gentoo Linux on different operating systems, the Gentoo Prefix project develops and maintains a way of installing Gentoo systems in a non-standard location, designated by a "prefix".

Usually, Gentoo Linux's package manager (Portage) installs in the root of the filesystem hierarchy known as /. On systems other than Gentoo Linux, this usually results in problems, due to conflicts of software packages, unless the OS is adapted like Gentoo FreeBSD. Instead, Gentoo Prefix installs within an offset, known as a prefix, allowing users to install Gentoo in another location in the filesystem hierarchy, hence avoiding conflicts. Next to this offset, Gentoo Prefix runs unprivileged, meaning no root user or rights are required to use it.

By using an offset (the "prefix" location), it is possible for many "alternative" user groups to benefit from a large part of the packages in the Gentoo ebuild repository. Currently users of the following systems successfully run Gentoo Prefix: macOS on PowerPC and Intel, Linux on x86, x86_64 and arm, Solaris 11 on Sparc, Sparc/64, x86 and x86_64. Other platforms have been successfully used in the past.

For example.
Package manager in userspace, which will handle all required dependencies. Not sure how practical it is on other distros though.
Can be useful for someone who regularly compile DXVK from git, but I'm not sure about multilib support in same prefix.

@pchome
Copy link
Contributor

pchome commented Jul 30, 2023

For simplicity lets assume both vulkan and spirv headers came from the same SDK version, and sit in the same includedir as /{vulkan,spirv}/, then checking only vulkan version should be enough.

meson.build test

project('test-headers')

dxvk_include_dirs = [
  './include',
]

vulkan_dep = dependency('vulkan', version: '>=1.3.239', required: false)
spirv_dep = dependency('SPIRV-Headers', version: '>=1.5.5', required: false)

if vulkan_dep.found() and spirv_dep.found()
  dxvk_include_dirs += [ vulkan_dep.get_pkgconfig_variable('includedir') ]
else
  dxvk_include_dirs += ['./include/vulkan/include']
  dxvk_include_dirs += ['./include/spirv/include']
endif

warning(dxvk_include_dirs)
$ meson setup build
...
Run-time dependency vulkan found: YES 1.3.250
Run-time dependency spirv-headers found: YES 1.5.5
meson.build:29: WARNING: ['./include', '/usr/include']
# bumped vulkan requirement
$ meson setup build
...
Dependency vulkan found: NO found 1.3.250 but need: '>=1.3.269'
Run-time dependency vulkan found: NO (tried pkgconfig)
Run-time dependency spirv-headers found: YES 1.5.5
meson.build:29: WARNING: ['./include', './include/vulkan/include', './include/spirv/include']

Additionally could be hidden behind meson config option (system_headers (?)).

UPD:

v2-example

project('test-headers-v2')

# Current vulkan requirement in git is higher than 1.3.239, often can be higher than system vulkan
vulkan_dep = dependency('vulkan', version: '>=1.3.254', required: false)
spirv_dep = dependency('SPIRV-Headers', version: '>=1.5.5', required: false)

# Common variant, do not use system include directory for cross-compilation
if vulkan_dep.found() and spirv_dep.found()
  sys_inc = vulkan_dep.get_pkgconfig_variable('includedir')
  cmd_kw = { 'capture': false, 'check': true }

  run_command('mkdir', '-p', 'include/vulkan/include', kwargs: cmd_kw)
  run_command('mkdir', '-p', 'include/spirv/include',  kwargs: cmd_kw)

# There is also vk_video
  run_command('ln', '-sf', sys_inc / 'vk_video', 'include/vulkan/include/', kwargs: cmd_kw)
  run_command('ln', '-sf', sys_inc / 'vulkan', 'include/vulkan/include/', kwargs: cmd_kw)
  run_command('ln', '-sf', sys_inc / 'spirv', 'include/spirv/include/', kwargs: cmd_kw)
endif

But this is what packagers already do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants