Page MenuHomeSolus

Inclusion of MangoHud
ClosedPublic

Authored by Trillon008 on Feb 25 2020, 10:19 AM.

Details

Reviewers
JoshStrobl
Group Reviewers
Triage Team
Maniphest Tasks
T8733: MangoHud
Commits
R5038:93e2532b4701: Inclusion of MangoHud
Summary

Resolve T8733. This should provide a working version of mango vulkan overlay (A MSI Afterburner like) for solus users.

Test Plan

Launch a game with steam proton (RE:2 remake here) to trigger DXVK and vulkan rendering. The game is launched with mangohud %command% in steam.
Launch another game Door Kickers action squad to trigger 32bits mangohud
Launch glxgears to test for opengl
The overlay works and shows gpu and cpu loads + FPS and frametime for the 3 tests. This is tested with amd mesa drivers, I can't test for Nvidia

Diff Detail

Repository
R5038 mangohud
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Trillon008 created this revision.Feb 25 2020, 10:19 AM
Trillon008 requested review of this revision.Feb 25 2020, 10:19 AM

Inside your summary you should point to the task with
resolves Txxxx

package.yml
6

you should use the source.tar file

17

alphabetical order of the build deps

pspec_x86_64.xml
7

you should set your mail as well inside the packager file

Girtablulu requested changes to this revision.Feb 25 2020, 2:30 PM
This revision now requires changes to proceed.Feb 25 2020, 2:30 PM
Trillon008 added a comment.EditedFeb 25 2020, 5:44 PM

@Girtablulu I see your comments and this will be corrected soon but I can't use the source tar here because the building process involve git submodule to fetch a specific git commit of imGui

Also it was mentioned here: https://dev.getsol.us/T8733 we could use some config-enable but I don't know how to handle this.

Trillon008 updated this revision to Diff 20001.Feb 26 2020, 8:38 AM

Fix things reported by coreteam. Also remove libglvnd as dependency after speaking with mango dev team as this is not useful

ah okay then it's fine but you still need to mention the task in your summary so it will be automatically closed when your patch gets accepted

package.yml
2

Over saw this one, needs to be all small letters

Girtablulu added a comment.EditedFeb 27 2020, 9:26 PM

Additional question, doesn't need the 32bit library a mangohud.json as well?

€: I looked at Ikeys comment in the Task and he asks for pattern the json file into a separate package and work with it over this, so it can be used for the 64bit and 32bit package

Trillon008 updated this revision to Diff 20094.Mar 3 2020, 11:27 AM
Trillon008 edited the summary of this revision. (Show Details)

Some update on this:
I added a configuration file for mangohud in /usr/share. While this is totally not mandatory for manghud to work or configure (env vars are ok) I find it nice to have it somewhere.
Also the name is now lowercase

I started to check for mangohud32/64.json files and I have a package.yml version that does it. However, it relies on some bash posted below.
I believe this is far from what Ikey proposed with a different package for the json file. But I really have no clue how to handle this.
Side note, I use mangohud everyday with an incorrect json file and it works well. I wonder where it's used.

MANGOJSONDIR=$installdir/usr/share/vulkan/implicit_layer.d
sed -i -e 's|libMangoHud.so|%libdir%/&|g' $MANGOJSONDIR/mangohud.json
if [ $EMUL32BUILD ]; then
   mv $MANGOJSONDIR/mangohud.json $MANGOJSONDIR/mangohud32.json 
else
   mv $MANGOJSONDIR/mangohud.json $MANGOJSONDIR/mangohud64.json 
fi
Trillon008 edited the summary of this revision. (Show Details)Mar 3 2020, 1:52 PM
Trillon008 edited the summary of this revision. (Show Details)

I may be wrong but I believe the suffix convention for Vulkan ICDs is .i686 and .x86_64

Additionally I'd glob the 32-bit ICD file into the 32bit subpackage..

Ok I checked icd.d related vulkan directory and you're correct for i686 and x86_64.
I 'll also checkout tomorrow if I can pack the i686 jsonfile with the 32 bit subpackage.

Trillon008 updated this revision to Diff 20149.Mar 5 2020, 10:48 AM

Update file path and lib path

So um... mangohud.json is installed everytime ninja_install passes. I didn't find a way via package tools to force the file to be associated with the 32bits libs. :/

Girtablulu added inline comments.Mar 5 2020, 2:56 PM
package.yml
14

should be patterns after this you can pattern the mangohud.i686.json into the 32bit

15

not needed

16

this one as well

31

not really needed

34

use install -Dm00644 $workdir/bin/MangoHud.conf -t $installdir/usr/share/mangohud

pspec_x86_64.xml
39

the 32bit package doesn't need any conf file?

Girtablulu requested changes to this revision.Mar 5 2020, 2:56 PM
This revision now requires changes to proceed.Mar 5 2020, 2:56 PM
Trillon008 updated this revision to Diff 20157.Mar 6 2020, 2:35 PM

Integration with patterns ! And now it makes much more sense.
I didn't add a conf file for 32bit as there is no need of having one. Even for 64 bits. It's just here for people to have a template if the feel the need to use it. Ofc, I can modify this if you're not ok with this.

Update mangohud to 0.3.1. This include new feature notably shell scripts for 32 and 64 bits. I rename 32 bits binary to .i686 but not sure how this fit in the ecosystem

Many updates since first review. I've playing with mangohud package on my PC for some time now and tested it on many games. Still not convinced about the mangohud.i686 in /usr/bin

DataDrake requested changes to this revision.May 17 2020, 4:20 AM
DataDrake added a subscriber: DataDrake.

Please remove the empty abi_symbols* files. I still need clarification on whether or not the 32-bit executable is actually needed. Does this not just LD_PRELOAD the library? If it is needed, I'd like to see confirmed testing on a 32-bit executable. Thanks!

This revision now requires changes to proceed.May 17 2020, 4:20 AM

@DataDrake Can do. Let's just clarify some things before we start another revision.
The only diff between the 64 and 64 bit exec is this

< LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/usr/lib32/mangohud"
---
> LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/usr/lib64/mangohud"

LD_PRELOAD is in another variable and the same between 64/32 bits. I tested some games with opengl and vulkan and long story short some of them (ex: door kickers action squad) require the 32bits version. I don't know why however because some other (ex Hellblade) work ok with only the 64 bits exec installed. The 2 games listed in example are launched with exactly the same command and use the same proton/dxvk version.

Second point, a few days ago a new version was released, it changes nothing about the exec part but add nvctrl as optional dependency. I'm not familiar with nvidia in solus but it looks like this will bring a lot of heavy dependencies just for old nvidia card support. I plan on disable it, if that's ok.

Fanton added a subscriber: Fanton.May 20 2020, 1:17 PM
Trillon008 updated this revision to Diff 21622.Jun 3 2020, 12:13 PM

New version and test plan for 32bits mangohud. Removed unused files

Trillon008 edited the summary of this revision. (Show Details)Jun 3 2020, 12:17 PM
Trillon008 edited the test plan for this revision. (Show Details)
Trillon008 edited the summary of this revision. (Show Details)
DataDrake requested changes to this revision.Jun 13 2020, 5:33 PM

Please add a MAINTAINERS.md file listing yourself as maintainer.

Last fix. The rest looks good. Cheers!

This revision now requires changes to proceed.Jun 13 2020, 5:33 PM
Trillon008 updated this revision to Diff 21877.Jun 17 2020, 6:05 PM

Last update before release was supposed to be small it ended rather long.

  • added maintainers file
  • update to latest version, released 6 day ago
  • remove /usr/bin/mangohud.i686 binary as it is not required anymore but still created after compile
JoshStrobl requested changes to this revision.Jun 20 2020, 3:13 PM
JoshStrobl added a subscriber: JoshStrobl.
JoshStrobl added inline comments.
package.yml
37

Would it hurt to have it symlinked though in case something like lutris expects the .x86 binary to exist?

pspec_x86_64.xml
42

Let's just pattern all of this into the main package. Kinda silly just to have the two so files in a dedicated package in this case. I imagine most installing mangohud would expect that it'd work for 32-bit applications / Steam anyways.

Should be:

patterns :
    - /*
This revision now requires changes to proceed.Jun 20 2020, 3:13 PM

Requested change about how patterns are handled

Now everything is one package. I used to rename mangohud.x86 to mangohud.i686 to match solus scheme, but in order to make sure there is a full 32 bits compatibily I let the actual naming. There is a hardcoded reference to mangohud.x86 in mangohud (x86_64) which doesn't change a thing when in 64 bits but will go bork if full 32 which is not possible anymore since everything is in 1 package.
😆

Jacek awarded a token.Jul 3 2020, 6:36 PM
Jacek added a subscriber: Jacek.

Patch looks good to me. Just to confirm before landing, is this needing libglvnd-devel or does it need mesalib-devel? @DataDrake performed a mesalib upgrade (including all the stuff around libglvnd-devel) and the gl pkgconfig was moved out of mesalib. If it's mesalib specific headers then the gbm pkgconfig should be used.

I'll check that ASAP.
Anyway you can merge this into unstable and if effectively I need to remove libglvnd I'll make sure this happen when I'll update mangohud. It should happen soon enough.

Jacek added a comment.Jul 9 2020, 7:59 PM

I built using this package.yml and verified it works with Steam games, so this is ready to be accepted into the repos

JoshStrobl accepted this revision.Jul 10 2020, 2:48 PM

Thanks for the validation @Jacek and the response @Trillon008 =)

JoshStrobl requested changes to this revision.Jul 10 2020, 2:49 PM
JoshStrobl added inline comments.
package.yml
8

I did notice that this is set to xorg.display. This should really just be set to system.utils because the xorg.display component is really only intended for what is needed to actually run xorg bits.

This revision now requires changes to proceed.Jul 10 2020, 2:49 PM
JoshStrobl accepted this revision.Jul 16 2020, 9:37 AM

I'm going to be accepted and landing this, fixing the component after, just so vkbasalt and goverlay aren't blocked by it.

This revision now requires review to proceed.Jul 16 2020, 9:37 AM
This revision is now accepted and ready to land.Jul 16 2020, 9:39 AM
This revision was automatically updated to reflect the committed changes.