Skip to content

mangohud: add bitness suffix to layer name#231874

Merged
Atemu merged 2 commits intoNixOS:masterfrom
Atemu:fix/mangohud-layer-name-bitness
May 15, 2023
Merged

mangohud: add bitness suffix to layer name#231874
Atemu merged 2 commits intoNixOS:masterfrom
Atemu:fix/mangohud-layer-name-bitness

Conversation

@Atemu
Copy link
Member

@Atemu Atemu commented May 14, 2023

Description of changes

The VK loader finds the 32-bit layer first and does not attempt to load the
64-bit layer afterwards; likely because it shares the same name. Simply giving
them different names fixes the issue; both layers are tried and the correct one
succeeds.

A similar patter is employed by obs-vkcapture which continued working after
#228870.

Fixes #230978

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/)
  • 23.05 Release Notes (or backporting 22.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
  • Fits CONTRIBUTING.md.

The VK loader finds the 32-bit layer first and does not attempt to load the
64-bit layer afterwards; likely because it shares the same name. Simply giving
them different names fixes the issue; both layers are tried and the correct one
succeeds.

A similar patter is employed by obs-vkcapture which continued working after
NixOS#228870.

Fixes NixOS#230978
Comment on lines +208 to +209
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have the means to test but if MangoApp needs this too, the file should be modified here aswell when support is enabled.

cc @Scrumplex

Copy link
Contributor

@kira-bruneau kira-bruneau May 14, 2023

Choose a reason for hiding this comment

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

Yep, this should be done for MangoApp too!

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh wait sorry nvm!

Copy link
Contributor

@kira-bruneau kira-bruneau May 14, 2023

Choose a reason for hiding this comment

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

Right now our current build isn't even supporting mixed 32bit & 64bit mangoapp (and I don't think upstream does either). Since it runs in a separate process, there's no need to have a 32bit build for overlaying 32bit games (on a 64bit system).

@K900
Copy link
Contributor

K900 commented May 14, 2023

Maybe we can get this upstreamed? Shouldn't be hard to add the arch suffix directly to the template.

This allows the user to disable 32-bit support for closure size reasons or in
order to mitigate loader issues like
NixOS#230978.

The name is weird because it can't start with a digit :/
@Atemu Atemu force-pushed the fix/mangohud-layer-name-bitness branch from c3483ff to 7eacc7f Compare May 14, 2023 16:36
@Atemu
Copy link
Member Author

Atemu commented May 14, 2023

I'll create an upstream issue.

cc @THERAAB @IceDBorn for testing.

@ofborg ofborg bot added the 8.has: clean-up This PR removes packages or removes other cruft label May 14, 2023
@ofborg ofborg bot requested a review from zeratax May 14, 2023 17:05
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 14, 2023
Copy link
Contributor

@kira-bruneau kira-bruneau left a comment

Choose a reason for hiding this comment

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

Still want to do a bit more testing (having some other problems with my system right now that's blocking me), but the changes look good to me!

Thanks for taking this on @Atemu & @K900 ❤️

@THERAAB
Copy link

THERAAB commented May 14, 2023

Hi @Atemu @kira-bruneau I'm happy to test this, but unsure of how I can do so? Should I replace nixpkgs reference in my flake.nix with this branch Atemu:fix/mangohud-layer-name-bitness?

@IceDBorn
Copy link
Contributor

It works like a charm! Thank you @Atemu

@Atemu
Copy link
Member Author

Atemu commented May 15, 2023

@THERAAB I don't use flakes but I'd expect that to work. Overriding the NIX_PATH would be the non-flakes way of doing it. My branch is based on last week's unstable.

@Atemu Atemu merged commit 746a5fc into NixOS:master May 15, 2023
@Atemu Atemu deleted the fix/mangohud-layer-name-bitness branch May 15, 2023 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: clean-up This PR removes packages or removes other cruft 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mangohud no longer appears when loading games

5 participants