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.

Sign up for GitHub

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

Jump to bottom

google-chrome: add /run/opengl-driver/share/vulkan/icd.d/ to path #155452

Merged
merged 1 commit into from
Jan 22, 2022
Merged

google-chrome: add /run/opengl-driver/share/vulkan/icd.d/ to path #155452

merged 1 commit into from
Jan 22, 2022

Conversation

brandonweeks
Copy link
Contributor

Motivation for this change

NixOS stores ICDs at /run/opengl-driver/share/vulkan/icd.d/, while Chrome is searching these paths:

DRIVER: Searching for driver manifest files
DRIVER:    In following folders:
DRIVER:       /home/bweeks/.config/vulkan/icd.d
DRIVER:       /etc/xdg/vulkan/icd.d
DRIVER:       /home/bweeks/.nix-profile/etc/xdg/vulkan/icd.d
DRIVER:       /etc/profiles/per-user/bweeks/etc/xdg/vulkan/icd.d
DRIVER:       /nix/var/nix/profiles/default/etc/xdg/vulkan/icd.d
DRIVER:       /run/current-system/sw/etc/xdg/vulkan/icd.d
DRIVER:       /etc/vulkan/icd.d
DRIVER:       /home/bweeks/.local/share/vulkan/icd.d
DRIVER:       /nix/store/z4p55yy4i15l5cppid37yih6gvrbczvy-cups-2.3.3op2/share/vulkan/icd.d
DRIVER:       /nix/store/ydckdz6qc010fclx3hrksa62yjphrl19-gtk+3-3.24.30/share/vulkan/icd.d
DRIVER:       /nix/store/zszg84zmq95fmka2xmb29j80p8y8671j-adwaita-icon-theme-41.0/share/vulkan/icd.d
DRIVER:       /nix/store/6pwg357yyh43aklmxnh2zlwi21dbwfv2-hicolor-icon-theme-0.17/share/vulkan/icd.d
DRIVER:       /nix/store/0rhbw783qcjxv3cqln1760i1lmz2yb67-gsettings-desktop-schemas-41.0/share/gsettings-schemas/gsettings-desktop-schemas-41.0/vulkan/icd.d
DRIVER:       /nix/store/ydckdz6qc010fclx3hrksa62yjphrl19-gtk+3-3.24.30/share/gsettings-schemas/gtk+3-3.24.30/vulkan/icd.d
DRIVER:       /nix/store/hnnlhlsn0qbf166b88kr5kynq4dxal13-patchelf-0.13/share/vulkan/icd.d
DRIVER:       /nix/store/jbi6kbiilbg7mxy2y8ydklbgz7r8iyqj-desktops/share/vulkan/icd.d
DRIVER:       /home/bweeks/.nix-profile/share/vulkan/icd.d
DRIVER:       /etc/profiles/per-user/bweeks/share/vulkan/icd.d
DRIVER:       /nix/var/nix/profiles/default/share/vulkan/icd.d
DRIVER:       /run/current-system/sw/share/vulkan/icd.d

Tested by running VK_LOADER_DEBUG=all google-chrome-unstable --enable-features=Vulkan and navigating to chrome://gpu.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from primeos January 18, 2022 08:33
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jan 18, 2022
@primeos primeos self-assigned this Jan 19, 2022
@primeos
Copy link
Member

primeos commented Jan 19, 2022

So I assume Google Chrome doesn't use the wrapped Vulkan loader from NixOS then?

Just do be sure: Does vulkaninfo from the vulkan-tools package run fine on your system (without overriding any environment variables)?

Adding this workaround for Google Chrome is not ideal (e.g. that path might change at some point: #141803) but I guess we can merge it if necessary. The commit message needs to be improved (adding the relevant information from your PR description to the body) before merging but I can also handle that via GH's squash and merge (out of laziness).

Looking at the driver discovery documentation for Linux it seems like $XDG_DATA_DIRS is indeed the most appropriate option: https://github.com/KhronosGroup/Vulkan-Loader/blob/60d17fe6c65befa0a738b6c33da2eb0670055f3b/docs/LoaderDriverInterface.md#driver-discovery-on-linux

The NixOS documentation (just for reference): https://nixos.org/manual/nixos/unstable/index.html#sec-gpu-accel-vulkan

Anyway, thanks for looking into it!

@brandonweeks
google-chrome: add /run/opengl-driver/share/vulkan/icd.d/ to path
007af34 
NixOS stores ICDs at /run/opengl-driver/share/vulkan/icd.d/. Because
Chrome ships its own vulkan-loader and doesn't use the NixOS system
vulkan-loader, Chrome won't search the /run/opengl-driver directory
withou either adding it to the path or patching Chrome's libvulkan.so.1.

This change adds "${addOpenGLRunpath.driverLink}/share" unconditionally
to the path. addOpenGLRunpath is the same module that NixOS system
vulkan-loader uses as the path.

Tested by running `VK_LOADER_DEBUG=all google-chrome-unstable
--enable-features=Vulkan` and verifying Vulkan is enabled with
chrome://gpu.
@brandonweeks
Copy link
Contributor Author

So I assume Google Chrome doesn't use the wrapped Vulkan loader from NixOS then?

That appears to be the case. As far as I can tell vulkan-loader is part of libvulkan.so.1 that is shipped with Chrome. Also, removing the vulkan-loader bits has no effect.

Just do be sure: Does vulkaninfo from the vulkan-tools package run fine on your system (without overriding any environment variables)?

Yup, vulkaninfo looks good on my machine.

Adding this workaround for Google Chrome is not ideal (e.g. that path might change at some point: #141803) but I guess we can merge it if necessary. The commit message needs to be improved (adding the relevant information from your PR description to the body) before merging but I can also handle that via GH's squash and merge (out of laziness).

Instead of hard coding the path, I now use the path defined by addOpenGLRunpath, which is the same path the system vulkan-loader package uses.

Looking at the driver discovery documentation for Linux it seems like $XDG_DATA_DIRS is indeed the most appropriate option: https://github.com/KhronosGroup/Vulkan-Loader/blob/60d17fe6c65befa0a738b6c33da2eb0670055f3b/docs/LoaderDriverInterface.md#driver-discovery-on-linux

The NixOS documentation (just for reference): https://nixos.org/manual/nixos/unstable/index.html#sec-gpu-accel-vulkan

Anyway, thanks for looking into it!

primeos
primeos approved these changes Jan 22, 2022
View reviewed changes
Copy link
Member

@primeos primeos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM then. Thanks a lot!

@primeos primeos merged commit f5a315a into NixOS:master Jan 22, 2022
@brandonweeks brandonweeks deleted the chrome-vulkan branch January 22, 2022 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@primeos primeos primeos approved these changes

Assignees

@primeos primeos

Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@brandonweeks @primeos