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

Jonas Ådahl jadahl at gmail.com
Wed Jun 8 02:51:30 UTC 2016


On Tue, Jun 07, 2016 at 07:10:25PM -0700, Bryce Harrington wrote:
> On Fri, Jun 03, 2016 at 09:26:24AM +0800, Jonas Ådahl 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,
> > 
> > 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. ?
> 
> How about just a simple:
> 
>   for each surface as appropriate.

I don't think it hurts with the extra clarity as one won't have to guess
what is appropriate or not.

> 
> > > +      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.
> > > +      </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.
> 
> Well, I suppose we should consider likely use cases here.
> 
> Is video-in-browser considered one of the big uses for subsurfaces?
> Here's a couple use cases off the top of my head:
> 
> a) I want to watch a video embedded in a web page.  I typically maximize
>    the video so I can see it well.  I would expect the screensaver to
>    not kick on for the duration of this video.  Once I'm done with it
>    and go back to regular web browser usage, I want my screensaver to
>    work normally.
> 
> b) I'm surfing the web reading articles on some annoying site that
>    auto-plays videos for me.  Maybe it's an advertisement, or maybe just
>    a live video news report version of the news article I'm reading.  I
>    just ignore it because I don't care.  When the video is done, it
>    automatically plays the next advertisement or video.  These videos
>    are annoying enough themselves, I definitely do not want these videos
>    causing my screensaver to not kick in.
> 
> So based on this I'm thinking subsurfaces probably should not be allowed
> to automatically override the parent.  But if the user takes some action
> that can be interpreted as "I am focusing on this subsurface" such as
> maximizing it or making it larger or something, then in that case I
> could see permitting it to inhibit.

I don't think we should start to adding things about focusing
subsurfaces. I also don't think we should really try to solve good vs
bad videos in a browser in the protocol; its the job of the browser to
do this for us and create the appropriate inhibit objects. We should
only care about allowing the web browser to create those objects to get
a well defined behavior.

> 
> Is a maximized subsurface still a subsurface or does it become a parent
> level surface?  If it does, then I think we can just let maximization be
> the indication to allow inhibition, and otherwise just not let
> subsurfaces interfere with their parent settings.  But if it doesn't,
> then perhaps we need a more sophisticated mechanism here.

There is no such thing as a maximized subsurface. We maximize or
fullscreen a shell surface, and the client updates its subsurface
accordingly. I don't think we should mix in shell surface behavior in
this protocol, we should just trust the web browser to protect us from
nasty web page behaviors, and let it inhibit things the way it sees fit.

The decision we need to make is: should we allow subsurfaces to inhibit
idle behavior, and if so, do they override their parent, or inherit it.
I suspect as long as we allow subsurfaces to affect the inhibit
behavior, we'll allow clients to construct their surface trees so that
they can get the inhibit behavior they want. Well, except if they don't
use subsurfaces for video content. If we care about that we'd need add
inhibit behaviour for sub regions of surfaces.


Jonas

> 
> Bryce
> 
> 
> > Jonas
> > 
> > > +    </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>
> > > -- 
> > > 1.9.1
> > > 
> > > _______________________________________________
> > > wayland-devel mailing list
> > > wayland-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list