[PATCH wayland-protocols v3] Add screensaver idle inhibitor protocol

[Bryce Harrington]

On Fri, Jun 03, 2016 at 10:39:24AM -0500, Yong Bakos wrote:
> >>> +	This destroys the inhibit manager.
> > 
> > Good addition. Should probably say how destroying the manager affects
> > the inhibitors created from it (no effect at all)?
> 
> Agreed w/ pq. But, is the inhibit manager a /singleton/ global?

On the client side yes.

> >>> +      </description>
> >>> +    </request>
> >>> +
> >>> +    <request name="create_inhibitor">
> >>> +      <description summary="create a new inhibitor object">
> >>> +	Create a new inhibitor object associated with the given surface.
> >>> +      </description>
> >>> +      <arg name="id" type="new_id" interface="zwp_idle_inhibitor_v1"/>
> >>> +      <arg name="surface" type="object" interface="wl_surface"
> >>> +	   summary="the surface that inhibits the idle behavior"/>
> >>> +    </request>
> 
> Does it make sense to associate inhibitors to surfaces, rather than clients?
> See below.
> 
> 
> >>> +
> >>> +  </interface>
> >>> +
> >>> +  <interface name="zwp_idle_inhibitor_v1" version="1">
> >>> +    <description summary="context object for inhibiting idle behavior">
> >>> +      An idle inhibitor prevents the output that the associated surface is
> >>> +      visible on from being blanked, dimmed, locked, set to power save, or
> >>> +      otherwise obscured due to lack of user interaction.  Any active
> >>> +      screensaver processes are also temporarily blocked from displaying.
> >>> +
> >>> +      If the surface is destroyed, unmapped, becomes occluded or otherwise
> >>> +      loses visibility, the screen behavior is restored to normal; if the
> >>> +      surface subsequently regains visibility the inhibitor takes effect once
> >>> +      again.
> 
> I don't believe this is a good choice. Imagine the case of a surface-less
> 'inhibitor daemon.' There may be no visible surface (is my thinking out
> of scope here?). Imagine another case, that of a "caffeine" widget. This
> widget's surface would be hidden when another app is fullscreen.

This use case did get discussed a bit in previous rounds.  But such
tools were felt to essentially be just crude workarounds for lack of
proper API level control over things.  Sort of in the same class as why
we don't provide an xrandr-equivalent tool for configuring the monitors
- don't provide tools to work around bad designs, just fix the design so
such tools are superfluous.  :-)

> Furthermore, I don't believe that inhibition should be coupled to outputs.
> See below.
> 
> 
> >>> +
> >>> +      Note that in the case of a compound window (a surface with
> >>> +      sub-surfaces), the inhibitor effects should be inherited from the top
> >>> +      level surface.  
> >> 
> >> Does this mean that if we have a subsurface with an inhibitor object,
> >> and for example an xdg_surface without one, the subsurface would still
> >> not inhibit the idle behavior, as it'd inherit the behavior?
> >> 
> >> If it should work like this, maybe we should make it an error to create
> >> an idle inhibitor on a subsurface. If it should inhibit, I think it
> >> needs to be clarified.
> > 
> > Right, I was thinking something like: Sub-surfaces that do not have an
> > inhibitor associated inherit the inhibition behaviour from their
> > parent. This applies recursively.
> > 
> > To clarify what I mean, lets consider an artificial case like this:
> > 
> > wl_surface A is a top-level, only on output 1.
> > wl_surface B is a sub-surface to A, only on output 2.
> > wl_surface C is a sub-surface to B, only on output 3.
> > 
> > wl_surface B has an inhibitor associated. All surfaces are completely
> > visible etc. on their own outputs.
> > 
> > Output 1 will go to powersave: no surface is inhibiting it.
> > 
> > Output 2 does not go to powersave: B inhibitor is in effect.
> > 
> > Output 3 does not go to powersave: C inherits B's inhibitor behaviour.
> 
> I don't believe that inhibition should /only/ be coupled to outputs.
> In the case of a "caffeine" widget or daemon, the use case is to
> prevent /all/ outputs from idling, regardless of what output the widget's
> surface resides upon.

You're right in gathering that you missed some earlier discussion on
this.  :-)  In the original RFC this protocol did provide an
"inhibit idle on all-outputs" type function but in earlier reviews there
just wasn't a solid use case for that so I took it out.

I believe the reason software like you mention operate on all monitors
is just that there isn't a convenient API for doing the per-output level
control.  Back in the day I suppose it was unusual to have more than one
output, so having finer grained control over screensavers and power
saving modes probably didn't make much sense.

Bryce

> > This raises a corner-case: If surface B becomes completely occluded so
> > that the compositor considers it is no longer inhibiting, what happens
> > with output 3? Should it powersave or not?
> > 
> > That is a strange use case but still possible. I'm not sure what should
> > happen. Should it behave like C had its own inhibitor, or should the
> > inheritance take also the effectiveness of the inhibitor on B?
> > 
> > I suppose you could pick either way.
> > 
> >>> +    </description>
> >>> +
> >>> +    <request name="destroy" type="destructor">
> >>> +      <description summary="destroy the idle inhibitor object">
> >>> +	This removes the inhibitor effect from the associated wl_surface.
> >>> +      </description>
> >>> +    </request>
> >>> +
> >>> +  </interface>
> >>> +</protocol>
> >>> -- 
> > 
> > Ok, this looks quite good, just a couple clarifications would be nice.
> > 
> > If you agree with my "inherited from parent" idea instead of from
> > top-level, you get:
> > Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > 
> > 
> > Thanks,
> > pq
> 
> Forgive me if I didn't quite grok prior conversations about this protocol.
> 
> Thank you,
> yong
> 
>