-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
init: nixgl module #5355
init: nixgl module #5355
Conversation
This is great! Would it work for passing into |
@jonahbron yes! the wrapped package can be used interchangeably with the original one, so you can pass it to any option that's expecting a package. |
Just tried out the PR, and it seems to not work with packages like easyeffects that use a separate debug output. At the same time, the module works well with other programs like mpv. # this one doesnt work for me
services.easyeffects = {
enable = true;
package = (config.lib.nixGL.wrap pkgs.easyeffects);
}; When using the snippet above I get this error trace:
@Smona can this issue be reproduced on your end? |
@neiios thank you for testing it! That's an interesting test case since I can't imagine easyeffects benefiting from nixGL. But I could see this potentially affecting other packages with debug outputs. I'm able to reproduce your issue with the same configuration, but interestingly that configuration builds just fine for me if I use the same wrapper as |
Any updates on this? Will this be merged? |
I tried the wrapper with Prusa Slicer today but it did not make a difference, the 3D viewer didn't work either way (it does when I download and install the binary from the website). Happy to help debug of there's any info I can provide. Here's the output: $ prusa-slicer
Gtk-Message: 09:12:14.981: Failed to load module "canberra-gtk-module"
Gtk-Message: 09:12:14.983: Failed to load module "canberra-gtk-module"
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.117: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.118: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.120: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.121: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.123: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.123: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): GLib-WARNING **: 09:12:15.166: getpwuid_r(): failed due to unknown user id (750198)
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.309: gtk_widget_set_size_request: assertion 'width >= -1' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.309: gtk_widget_set_size_request: assertion 'width >= -1' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.318: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.318: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.319: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.319: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.320: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.320: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.320: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
(prusa-slicer:58991): Gtk-CRITICAL **: 09:12:15.321: gtk_cell_layout_get_cells: assertion 'GTK_IS_CELL_LAYOUT (cell_layout)' failed
09:12:17: Debug: window wxTreeCtrl@0x75bedb0 ("treeCtrl") lost focus even though it didn't have it
09:12:17: Debug: window wxTreeCtrl@0x75bedb0 ("treeCtrl") lost focus even though it didn't have it
09:12:17: Debug: window wxTreeCtrl@0x75bedb0 ("treeCtrl") lost focus even though it didn't have it
09:12:17: Debug: window wxTreeCtrl@0x75bedb0 ("treeCtrl") lost focus even though it didn't have it
09:12:17: Debug: window wxTreeCtrl@0x75bedb0 ("treeCtrl") lost focus even though it didn't have it
09:12:17: Debug: window wxTreeCtrl@0x75bedb0 ("treeCtrl") lost focus even though it didn't have it
09:12:17: Debug: window wxTreeCtrl@0x75bedb0 ("treeCtrl") lost focus even though it didn't have it
09:12:17: Debug: window wxTreeCtrl@0x75bedb0 ("treeCtrl") lost focus even though it didn't have it |
@jonahbron I don't see any libGL/GLX related errors in those logs, so my guess would be that those packaging issues are unrelated to this change? Especially given that Gtk-CRITICAL errors are considered known and ignored by the project. I would expect that if nixGL weren't working properly here, you would see glXChooseFBConfig failures instead. |
I finally found some time to look at this. I like the approach, it is elegant. It falls a bit short, though. I'll work on extending if, hopefully later today. But I wanted to write this comment immediately just in case something comes up (as it often does) preventing me from working on this for another week. The current implementation of wrapping falls short on packages that contain Using a single wrapper prefix can only provide OpenGL from a single vendor. This is insufficient because you surely also want Vulkan, and you may want some flexibility as to which vendor you use, e.g. for Prime offloading. This stuff gets hairy pretty fast. My own wrappers are more complete, and also quite straightforward to use, but the implementation is quite hairy. I'll try to refactor them and propose them for inclusion in this PR. My goal is to allow the user to fully use an Optimus laptop without having to know the internals of nixGL. And to keep things simple for people who don't have an Optimus laptop. And last, I'm not convinced wrapping some packages by default the way it's done now is a good idea. If we wrap every package that needs this, it would be great, but it's not feasible. If we wrap just a couple of common packages, I imagine users would be stumped as to why some packages don't work while others do. If, instead, we wrap nothing by default, and clearly document (e.g. in the User Manual) that anything needing graphics acceleration needs to be wrapped and how to do it, it would empower users. But, in the end, I don't have strong feelings about this. If someone wants to invest effort into wrapping lots of stuff by default, I wouldn't mind too much. Just note that the wrapper would need to be extended so as to detect and avoid double wrapping: I think it's a given that the project of default wrapping would take some time, and in the meantime, users would wrap stuff themselves. This would cause issues later when HM starts wrapping a package that wasn't wrapped before. Which begs the question: has anyone tried simply wrapping all the packages in their home configuration? Perhaps it doesn't cause any problems, and we could go with that? |
@exzombie thank you for the careful review! You brought up some very good points regarding wrapping packages by default, particularly the double-wrapping concern, so I've removed those changes from this PR for now. I'll review the rest of your proposed changes ASAP, since you've brought up a use case I didn't really consider. It looks like you primarily want to support customizing the opengl vendor on a per-package basis, but I also want to make sure config modules that use this wrapper can be easily re-used across systems with different graphics device setups for people like me who are reusing the same HM modules across multiple systems in a flake. We may want to just leave that up to user-defined settings which get passed into the wrapper along with the package, which I think your change would support. but I want to make sure I understand both use cases and how they interact first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See how I tested it: #3968 (comment)
I've applied part of @exzombie's suggestion, to rewrite .desktop files with absolute paths to the original derivation to point to the version wrapped with nixGL instead. Some of the other changes (the prime/offloading support) still have issues to work out, so we may want to hold off on those to get a basic version out sooner which supports the majority of use cases. I've also switched to Is there anything else needed to get this ready to merge? |
I'm not familiar with how this kind of thing is handled in HM; is it acceptable to merge an "early preview", when it is known that the user-facing options will have to change in an incompatible manner in the future? BTW, which issues have you found? I'm aware of the emacs problem you've had but that I couldn't reproduce to investigate. Was there something else? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have been using this, works great
Works great, except that when I have hyprlands |
@timoleistner you should just need to pass the wrapped |
I see. This works. Amazing, thank you for this, I hope it gets merged soon. |
What is currently blocking this PR at the moment, I don't think there are any pending changes requested ? Are we waiting on feedback from more people to try the PR before it gets merged ? |
My motivations for merging this as-is were 1) not wanting to delay merging of the basic functionality, and 2) a desire to be able to thoroughly test the contents of my PRs. At this point, it seems like keeping this PR simpler is not speeding up a merge, and may actually be blocking it. In the interest of uniting our efforts, I will gladly merge @exzombie's PR here once:
Even if there are problems with the multi-GPU side of that PR that I can't catch (I don't think there will be), multi-GPU support is the right goal to work towards, and any issues that might arise should be easily fixable post-merge, without changing the interface. EDIT: I just want to say I appreciate @exzombie for working against this PR, so that the discussion isn't fractured and my commits can remain in the history :) |
Oh, you're welcome. Truth be told, I would have kept your commits even if I were to open a new PR; it's important to give credit where credit's due. I wish there was a straightforward way to credit all those who have posted partial solutions to the various issues, there was a lot to learn from about this problem. Anyway, I'll take a look at your comment in the coming days, then rebase. I never had a non-flake HM setup, so I'll need to think about this. |
Okay, I've merged @exzombie's work into this PR! I think we're all aligned on the options interface now, and this change now includes support for dual-graphics systems. Anyone who's following this PR and wants to help get it merged, please test the integration again with the latest commits and leave your feedback here. Most of you will only need to set Thanks again @exzombie for making this integration work for more people! |
EDIT: My original question was basically: where do I set Solution: (only including relevant parts) # flake.nix
# include nixgl as an input and output
{
inputs = {
nixgl = {
url = "github:nix-community/nixGL";
inputs.nixpkgs.follows = "nixpkgs";
};
};
outputs = {nixpkgs, nixgl, etc...};
let
# I passed it down as an overlay as well, hence pkgs.nixgl in home.nix
# I have no idea why the passed down `nixgl` doesn't have the wrapper packages
pkgs = import nixpkgs {
inherit system;
overlays = [
nixgl.overlay
];
};
in {
} # home.nix
{ config, pkgs, nixgl, ... }:
{
imports = [
# TODO: remove when https://github.com/nix-community/home-manager/pull/5355 gets merged:
(builtins.fetchurl {
url = "https://raw.githubusercontent.com/Smona/home-manager/nixgl-compat/modules/misc/nixgl.nix";
sha256 = "1krclaga358k3swz2n5wbni1b2r7mcxdzr6d7im6b66w3sbpvnb3";
})
];
nixGL = {
packages = nixgl.packages; # you must set this or everything will be a noop
defaultWrapper = "mesa"; # choose from options
};
home.packages = [
pkgs.nixgl.nixGLIntel # change this to what you need, find options in nixgl repo, use `home-manager switch --impure` when using nvidia or build will fail
(config.lib.nixGL.wrap pkgs.alacritty)
];
} I've spent a day on getting Alacritty installed by home-manager running, I am a broken man, I just hope this comment helps someone. @Smona you're doing the lord's work |
awesome work @Smona @exzombie ! I also got alacritty working for the first time following @bevicted's steps misc questions:
|
If your distro is "recent", in the sense that its executables use libraries sufficiently similar to those in the version of nixpkgs you are using, you would probably be fine. On my Ubuntu 22.04 system, I never noticed any issues until I updated to nixpkgs 24.05. The issues arise when the wrapped program runs an executable provided by the distro, which I expect alacritty to do quite a lot. In that case, the library overrides needed for Vulkan can introduce symbol conflicts when libraries provided by nixpkgs are newer and have a different ABI than those in the distro. Something like the error shown below. For alacritty, a possible workaround would might be to unset
|
@jackmac92 AFAIK nixGL is not required on darwin. I'm able to run kitty for example on darwin with no As far as firefox, what does your configuration look like? I tested it (without vulkan), and for me it was reporting hardware-accelerated WebRender. That computer runs Ubuntu 24, so I have to keep a flake input with a relatively old version of nixpkgs pinned for that installation (compatible with their gnome version). That might be helping with library conflicts. |
@Smona Ahh ok good to know re: darwin, I just had some config shared between linux/darwin for mpv and was testing that build. But can separate those concerns in my config I'm using the home-manager setup for firefox, and tested But, I've been updating my flake often recently. Maybe it's my pinned version of nixpkgs that is the issue? Mind sharing the nixpkgs pin you're using later? Not sure if it is relevant, but below are my nixpkgs inputs, that I updated yesterday
|
This module enables wrapping programs which require access to libGL with nixGL on non-NixOS systems.
Some desktop files will refer to the absolute path of the original derivation, which would bypass nixGL wrapping. So we need to replace the path with the path to the wrapper derivation to ensure the wrapped version is always launched.
makeWrapper is more consistent with the rest of nixpkgs & home-manager, so it should be a little more maintainable. It can also validate that the wrapper command is executable at build time.
Co-authored-by: V. <[email protected]>
Co-authored-by: V. <[email protected]>
Co-authored-by: V. <[email protected]>
Thanks, great stuff! Merged to master now 🙂 |
@Smona I get an errors on my (flakes-based) setup when trying to use the module:
I'm not sure why though, I do have |
It looks to me like that's the problem. In a flake, you don't import nixpkgs, you get it as an argument to the outputs. Take a look at the HM flake template. |
Unfortunately, this messes with my ability to overlay a custom In general, I don't think we should be relying on how the |
I won't argue with that. Let me just point out that in the case of NixGL, there is a difference in package structure when you import NixGL, vs. when you get it as a flake input. So, you don't necessarily end up with the same thing. Don't ask me why they've done it that way. That said, I don't think this should be a problem with nixpkgs. I mean, when I run |
Yeah I also tried it in the REPL and didn't reproduce the lack of |
I've just added a tracing log to my configuration, |
@ambroisie can you open a separate issue for that? I think replying on this thread pings quite a few people. |
Description
This PR integrates
nixGL
into home manager in a way that does not introduce anixGL
dependency, and attempts to make user configuration as simple as possible. This accomplishes the same goal as #5332, namely making it simpler for users to run applications which requirelibGL
on non-nixos systems managed by home manager. There's some elaboration on why I think this is a better API in the original issue.It makes use of a wrapper which I have been using for some months to run a few GPU-bound applications on Ubuntu. Judging by post reactions it seems that many other users have also found success with this wrapper. By default it is a a no-op, but if a user has configured a
nixGL.prefix
, then it will symlink all files from the original package into a new one, except for binaries which are replaced by a wrapper script calling them with the configured prefix. Unlike other wrappers proposed in #3968, all derivation attributes from the original package are preserved, so the result of the wrapper can be passed along to program modules for additional overriding. And since the program binary itself is replaced, the program will be launched in the nixGL context regardless of where it is called from (terminal, desktop file, etc).The wrapper function is made available on
lib.nixGL.wrap
, so users can wrap applications which require libGL like so:I'm very open to changing option/wrapper paths if it would help get this merged. Just having the wrapper and the option would be enough for users to wrap packages with nixGL ergonomically.
Wrapping packages by default
The default no-op behavior and derivation attribute preservation of this wrapper means that HM modules for applications could wrap their default package with nixGL. This would have the benefit of tracking which applications need
nixGL
to function properly on non-nixos systems, and saving end users time by makingnixGL.prefix
the only option they need to set to get these applications working. If a user wanted to undo the default wrapping for certain applications, they could simply overrideprograms.<application>.package
to a non-wrapped package. This PR originally included examples forkitty
andfirefox
, which both benefit fromnixGL
on non-nixos systems. However, @exzombie brought up some valid concerns about doing this, particularly detecting and preventing double-wrapping, so in the interest of getting the wrapper itself merged this default wrapping has been removed, to be left for future improvement.Checklist
Change is backwards compatible.
Code formatted with
./format
.Code tested through
nix-shell --pure tests -A run.all
ornix develop --ignore-environment .#all
using Flakes.Test cases updated/added. See example.
TODO
Commit messages are formatted like
See CONTRIBUTING for more information and recent commit messages for examples.
If this PR adds a new module
Maintainer CC
@kira-bruneau @rycee
Alternative to #5332
Closes #3968