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? >>> + <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? >>> + </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. 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. > > 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. Thank you, yong _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
