[PATCH 0/4] Add mangohud (update dear-imgui)

Hi Guix,

Here is a short series of patches to add mangohud (a cool Vulkan/OpenGL overlay for things like framerate and hardware display in games). There were a few steps involved that required some tweaks to other packages. Here is the breakdown, corresponding to the forthcoming patches:

1. pciutils-no-zlib: A new hidden package to provide uncompressed hardware info (pci.ids). Mangohud and at least one other package I have read this file directly, which is typically uncompressed, i.e. on Arch. We could later change it in pciutils or have both uncompressed and compressed, or a separate package just for the hardware info. Since this would be a bigger change, I opted for a simple hidden package of the full pciutils (I think programs will need the rest of the package anyway).

2. spdlog: Build with '-fpic'. Since this is compiled as a static library, and upstream implies this is more typical, compile it in a way to be useful to shared libraries as well (needed for mangohud). I checked a random package that uses spdlog and it built fine at least.

3. dear-imgui: Update to 1.81. This not the latest, but most recent of the Debian makefiles we use. The only dependent is Ogre (very out of date) which just uses the source, but needs 1.79. I made the previous one a hidden package and updated Ogre to use that. There were enough build changes that I didn't think an inherit would be very clean. Also added in the static library output (preferred by mangohud). New is many inputs for the backends (optional).

4. mangohud: The new addition. Unbundled libraries to use guix's spdlog, dear-imgui, fixed hardcoded paths. Tested that it builds and works. The only guix lint complaint was about the dear-imgui static input name not matching (also needed dear-imgui for the headers and pkgconfig file).

Okay, that's it! A few changes but I think these are not invasive and were the cleanest on my end to use.

Thanks!

John

Empty Message

Empty Message

Empty Message

Empty Message

Hello,

This patch (2/4 build option for spdlog, part of the MangoHud patch series) may be superseded by https://git.savannah.gnu.org/cgit/guix.git/commit/?id=b1542d59606919d0da04914fa6916b85354e2f89 (CCing Nicolas, hope that's okay!) I've built MangoHud with the shared spdlog before, seems to work.

However, given what spdlog has in its documentation, I wonder if it would be better to default to the static version as before? I think the "-fpic" build option would also work for shared linking in general? Or does that not work for nheko, which this change was for I believe?

I'm not really sure though, nor have a strong opinion, just going based on spdlog's info and that some projects (like MangoHud) build spdlog as a static library by default when bundling. I'm guessing it is for performance reasons, though I don't know if that is realized.

John

Hi Maxim,

I wanted to ping you on (at least) the imgui part of this patch, since you had added imgui 1.86 in 1a4cc954d2fcea172a450ae03419b7fdda28b81e to see how we should sort out the different imgui versions we have. I hope that is okay!

This was my note in the original patch:

My inclination is to keep the old 1.79 as a hidden package for Ogre until Ogre can be updated. Likewise, the package I'm trying to add, mangohud, currently requires imgui 1.81, so perhaps add this as a hidden package as well?

Or it could be a public version since it is the current one that follows the Debian makefiles of the dear-imgui package we currently have. And for one, it adds the pkg-config file which is useful. Then once mangohud has a newer version with the current imgui version it could be removed, as the only package using it as an input.

WDYT?

Thanks!

John

Hi John,

John Kehayias <john.kehayias@protonmail.com> writes:

That should solve the problem, yes!

Do you mean this:

https://github.com/gabime/spdlog/wiki/How-to-use-spdlog-in-DLLs?

Typically in Guix the libraries are offered as shared objects by

default; and when desirable a static output can be added to which the

static objects are copied.

You could add a static output if you have such a need :-).

Maxim

Hi Maxim,

------- Original Message -------

On Sunday, February 20th, 2022 at 11:11 PM, Maxim Cournoyer wrote:

Yes, that's what I meant. I'm not familiar with spdlog, but was just going based on that and seeing packages build a static spdlog to use.

Got it. Personally, it doesn't matter much to me, I'm only assuming projects use the static version due to what upstream says and/or for performance reasons (no idea if that is realized though).

In the interest of moving this forward, I'm definitely fine leaving spdlog as it was updated. We could add a note that upstream and other projects prefer a static build and leave that to add as a TODO. I did take a stab at it some time ago, but it got slightly messy in trying to do it all together (I suppose I could just run the build a second time, but I think it also required modifications to other files to make the static build work separately? I forget now.)

Any thoughts on the imgui details? For me, I'd go for keeping both current (old) dear-imgui as hidden for ogre, adding the one I have here (hidden too? though useful Debian additions in the build) for mangohud, and consolidating to one up to date version once everything can use that. We could use the imgui you added for that, or wait for the more complete Debian build files we are using for dear-imgui right now (the pkg-config, for instance, is helpful).

Finally, the pciutils hardware data pci.ids came up in a new patch: https://issues.guix.gnu.org/54069I commented on that issue about doing something similar here. I think we can figure out what to do in a core updates change (due to pciutils dependents). In the meantime, a pciutils variant and/or a separate hwdata package would be helpful. Funny enough, this is the last non-trivial change in this patch series, brought up by someone else. I guess the repeated work shows this would be useful though.

Would love to finally get this merged, with whatever changes we decide for imgui and pciutils. WDYT?

Thanks!

John

Hi John,

John Kehayias <john.kehayias@protonmail.com> writes:

[...]

I sorted out the imgui vs dear-imgui mess I had created yesterday by

upgrading our 'ogre' package to its latest version and using imgui 1.86

for it (it wouldn't work with 1.87).

So now we have imgui@1.87 and imgui@1.86 available. I suggest you try

one of these and if it fails you can try adding yet another public

version for it.

I've looked at the pciutils issue; it seems suboptimal that pciutils

support having the database compressed yet doesn't provide an easy API

to look things from it (or perhaps it does and this is what mangohud

should use? to be investigated).

I'll try to have a look later into pciutils and whether it offers some

kind of API that users of the DB should use instead of manually parsing

their way through :-).

Thanks,

Maxim

Hi Maxim (and anyone else),

Attached is an update to this patch series, reduced to just one patch. This relies on the hwdata package added in https://issues.guix.gnu.org/54069#2 and the update to imgui I sent in https://issues.guix.gnu.org/54132

With that, mangohud just uses imgui-1.86 (the newer version has a breaking change). The modifications to the meson.build is handled in a build phase substitute* rather than a patch, as it needs the imgui location.

I've checked this builds and works. Let me know if any other modifications are needed, other than the hwdata and imgui fixes referenced.

Thanks!

John

Attached is a revised patch to use the new hwdata package with inputs, so the correct input is now `(,hwdata "pci"). I checked that it applied and built on master.

John

Hi John,

John Kehayias <john.kehayias@protonmail.com> writes:

I took the freedom to make some cosmetic adjustments:

1. ordering of inputs

2. ordering of fields (match the conventions, such as the 'home-page'

field appearing before the synopsis)

3. indent some longer lines so they fit under 80 columns

4. normalize paths passed to search-input-file by stripping lead '/'.

like so:

And pushed 31ecd80db7.

Thank you!

Closing.

Maxim

Hi Maxim,

------- Original Message -------

On Saturday, February 26th, 2022 at 9:31 PM, Maxim Cournoyer wrote:

Thanks, will take note for the future. No excuse for not alphabetizing when I know I did it twice. Insert ashamed emoji.

Thank you! Very happy to finally have this in, along with the various other package changes along the way.

John

I think it requires `(,hwdata "pci") as input, not just hwdata?

------- Original Message -------

On Tuesday, March 1st, 2022 at 8:37 PM, Brendan Tildesley wrote:

Yes, that's right, and what was in the most recent version of the patch (which was pushed).

close 53015

quit