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

GameControllerWakelock: Use dbus to inhibit screensaver #1897

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

infirit
Copy link
Contributor

@infirit infirit commented Oct 16, 2022

This actually works for me on KDE. I'll test XFCE later.

@infirit
Copy link
Contributor Author

infirit commented Oct 16, 2022

Joy, Xfce's screensaver doesn't implement org.freedesktop.ScreenSaver

@sonarcloud
Copy link

sonarcloud bot commented Oct 16, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@infirit
Copy link
Contributor Author

infirit commented Oct 16, 2022

But luckily it does use the same arguments and function names. Looked at Gnome in a VM and it also implements org.freedesktop.ScreenSaver. Any other relevant desktops I need to check?

@cschramm cschramm linked an issue Oct 17, 2022 that may be closed by this pull request
@cschramm
Copy link
Member

I'm not sure if that's a proper way. At least it's compatibility hell. There's org.xfce.ScreenSaver, org.mate.ScreenSaver, org.cinnamon.ScreenSaver, com.deepin.ScreenSaver, ... and other lockers do not have such an interface at all.

@infirit
Copy link
Contributor Author

infirit commented Oct 17, 2022

If they expose org.freedesktop.ScreenSaver I don't have to specifically handle them. xdg-screensaver doesn't work for me on plasma and I doubt it still work on gnome. I'll have a look at the others and see what they put on dbus.

Inhibit from GtkApplication does absolutely nothing for me.

@cschramm
Copy link
Member

cschramm commented Oct 18, 2022

If they expose org.freedesktop.ScreenSaver I don't have to specifically handle them.

They don't. The names I mentioned are both the bus and interface names. Object paths differ as well. I think I saw Inhibit methods on all of those interfaces.

xdg-screensaver doesn't work for me on plasma and I doubt it still work on gnome.

You mean the implementation that we currently have in place? Shoot. I thought we're only talking about simplifying things and extending to Wayland, not fixing. 😅

xdg-screensaver does support KDE, though. It actually uses the same interface as you do here for KDE 4 🤔 and targets the Gnome, MATE, and Cinnamon equivalents as well as other handlers.

Inhibit from GtkApplication does absolutely nothing for me.

Awesome... Could be the same reason as for xdg-screensaver suspend or the system needs to implement some handler. 🤷

@infirit
Copy link
Contributor Author

infirit commented Oct 18, 2022

If they expose org.freedesktop.ScreenSaver I don't have to specifically handle them.

They don't. The names I mentioned are both the bus and interface names. Object paths differ as well. I think I saw Inhibit methods on all of those interfaces.

xdg-screensaver

You mean the implementation that we currently have in place? Shoot. I thought we're only talking about simplifying things and extending to Wayland, not fixing. sweat_smile

Yup, I just tested this on the fresh debian 11 vm with xfce4, it's not working. Screen blanks after the timeout of 1 minute that I set.

xdg-screensaver does support KDE, though. It actually uses the same interface as you do here for KDE 4 thinking and targets the Gnome, MATE, and Cinnamon equivalents as well as other handlers.

If I monitor the destination with gdbus monitor -e -d org.freedesktop.ScreenSaver and call xdg-screensaver suspend wid nothing shows up.

Inhibit from GtkApplication does absolutely nothing for me.

Awesome... Could be the same reason as for xdg-screensaver suspend or the system needs to implement some handler. shrug

I don't know. I'll test the hacky script on the debian 11 vm and see if it actually works.

edit: The gtk application inhibit script also does nothing in the debian 11 xfce vm

@infirit
Copy link
Contributor Author

infirit commented Oct 22, 2022

I'm really tempted to just open a PR that removes it. If someone want to resurrect it with a method that works on various desktops they can open a PR.

@cschramm
Copy link
Member

I still don't get why it doesn't. If you look at https://cgit.freedesktop.org/xdg/xdg-utils/tree/scripts/xdg-screensaver.in, you see that it should do exactly what you're doing here in its screensaver_freedesktop function. If your change works but xdg-screensaver does not, either we fail to call it correctly or its broken on your system. Could it be that it simply does not get KDE_SESSION_VERSION so that it tries its KDE 3 handling?

@infirit
Copy link
Contributor Author

infirit commented Oct 22, 2022

I got a clue $ gdbus call --session -d org.freedesktop.ScreenSaver -o /org/freedesktop/ScreenSaver --method org.freedesktop.ScreenSaver.Inhibit "test" "test" also doesn't work while in blueman it does. It looks like the calling process has to remain running for the Inhibit to persist. xdg-screensaver for sure doesn't. it does so idk

edit: also, it's not just plasma, it fails just as much on xfce4 on a default debian 11install

@infirit
Copy link
Contributor Author

infirit commented Oct 22, 2022

It doesn't support xfc4's screensaver, wip https://gitlab.freedesktop.org/xdg/xdg-utils/-/merge_requests/38

Ah, and I was right about the process needing to be running, here is why it doesn't work on plasma https://gitlab.freedesktop.org/xdg/xdg-utils/-/issues/114 (only 5 years old).

All these are old af so I don't think we should rely on it anymore. I rather just read XDG_CURRENT_DESKTOP as I doubt we'll see any fixes from the xdg folks anytime soon.

@cschramm
Copy link
Member

Oh wow, now that makes a lot of sense... 😒

So, ideally we'd have a library that we can use from within our process to make the KDE Inhibit calls work properly and with Xfce support and probably a lot more.

I don't want to keep you from building that monster, but I'm not sure if this plugin is such an important feature. To be honest I don't fully understand the use case at all. A screensaver / locker should not jump in during gaming sessions anyway, at least due to regular input, no?

@infirit
Copy link
Contributor Author

infirit commented Oct 22, 2022

A screensaver / locker should not jump in during gaming sessions anyway, at least due to regular input, no?

Controller and joysticks are not considered regular input and won't reset the timer.

I don't want to keep you from building that monster, but I'm not sure if this plugin is such an important feature.

Nope, now that I start to think about it'll never end. So my suggestion is to drop the plugin for now. And I'll update #1896 restoring the plugin and if it ever starts working it can be merged rebased and merged.

I really thought, cool, standard freedesktop interface, this should be simple 😭 .

@cschramm
Copy link
Member

Thanks für the explanation. 👍 I thought controllers usually do show up as input devices.

I'm fine with the removal, just want to add: If I'm not mistaken, the current implementation does not work with KDE 4's screensaver and Xfce's but it does seem to work for others, including MATE's, Cinnamon's, xscreensaver, xautolock, and anything that respects the X screensaver options set with xset.

@github-actions github-actions bot added the stale label Feb 22, 2024
@github-actions github-actions bot closed this Mar 1, 2024
@cschramm cschramm removed the stale label Mar 1, 2024
@cschramm cschramm reopened this Mar 1, 2024
Copy link

sonarcloud bot commented Mar 1, 2024

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

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.

Handle screensaver independent of X11
2 participants