Skip to content
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

libcosmic: Add desktop-file helpers #281

Merged
merged 1 commit into from
Jan 31, 2024
Merged

libcosmic: Add desktop-file helpers #281

merged 1 commit into from
Jan 31, 2024

Conversation

Drakulix
Copy link
Member

This is an attempt at unifying implementations of this across cosmic-applibrary, cosmic-launcher, cosmic-app-list (in cosmic-applets) and soon cosmic-store.

This is mostly based on the app-list implementation, as that parses most of the desktop files contents. Particular attention was given to allow for all the different filtering these apps do, as well as to easy icon handling. Additionally contains code necessary for handling gpu selection in the future (prefers_dgpu and explicitly setting a wayland_display for the spawn method).

This is just doing code deduplication and cleanup, the goal is not e.g. to do complete exec parsing in this PR.

I'll keep this as a draft until I have written proper implementations against all our use-cases, but in principle this is ready for review.

@mmstick
Copy link
Member

mmstick commented Jan 26, 2024

I noticed that COSMIC applications weren't running on integrated graphics by default unless explicitly setting WGPU_POWER_PREF=low. It's currently being worked around by setting low on application startup from libcosmic. But we could use the prefers_dgpu property here to automatically set this environment variable low for integrated, and high for dedicated.

@Drakulix
Copy link
Member Author

I noticed that COSMIC applications weren't running on integrated graphics by default unless explicitly setting WGPU_POWER_PREF=low. It's currently being worked around by setting low on application startup from libcosmic. But we could use the prefers_dgpu property here to automatically set this environment variable low for integrated, and high for dedicated.

So an app would lookup it's own desktop file and assuming it finds one check if it has the dGPU preference set? Sounds neat, but I don't think this would work in a sandbox like flatpak. I am honestly a bit annoyed of this behavior by wgpu. It should select whatever mesa picks by default to allow environment variables to work correctly.

@jackpot51
Copy link
Member

There's only two main env vars that will be set by switccheroo control, one for nvidia, one for amd. Our apps should set wgpu power pref if those are not set

@jackpot51
Copy link
Member

DRI_PRIME

the default GPU is the one used by Wayland/Xorg or the one connected to a display. This variable allows to select a different GPU. It applies to OpenGL and Vulkan (in this case “select” means the GPU will be first in the reported physical devices list). The supported syntaxes are:

DRI_PRIME=N: selects the Nth non-default GPU (N > 0).

DRI_PRIME=pci-0000_02_00_0: selects the GPU connected to this PCIe bus

DRI_PRIME=vendor_id:device_id: selects the first GPU matching these ids.

For Vulkan it’s possible to append !, in which case only the selected GPU will be exposed to the application (eg: DRI_PRIME=1!).

@Drakulix
Copy link
Member Author

There's only two main env vars that will be set by switccheroo control, one for nvidia, one for amd. Our apps should set wgpu power pref if those are not set

Honestly, parsing these environment variables manually feels like a hack as well and get more complicated when you consider things like __NV_PRIME_RENDER_OFFLOAD_PROVIDER to select a specific nvidia gpu.

I think the first thing we should do is neither force this to low or high, but use WGPU_ADAPTER_NAME to select a specific gpu. And for what gpu to select we should take the one advertised by the compositor, which we can get with a throw-away wayland/x11 connection.

Overall I am not happy we need to fix WGPU's enumeration here at all, but at least there is an open issue for that, so I am not entirely unhopeful that this will be fixed long-term upstream: gfx-rs/wgpu#3464

(Possibly something we could contribute.)

@jackpot51
Copy link
Member

This looks good to me, it just needs a cargo fmt, anything else you want to do in this PR?

@Drakulix Drakulix marked this pull request as ready for review January 31, 2024 14:05
@Drakulix
Copy link
Member Author

Ready for review, now that I have verified, that this is good enough for all potential users:

@Drakulix Drakulix merged commit bf05088 into master Jan 31, 2024
10 checks passed
@jackpot51 jackpot51 deleted the desktop-feature branch January 31, 2024 14:10
@jackpot51 jackpot51 restored the desktop-feature branch January 31, 2024 14:10
@jackpot51
Copy link
Member

Gonna leave the branch for you to delete when the prs are updated

@Drakulix Drakulix deleted the desktop-feature branch January 31, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants