On Fri, 3 Jun 2016 09:26:24 +0800 Jonas Ådahl <[email protected]> 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 <[email protected]> > > --- > > > > 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 <[email protected]> > > 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> > > + > > + <request name="destroy" type="destructor"> > > + <description summary="destroy the idle inhibitor object"> > > + This destroys the inhibit manager. Good addition. Should probably say how destroying the manager affects the inhibitors created from it (no effect at all)? > > + </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> > > + > > + </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. > > + > > + 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. 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 <[email protected]> Thanks, pq
pgpD72VLSV1IO.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
