On Jun 6, 2016, at 7:18 AM, Pekka Paalanen <ppaala...@gmail.com> wrote: > > On Fri, 3 Jun 2016 10:39:24 -0500 > Yong Bakos <j...@humanoriented.com> wrote: > >> Hi, >> >> On Jun 3, 2016, at 4:04 AM, Pekka Paalanen <ppaala...@gmail.com> wrote: >>> >>> On Fri, 3 Jun 2016 09:26:24 +0800 >>> Jonas Ådahl <jad...@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 <br...@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 <br...@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.paala...@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
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel