Implement Idle Inhibit Protocol #568
Conversation
There was a problem hiding this comment.
Hmm, this doesn't seem to implement the protocol fully. Idle inhibitors are created for a specific surface and defined to be active exactly when that surface is visible on a given output.
An idle inhibitor prevents the output that the associated surface is
visible on from being set to a state where it is not visually usable due
to lack of user interaction (e.g. blanked, dimmed, locked, set to power
save, etc.) Any screensaver processes are also blocked from displaying.
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.
Likewise, the inhibitor isn't honored if the system was already idled at
the time the inhibitor was established, although if the system later
de-idles and re-idles the inhibitor will take effect.
|
Looks like my latest commits don't actually associate the inhibitor with the view. @ifreund is there a way to get the view given tha sub surface? I've tried using Or is there a way to determine whether a wlr surface is visible? |
|
Added handling for mapping subsurfaces to views, though the calls used are dependent on swaywm/zig-wlroots#43. With this, the idle inhibitor will be enabled only when the view is visible. @ifreund could you give it a review? |
|
@ifreund is there anything else blocking this PR from being merged? |
There was a problem hiding this comment.
Thanks for the patch and your patience waiting on a review.
This isn't a perfect implementation of the protocol, but that's due to shortcomings in river's current architecture (we need a scene graph) not shortcomings in this PR.
Despite that, I think having this implementation in river would be better than not having it. In the worst case we inhibit idle a bit too often which sounds less annoying than a display going to sleep while watching a movie. With the small change mentioned I'd be happy to merge this.
river/IdleInhibitorManager.zig
Outdated
| // If for whatever reason the inhibitor does not have a view, then | ||
| // assume it is visible. | ||
| if (node.data.view == null) { | ||
| inhibited = true; | ||
| break; | ||
| } | ||
| // If view is visible, | ||
| if (node.data.view.?.current.tags & node.data.view.?.output.current.tags != 0) { | ||
| inhibited = true; | ||
| break; | ||
| } |
There was a problem hiding this comment.
This check isn't really correct as it doesn't take e.g. layer surfaces or popups into account and doesn't check for visibility of views beyond matching them against the output's focused tags. A view might be hidden behind another opaque view even if it matches the output's tags.
However, there isn't really a way to do this properly and there won't be until we have a proper scene graph. Wlroots has a scene graph API now, but I haven't gotten around to updating river to use it yet.
There was a problem hiding this comment.
Yea, I wasn't too sure how river was detecting visible views, but that was what I found. Out of curiosity, where can I read more about scene graphs, and is there an open issue for this?
There was a problem hiding this comment.
Nope, no open issue. It's the next big thing I plan to work on for river though.
I pretty much learned about scene graphs by contributing to the wlroots scene graph implementation. I suppose the wikipedia page might be useful for a general overview though: https://en.wikipedia.org/wiki/Scene_graph
river/IdleInhibitorManager.zig
Outdated
| var view = View.fromWlrSurface(node.data.inhibitor.surface); | ||
| if (view) |v| { |
There was a problem hiding this comment.
I'd write this like so, what do you think?
| var view = View.fromWlrSurface(node.data.inhibitor.surface); | |
| if (view) |v| { | |
| if (View.fromWlrSurface(node.data.inhibitor.surface)) |view| { |
There was a problem hiding this comment.
Changed it in 93a95b4
Implementation of the idle inhibit protocol. Closes https://github.com/riverwm/river/issues/199
Depends on swaywm/zig-wlroots#43