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

Yong Bakos junk at humanoriented.com
Mon Jun 6 15:08:12 UTC 2016


On Jun 6, 2016, at 7:18 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 
> On Fri, 3 Jun 2016 10:39:24 -0500
> Yong Bakos <junk at humanoriented.com> wrote:
> 
>> Hi,
>> 
>> On Jun 3, 2016, at 4:04 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>>> 
>>> On Fri, 3 Jun 2016 09:26:24 +0800
>>> Jonas Ådahl <jadahl at gmail.com> wrote:
>>> 
>>>> On Thu, Jun 02, 2016 at 02:24:20PM -0700, Bryce Harrington wrote:
>>>>> This interface allows disabling of screensaver/screenblanking on a
>>>>> per-surface basis.  As long as the surface remains visible and
>>>>> non-occluded it blocks the screensaver, etc. from activating on the
>>>>> output(s) that the surface is visible on.
>>>>> 
>>>>> To uninhibit, simply destroy the inhibitor object.
>>>>> 
>>>>> Signed-off-by: Bryce Harrington <bryce at bryceharrington.org>
>>>>> ---
>>>>> 
>>>>> v2:
>>>>> + Rename protocol to idle-inhibit
>>>>> v3:
>>>>> + Added a destructor for the idle manager
>>>>> + Changed sub-surface behavior to inherit idle from parent surface
>>>>> + Various wording changes suggested by pq
>>>>> 
>>> 
>>> Hi,
>>> 
>>> you posted the last one marked as v3, so this would have been v4.
>>> 
>>>> Hi,
>>>> 
>>>> I think this looks sane, I only have a couple of questions that might
>>>> need clarification.
>>>> 
>>>>> 
>>>>> Makefile.am                                        |  1 +
>>>>> unstable/idle-inhibit/README                       |  4 ++
>>>>> unstable/idle-inhibit/idle-inhibit-unstable-v1.xml | 84 ++++++++++++++++++++++
>>>>> 3 files changed, 89 insertions(+)
>>>>> create mode 100644 unstable/idle-inhibit/README
>>>>> create mode 100644 unstable/idle-inhibit/idle-inhibit-unstable-v1.xml
>>>>> 
>>>>> diff --git a/Makefile.am b/Makefile.am
>>>>> index 71d2632..de691db 100644
>>>>> --- a/Makefile.am
>>>>> +++ b/Makefile.am
>>>>> @@ -8,6 +8,7 @@ unstable_protocols =								\
>>>>> 	unstable/relative-pointer/relative-pointer-unstable-v1.xml		\
>>>>> 	unstable/pointer-constraints/pointer-constraints-unstable-v1.xml	\
>>>>> 	unstable/tablet/tablet-unstable-v1.xml			                \
>>>>> +	unstable/idle-inhibit/idle-inhibit-unstable-v1.xml			\
>>>>> 	$(NULL)
>>>>> 
>>>>> stable_protocols =								\
>>>>> diff --git a/unstable/idle-inhibit/README b/unstable/idle-inhibit/README
>>>>> new file mode 100644
>>>>> index 0000000..396e871
>>>>> --- /dev/null
>>>>> +++ b/unstable/idle-inhibit/README
>>>>> @@ -0,0 +1,4 @@
>>>>> +Screensaver inhibition protocol
>>>>> +
>>>>> +Maintainers:
>>>>> +Bryce Harrington <bryce at osg.samsung.com>
>>>>> diff --git a/unstable/idle-inhibit/idle-inhibit-unstable-v1.xml b/unstable/idle-inhibit/idle-inhibit-unstable-v1.xml
>>>>> new file mode 100644
>>>>> index 0000000..af3a911
>>>>> --- /dev/null
>>>>> +++ b/unstable/idle-inhibit/idle-inhibit-unstable-v1.xml
>>>>> @@ -0,0 +1,84 @@
>>>>> +<?xml version="1.0" encoding="UTF-8"?>
>>>>> +<protocol name="idle_inhibit_unstable_v1">
>>>>> +
>>>>> +  <copyright>
>>>>> +    Copyright © 2015 Samsung Electronics Co., Ltd
>>>>> +
>>>>> +    Permission is hereby granted, free of charge, to any person obtaining a
>>>>> +    copy of this software and associated documentation files (the "Software"),
>>>>> +    to deal in the Software without restriction, including without limitation
>>>>> +    the rights to use, copy, modify, merge, publish, distribute, sublicense,
>>>>> +    and/or sell copies of the Software, and to permit persons to whom the
>>>>> +    Software is furnished to do so, subject to the following conditions:
>>>>> +
>>>>> +    The above copyright notice and this permission notice (including the next
>>>>> +    paragraph) shall be included in all copies or substantial portions of the
>>>>> +    Software.
>>>>> +
>>>>> +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>>>>> +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>>>> +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>>>>> +    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>>>>> +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>>>>> +    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>>>> +    DEALINGS IN THE SOFTWARE.
>>>>> +  </copyright>
>>>>> +
>>>>> +  <interface name="zwp_idle_inhibit_manager_v1" version="1">
>>>>> +    <description summary="control behavior when display idles">
>>>>> +      This interface permits inhibiting the idle behavior such as screen
>>>>> +      blanking, locking, and screensaving.  The client binds the idle manager
>>>>> +      globally, then creates idle-inhibitor objects for each surface.
>>>> 
>>>> + for each surface that should inhibit the idle behavior. ?
>>> 
>>> Yeah, makes sense.
>>> 
>>>>> +
>>>>> +      Warning! The protocol described in this file is experimental and
>>>>> +      backward incompatible changes may be made. Backward compatible changes
>>>>> +      may be added together with the corresponding interface version bump.
>>>>> +      Backward incompatible changes are done by bumping the version number in
>>>>> +      the protocol and interface names and resetting the interface version.
>>>>> +      Once the protocol is to be declared stable, the 'z' prefix and the
>>>>> +      version number in the protocol and interface names are removed and the
>>>>> +      interface version number is reset.
>>>>> +    </description>
>>>>> +
>> 
>> Shouldn't there be a "create" request so clients can obtain the manager object?
> 
> Hi,
> 
> that request is wl_registry.bind, as for all globals.
> 
>>>>> +    <request name="destroy" type="destructor">
>>>>> +      <description summary="destroy the idle inhibitor object">
>> 
>> destroy the idle inhibit manager
>> 
>> 
>>>>> +	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?
> 
> 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.
> 
> Yes, they must be associated to surfaces.
> 
>>>>> +
>>>>> +  </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.'
> 
> An ordinary client must not be able to do that.
> 
>> 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.
> 
> If you cannot see the widget anyway, it must not be able to affect
> screen saving. Therefore by definition, surfaceless clients must not be
> able to inhibit.
> 
>> Furthermore, I don't believe that inhibition should be coupled to outputs.
>> See below.
> 
> It should work per-output. If there is important info on just one
> output, why should the other outputs also stay on for no good reason?

Just because I'm looking at info on one monitor doesn't mean I want my other
monitors to go to sleep. I can imagine this being specified via a shell's
screensaver/power savings config panel, with a checkbox enabling "idle per screen" or "idle
all screens in unison." So I think this is a moot point. However, I
don't believe that just because a surface is occluded, that it's ability
to inhibit idling should be suppressed.


>>>>> +
>>>>> +      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.
> 
> That is explicitly not what we want to enable.
> 
> What is this "caffeine" you talk about? I've never heard of it. Maybe I
> never had to fight against screen saving.

Here's a screenshot of the Gnome-flavored implementation of the widget:
http://tinyurl.com/caffeine-widget-2

OSX and others have something similar. This program visually resides in the
menubar/taskbar of the shell. It has two states, on and off. On (caffeinated)
overrides the configured screensaver/sleep idle timeout, preventing /all/
monitors from idling. In the off (un-caffeinated) state, the system behaves
according to configuration.

Use case: I've configured my system to screensave at 5 minutes and put the
monitors to sleep at 10 minutes. But sometimes, I want to override this
without going in and changing my config, only to have to change it back
later. I run the caffeine widget, turn it on, and all my monitors remain on
despite me not interacting with my machine. This behavior remains valid
even if I am running a program fullscreen, which occludes the visibility
of the widget.

It sounds like, from what Bryce had wrote in a separate reply, such a
widget would use a different API and not this idle inhibitor protocol. I
might be misunderstanding his point.


> 
>>> 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.
> 
> It seems like your mindset is tuned to fighting against bad
> screensaving policies in a compositor, while Bryce's goal with the
> extension is to allow implementing proper and good screensaving policies
> *in the first place*, so that no fighting is needed.
> 
> We want the compositor to behave correctly, not offer tools to override
> the compositor behaviour. Bugs should be fixed at their source, not
> worked around at every other place.

I hear you. Not fighting, per se, but rather 'temporarily override'.
I'm just stating that an application should be able to inhibit idling:

a) On all outputs
b) Whether it is visible or not

Per the use case I described above. Whether it uses this inhibitor protocol
or some other API is fine. But if such a use case /should/ use the inhibitor
protocol, then the per-output constraint and visibility constraint in the
protocol would make this functionality 'impossible'.

yong


> Thanks,
> pq

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160606/ec19d37c/attachment-0001.sig>


More information about the wayland-devel mailing list