WayfireWM / wayfire

A modular and extensible wayland compositor
https://wayfire.org/
MIT License
2.36k stars 175 forks source link

Ignore idle inhibitors from surfaces which are not visible #1888

Open dagbdagb opened 1 year ago

dagbdagb commented 1 year ago

Describe the bug By default, the idle plugin deactivates DPMS timers whenever an application runs fullscreen. Which makes perfect sense for watching videos and stuff. Still, this only applies when the workspace with the fullscreen app is in focus.

If the terminal application kitty is started with '--start-as=fullscreen', DPMS timers will stay disabled until the kitty application has been disabled. Independent of kitty still being fullscreen or even in the current workspace.

To Reproduce Steps to reproduce the behavior:

  1. install kitty
  2. configure the [idle] plugin with dpms_timeout = 60
  3. start wayfire like this: wayfire -d | tee wayfire.debug.out
  4. start some terminal in workspace A, tail -f wayfire.debug.out
  5. change to workspace B, start firefox, make it fullscreen with F11
  6. switch back to workspace A, verify that you see 'Enabling idle timers for all seats' whenever you switch away from workspace B with the fullscreen firefox session, and vice versa.
  7. Now start 'kitty --start-as=fullscreen' in workspace C.
  8. switch back to workspace A and notice how idle timers were disabled when kitty was started, but does not enable when going back to workspace A.
  9. switching kitty to windowed mode (<super> UP in my config) does not change anything.

Expected behavior I would expect DPMS timers to behave the same, independent of which application is started in fullscreen.

Screenshots or stacktrace Dropping in and out of a workspace with a fullscreen kitty will render this in the debug log:

DD 05-09-23 22:26:30.650 - [wayfire-0.7.5/src/core/idle.cpp:15] creating idle inhibitor %p, previous count: %d0x55efa16e1af00
DD 05-09-23 22:26:30.650 - [types/wlr_idle.c:183] Disabling idle timers for all seats
DD 05-09-23 22:26:30.654 - [wayfire-0.7.5/src/core/idle.cpp:15] creating idle inhibitor %p, previous count: %d0x55efa13482a01
DD 05-09-23 22:26:33.485 - [wayfire-0.7.5/src/core/idle.cpp:23] destroying idle inhibitor %p, previous count: %d0x55efa13482a02
DD 05-09-23 22:27:58.790 - [wayfire-0.7.5/src/core/idle.cpp:15] creating idle inhibitor %p, previous count: %d0x55efa13482a01
DD 05-09-23 22:28:02.403 - [wayfire-0.7.5/src/core/idle.cpp:23] destroying idle inhibitor %p, previous count: %d0x55efa13482a02
DD 05-09-23 22:29:21.893 - [wayfire-0.7.5/src/core/idle.cpp:23] destroying idle inhibitor %p, previous count: %d0x55efa16e1af01

But only by killing said kitty terminal (independent of fullscreen state or not, will I get the following:

DD 05-09-23 22:29:21.893 - [types/wlr_idle.c:183] Enabling idle timers for all seats

This does not occur with firefox in a fullscreen session. Here, I get :

DD 05-09-23 22:38:45.695 - [wayfire-0.7.5/src/core/idle.cpp:15] creating idle inhibitor %p, previous count: %d0x55efa13482a00
DD 05-09-23 22:38:45.695 - [types/wlr_idle.c:183] Disabling idle timers for all seats
DD 05-09-23 22:39:36.678 - [wayfire-0.7.5/src/core/idle.cpp:23] destroying idle inhibitor %p, previous count: %d0x55efa13482a01
DD 05-09-23 22:39:36.678 - [types/wlr_idle.c:183] Enabling idle timers for all seats

... every time I drop in and out of the workspace with the firefox session in a fullscreen session. Killing firefox is not necessary for DPMS to kick in.

Wayfire version 0.7.5-r1 gentoo

I logged this issue with wayfire because I get excellent service here. Sorry. If the blame lies with kitty or glfw, maybe you guys can add an ever so tiny layer of what-am-I-talking-about-here, before I go to either of those?

Side note: How can I get some kind of idea about the state of the idle plugin? I can toggle it with a hot-key, is there a way to query the state?

dagbdagb commented 1 year ago

The following patch by soreau appears to fix this for me:

diff --git a/src/core/core.cpp b/src/core/core.cpp
index 594c4331..896ff3b1 100644
--- a/src/core/core.cpp
+++ b/src/core/core.cpp
@@ -157,9 +157,9 @@ void wf::compositor_core_impl_t::init()
     protocols.idle_inhibit = wlr_idle_inhibit_v1_create(display);
     idle_inhibitor_created.set_callback([&] (void *data)
     {
-        auto wlri = static_cast<wlr_idle_inhibitor_v1*>(data);
+        //auto wlri = static_cast<wlr_idle_inhibitor_v1*>(data);
         /* will be freed by the destroy request */
-        new wlr_idle_inhibitor_t(wlri);
+        //new wlr_idle_inhibitor_t(wlri);
     });
     idle_inhibitor_created.connect(
         &protocols.idle_inhibit->events.new_inhibitor);

Other than that it appears to remove the symptoms I can test for, I can't make any statement about the patch being correct or not.

ammen99 commented 1 year ago

The patch seems to basically disable any inhibitors created by clients. @dagbdagb Can you run kitty with WAYLAND_DEBUG=1 and attach the log here? (Please reproduce the issue as quickly as possible and kill/stop the terminal asap, otherwise the log may get too long).

dagbdagb commented 1 year ago

kitty-fs-inhibitors.txt

The attachment is the WAYLAND_DEBUG=1 log from starting kitty fullscreened in workspace A, immediately jumping to workspace B, waiting for DPMS timeout (10 seconds, but does not occur), jumping to workspace A, workspace C, workspace A, and finally "windowing" kitty and terminating it. On an unpatched wayfire 0.7.5.

If this wasn't what you had in mind, feel free to spoon-feed me specific actions with a particular order and/or timing.

ammen99 commented 1 year ago

I see, so the problem is that kitty keeps an idle inhibitor at all times when it is fullscreen. According to the protocol, Wayfire should ignore the idle inhibitor when kitty becomes invisible (but this is currently not implemented):

  If the surface is destroyed, unmapped, becomes occluded, loses
  visibility, or otherwise becomes not visually relevant for the user, the
  idle inhibitor will not be honored by the compositor; if the surface
  subsequently regains visibility the inhibitor takes effect once again.
dagbdagb commented 1 year ago

Ok. So we can pin this on wayfire?

Although, I do think I had one more observation which I still find a bit strange.

The observed behavior does not change, even if I "window" a kitty which was started with the 'start-as=fullscreen' option. Should I not expect it to?

ammen99 commented 1 year ago

Not really, kitty will have to destroy its idle inhibitor. I'm not sure what kitty's policy about this is. If they want to inhibit idle only for fullscreen surfaces, then that particular problem is a kitty bug.