Details

Reviewers

Commits

rP26178: Fix two typos noticed by @Nescio, and use the performance GPU on Linux too.

Summary

PrefersNonDefaultGPU=true makes laptops/computers with multiple GPUs prefer to launch the application with the most powerful GPU available by default when launched through the icon.

Currently this key is used in games such as SuperTuxCart and presumably others.

The option allows users to use their dGPU without having to worry about setting environmental variables etc.
Currently in order to run the game on the discrete GPU with recent versions of GNOME the user has to right-click the icon and then press the "Launch using Dedicated Graphics Card" button, using this patch however no other actions are required rather than just clicking the game's icon.

Also on GNOME, once the key is set a user can choose to run the game on the integrated GPU by following the same procedure used to run to run the game on the discrete GPU.
That is, if a user on a laptop wants to save battery using the iGPU he can easily do so.

For reference purposes:
DesktopEntryKeys specification
Discussion by the specification's authors
Useful reading

Test Plan

Use a dual-gpu system with GNOME (common scenario: optimus laptop)
Clicking 0 A.D. icon should launch the game straight on the dGPU
Right-clicking 0 A.D. should let the game run on the iGPU

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

DynamoFox requested review of this revision.Jan 25 2021, 4:25 PM

DynamoFox created this revision.

Added context to diff.
Added further stuff in summary to make clear that PrefersNonDefaultGPU=true doesn't make the system use the iGPU instead of the dGPU if the dGPU is default.

Owners added a subscriber: Restricted Owners Package.Feb 8 2021, 1:50 PM

The patch seems useful. How it would behave for versions not supporting PrefersNonDefaultGPU?

ps/trunk/build/resources/0ad.desktop
10	Comment should be before the line or after a space. Like we do for other configuration files.

Placed the comment above the key (it was also an error apparently, whoops).
Bumped Version of compliance to 1.4

In D3475#155321, @vladislavbelov wrote:

The patch seems useful. How it would behave for versions not supporting PrefersNonDefaultGPU?

I have no experience nor knowledge with DE different than GNOME, anyway I believe that even if the implementation (i.e. switcheroo-control on GNOME) is missing it should still exceute the program, but on the default GPU, like if the key didn't existed.
Certain distribution (i.e. Arch Linux -> Manjaro) don't even provide the necessary package to implement this key, still, at least on gnome no brackage occurs.

SuperTuxKart for example has already been using this for a while without worrying too much anyway.

I've also bumped the Version compliance number of the .desktop file, checked the validity with desktop-file-validate and tested it on my machine, all looks good.

(You should generate from the 0ad folder, the file in question is build/resources/0ad.desktop.)

ps/trunk/build/resources/0ad.desktop
19	While at it, please: correct `Realtime` → `Real-Time` purge `Infantry;Cavalry;Siege Engines;Fortress;` (not very meaningful, nor complete) purge `Celtics` [sic] purge `Hellenes` correct `Mauryans` → `Mauryas`

Stan added a subscriber: Stan.Feb 8 2021, 5:47 PM

Stan added inline comments.

ps/trunk/build/resources/0ad.desktop
19	Missing Kushites

Made some changes to Keywords:

Removed some meaningless keywords
Corrected some old typos

@wraitii @Stan this needs to be committed for a24 no?

any news or decision?

For my end, I think distributions/maintainers are free to patch if they want to (this seems something they could be concerned with). Not planning to merge this for A24.

I don't really know anything about it anyways, I don't think I'm really the right person to merge this or to decide on this anyways.

For me it's A25 material. I will ask maintainers about it but so far the people I asked were really mitigated about it.

Silier added a reviewer: Restricted Owners Package.Feb 16 2021, 6:55 PM

@s0600204 do you have any experience with such files ?

For the fun of it we could add GenericName=Real-Time Strategy Game too

build/resources/0ad.desktop
2 ↗	(On Diff #15894)	Seems like we can bumb it to 1.5 now?
11 ↗	(On Diff #15894)	Seems indeed a good idea
21 ↗	(On Diff #15894)	Not sure why you removed some of the keywords. The keywords were explicitly added in rP18904 and should be kept. Obviously adding Kushites is good.

This revision now requires changes to proceed.Sep 9 2021, 5:20 PM

Stan added inline comments.Sep 9 2021, 5:25 PM

build/resources/0ad.desktop
11 ↗	(On Diff #15894)	Couldn't find any truth at whether it can hurt or not on linux. Maybe you'll have more luck?
21 ↗	(On Diff #15894)	See Nescio's inline above While at it, please: correct Realtime → Real-Time purge Infantry;Cavalry;Siege Engines;Fortress; (not very meaningful, nor complete) purge Celtics [sic] purge Hellenes correct Mauryans → Mauryas

DynamoFox marked 3 inline comments as done.Sep 9 2021, 7:16 PM

DynamoFox added inline comments.

build/resources/0ad.desktop
2 ↗	(On Diff #15894)	I couldn't find what changed between these two versions, currently 1.4 should provide all the features we need (that is, the PrefersDefault* key). Also desktop-file-validate didn't support that version of the specification back when I wrote this patch...
11 ↗	(On Diff #15894)	Couldn't find any truth at whether it can hurt or not on linux. Maybe you'll have more luck?
11 ↗	(On Diff #15894)	Couldn't find any truth at whether it can hurt or not on linux. Maybe you'll have more luck? I fail to see how it could hurt, at worst if there are problems with the .desktop file it won't appear in the application launcher (but it will still be possible to run 0ad from the terminal). It might be possible that certain users with a broken (for whatever reason) multi-gpu setup might have issues, but it's their responsibility to have a working OS tho.

bb added inline comments.Sep 9 2021, 8:40 PM

build/resources/0ad.desktop
2 ↗	(On Diff #15894)	desktop-file-validate still doesn't support it https://gitlab.freedesktop.org/xdg/desktop-file-utils/-/issues/59
11 ↗	(On Diff #15894)	From https://specifications.freedesktop.org/desktop-entry-spec/1.5/ar01s06.html If true, the application prefers to be run on a more powerful discrete GPU if available, which we describe as “a GPU other than the default one” in this spec to avoid the need to define what a discrete GPU is and in which cases it might be considered more powerful than the default GPU. This key is only a hint and support might not be present depending on the implementation. This suggests that any OS can choose to implement some GPU handling or not. So nothing should be broken (and if something is broken it is the OS' fault). Also as STK and others are using it, I expect no issues.
21 ↗	(On Diff #15894)	While that explains why the author removes them, it isn't really correct. These are search keywords for users to search in their applications. I rather put to many (even if not very meaningful) entries than too few. We want ppl to find the game, do we? Obviously the spelling issues should be fixed.

Stan added inline comments.Sep 9 2021, 8:42 PM

build/resources/0ad.desktop

11 ↗

(On Diff #15894)

From https://specifications.freedesktop.org/desktop-entry-spec/1.5/ar01s06.html
If true, the application prefers to be run on a more powerful discrete GPU if available, which we describe as “a GPU other than the default one” in this spec to avoid the need to define what a discrete GPU is and in which cases it might be considered more powerful than the default GPU. This key is only a hint and support might not be present depending on the implementation.
This suggests that any OS can choose to implement some GPU handling or not. So nothing should be broken (and if something is broken it is the OS' fault). Also as STK and others are using it, I expect no issues.

Source for Stk ?

Btw reference file for stk is here https://github.com/supertuxkart/stk-code/blob/master/data/supertuxkart.desktop

build/resources/0ad.desktop
11 ↗	(On Diff #15894)	https://github.com/supertuxkart/stk-code/blob/master/data/supertuxkart.desktop

Any decision regarding inclusion to master whether we agree and thus it is committable or disagree?

In D3475#185712, @asterix wrote:

Any decision regarding inclusion to master whether we agree and thus it is committable or disagree?

The concern is still raised, so it needs to be fixed first. After that we need some testing of different environments. If it works I believe it should be useful.

@nwtour: feel like commandeering this?

nwtour added inline comments.Dec 30 2021, 11:55 AM

build/resources/0ad.desktop
2 ↗	(On Diff #15894)	https://cgit.freedesktop.org/xdg/xdg-specs/commit/?id=7adfee6bac04704de98daf6ed818e526a4310d98 1.5 - development version. "Request for Comments". Any parameter can be renamed. 1.4 - standart. with fixed keys

How it works
For this to work in the gnome3 (KDE did not study) - you must install a separate package https://packages.ubuntu.com/bionic/gnome/switcheroo-control

Microservice connects to "DBus" bus and sets environment variables for each launch of the gnome shell
https://gitlab.freedesktop.org/hadess/switcheroo-control/-/blob/master/src/switcheroo-control.c#L235

Without installing this package - no ENV variables are set and there is no context menu (even if PrefersNonDefaultGPU parameter is set in desktop file)

If PrefersNonDefaultGPU option is not specified in the desktop file, then the variables are set if you select 'Launch using Discrete Graphics Card' in the context menu
If specified, then by default the variables are set, but you can run it without variables through the context menu ''Launch using Integrated Graphics Card''
https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1226/diffs#8226d44edae1ca00c1e3f1b8b208d2a669f40445_2536_2520

Description of ENV variable DRI_PRIME=1 https://wiki.archlinux.org/title/PRIME

In D3475#188175, @Stan wrote:

@nwtour: feel like commandeering this?

What I should do?

My main concern was the GPU thing, but it's harmless enough and STK does it;

build/resources/0ad.desktop
21 ↗	(On Diff #15894)	Just re-add the tags that were removed :)

nwtour commandeered this revision.Dec 30 2021, 2:32 PM

nwtour added a reviewer: DynamoFox.

Keep keywords except: Realtime -> Real-Time, Mauryans -> Mauryas

In D3475#188182, @Stan wrote:

My main concern was the GPU thing, but

In MESA source code 5 references to DRI_PRIME (2 for Vulkan) and all about select of the device.
So if the game window starts - Inside the game, the behavior will not change