feat(inhibit): Add inhibit module

[slowsage]

Resolves #929.
Sample config:

{
      type = "inhibit"
      backend = "systemd"
      durations = ["30m" "1h" "1h30m" "*2h" "inf"]
      on_click_left = "toggle"
      on_click_right = "cycle"
      format_on = "☕ {duration}"
      format_off = "💤 {duration}"
 }

Notes:
*2h => * implies 2h is picked as default.
Can pick systemd or wayland backend.
systemd persists across ironbar restarts (launches a transient service that sleeps for desired time).
wayland surface dies with ironbar process.
Inhibit.md included.

[JakeStanger]

Thanks for this.

I've raised a few comments around areas I feel need attention before this can go in. If you have any questions let me know.

There are a lot of places where the code feels quite cramped - can you add some blank lines between statements to try and space things out a bit please? The rest of the codebase should give a feel.

[slowsage]

These are solid points Will get back with an updated PR.
I much prefer systemd since I restart hyprland which restarts ironbar. For this reason, I wanted something persistent as default - fine with either.

[JakeStanger]

My personal opinion on that:

• The Wayland protocol is already very well supported, and as Wayland is a hard requirement, it's more likely to be available (ie vs Void/Artix/Gentoo/etc users where systemd may not be present).
• As it is a protocol, it means there could be compositor-specific implementation details, which talking directly to systemd would bypass these. I've no idea what extra server-side logic you could tie to idle inhibit, but best to make it available by default.
• It sounds to me like your use-case is fairly niche. Most people are going to keep their compositor and bar running for as long as the machine is up.

I'd therefore say Wayland by default, Systemd available as fallback.

[slowsage]

Ended up needing a new client and removing systemd (for v1) and figured you'd have some thoughts.

After going through client code, I agree - systemd should be postponed till a proper systemd client is created - out of scope for now. Would be wasteful to use the niche systemd use-case to drive the client. systemd should be its own flag etc - want to wait to add that.
Why new gtk-wayland client?:

1. The wayland client cannot be reused - it gets the connection before gtk is loaded (by design - its meant to be used before ui - from comments). Will get super confusing to have one client for two connections.
2. I could have changed bar.rs as well to cache the surface but that seemed not ideal - did not want to change bar to accommodate inhibit.
3. The client could be called inhibit/inhibitor/etc. - open to suggestions if design looks ok. I figured each client wraps a resource so each client name maps to a resource. module names map to functionality. This client wraps the gtk enabled wayland connection [where surface id from wayland can be found], so I called it gtk_wayland.

Either way, it looks like we need a wayland conn object post gtk initialization -> implies new client. Please let me know if you have any thoughts. Not sure if its possible without perf hit to make the wayland client connection work with gtk surface id. Seems pretty core - did not want to change it.

Please let me know what you think - dont like a dedicated client for this but dont see an obvious, consistent way out.
Thank you!

Edit: High level feedback ok for now.

[JakeStanger]

I'm not sure if point 1 is fully correct. The wayland client gets initialised at startup (before the UI), but it persists for the application lifetime, and there are modules that both send/receive events from it consistently (clipboard, focused, launcher). The architecture is a little complicated because there's effectively two layers of channels that requests/responses need to travel through (module <--> wayland client <--> wayland environment) but it should be possible.

You'd have to get the surface at request-time, rather than init-time, but that should be virtually free.

I'd rather not add to the Bar if not necessary, but I'm not too strongly against point 2 if you deem it the best path forward.

As for option 3, agreed it shoul dbe named after the service/resource name rather than the module name. For Wayland this should ideally match the protocol name.

[slowsage]

I'm not sure if point 1 is fully correct. The wayland client gets initialised at startup (before the UI), but it persists for the application lifetime, and there are modules that both send/receive events from it consistently (clipboard, focused, launcher). The architecture is a little complicated because there's effectively two layers of channels that requests/responses need to travel through (module <--> wayland client <--> wayland environment) but it should be possible.

I had an earlier draft that tried this but ended up with one client and two wayland connections. The issue is gtk/gk4_wayland has its own wayland connection (it should really take one as an arg but oh well). Trying the window surface id from gtk/gdk4_wayland post gtk init on the old connection through existing wayland client (for inhibition) did not work. Debugging also became less clear with two connections. We could keep it as one client, one connection by having it support a phase transition from pre-gtk -> gtk connection as a backup.

You'd have to get the surface at request-time, rather than init-time, but that should be virtually free.

I tried getting the widget (from button) at click and passing that. It still did not work with the non gtk wayland connection object.

Why I feel this is appropriate for now:
The main issue is having a separate post-gtk wayland connection in addition to existing connection is confusing. For the sake of scope creep, I think its better to have a separate diff to consolidate later.

Happy to be assigned an issue to investigate: I want to run some tests to see if gtk connection is actually slower and see what tasks exactly need running before ui - perhaps they can be queued or something else. There are a few possibilities on which direction to take based on what will be found. It could have its own set of tradeoffs, testing etc - need to dig into timings, slowdowns etc between two clients.

Worst case, we may decide to swap out the old connection on first inhibition as I mentioned above. Did not want to change such a core part of the code when adding a module. Keeps testing much more manageable.

Lmk what you think.

[slowsage]

Might be much simpler: gtk4::Application::inhibit().
Are you familiar with this api?

[JakeStanger]

Good find.

https://docs.gtk.org/gtk4/method.Application.inhibit.html

I've not used it before, but it certainly sounds like it does what we want? If so, that'd be a much cleaner implementation.

If not, am I right in thinking the need for the separate wl client stems from the request to use the bar's existing surface? If it's just that adding the complexity then using a dummy surface as before might be preferable after all.

[JakeStanger]

This will still need a central client-based implemenentation btw, as otherwise it will create a seperate inhibitor per module instance

[slowsage]

Its ready for review.
Monitor name is now the key to support multi monitors to hopefully inhibit screens independently based on which monitor the button is on. Code now uses smart pointer for multiple inhibits on same monitor.

Picked gtk over ModuleInfo for monitor name - to avoid passing around an extra arg.

[JakeStanger]

Cool, getting there. I appreciate you have put a lot of work into this and there has been a lot of big changes along the way, so a big thanks for keeping at it and being so patient.

There's a few more concerns I have, most are pretty minor at this point. Once those are cleared up I reckon we'll be about ready to go.

[JakeStanger]

Is there a particular reason or use-case why we want this to be per monitor, if the inhibit state is system-wide? To me this would seem confusing vs sharing this state across the whole application.

[slowsage]

Yes - this is intentional. I do not want to assume much about client environments. Given the state of apis and inconsistencies, seemed better to expose what its doing while leaving in deduplication - can make it a single instance or let each module manage later. The key issue is other system components that need to respect the state set by gtk/wayland. Since the wayland api requires surface and sleep can be at the monitor level, I aggregate at monitor level.

I think this is the safest medium between no client and singleton inhibit.

A systemd/dbus client will be the other option.

[JakeStanger]

Sorry I'm not sure I'm following the logic here. The inhibit is created/managed at the GTK application level, which means it lives for as long as Ironbar runs and we don't need to think about the other potential APIs. Why not use a singleton?

[slowsage]

Thanks for your patience and persistence in pushing back - had a couple of false starts before realizing:

1. gtk4's inhibit() is the way to go. it delegates to dbus and wayland (wayland - Not applicable for ironbar - see below).
2. wayland is a fallback after dbus fails. dbus is system wide. wayland may choose to inhibit (and docs says this is what it does) per screen.
3. wayland - wl_surface based inhibition is not available for gtk-layer-shell apps - inhibit() silently fails for non top-level apps:

   // gtkapplication-wayland.c:180-192:
   
     if (flags & GTK_APPLICATION_INHIBIT_IDLE && window && impl->application == gtk_window_get_application (window))
     {
         surface = gtk_native_get_surface (GTK_NATIVE (window));
         if (GDK_IS_WAYLAND_TOPLEVEL (surface))
         {
             success = gdk_wayland_toplevel_inhibit_idle (GDK_TOPLEVEL (surface));
             if (success)
             {
                 flags &= ~GTK_APPLICATION_INHIBIT_IDLE;
                 inhibitor->surface = surface;
             }
         }
     }

   The only way to do wayland protocol based inhibition is to reuse the surface (gdk4_wayland crate) /create a dummy wl_surface. This adds a lot of code with gdk4_wayland or adds a invisible surface (not the cleanest solution) as you saw in my previous attempt.
4. This is the dbus flow:
   gtk::Application::inhibit(None, IDLE, reason)
   org.freedesktop.portal.Desktop (xdg-desktop-portal-gtk)
   org.freedesktop.ScreenSaver (hypridle/swayidle/gnome-session)
5. gtk4 has two flags: IDLE and SUSPEND.
   dbus IDLE is inhibit picked up by xdg-desktop-portal-gtk which then sends to the ScreenSaver dbus interface. This should inhibit suspend on hypridle, swayidle and gnome-session. However gnome-session also allows for manual inhibition (laptop lid close, etc.) when SUSPEND is passed. This is out of scope.

Main changes:

1. Cookie is now a singleton, not per-monitor.
2. Strict validation/deserialization around durations and default_duration consistent with other modules. Now, it fails when default_duration not in list.

[JakeStanger]

Thanks again for your hard work, implementation looks solid now. One last round of tiny changes and then I think we're good to merge.

I will be holding off merging for a couple of weeks btw, as I want to ensure the GTK4 update is solid and ship that before introducing any new major features.

[JakeStanger]

LGTM! This'll be first in for 0.19.

[slowsage]

Since you are not going to merge for a while, will undraft after running it for a while to catch any issues - a lot of interacting pieces with this one.

[slowsage]

Feel its ready. Merge and assign issues as appropriate. See this for possible hypridle bugs (that this module helped uncover).

[Brisingr05]

Works properly on niri compositor with Swayidle or Hypridle.