[PATCH wayland-protocols v3] Add screensaver idle inhibitor protocol
Pekka Paalanen
ppaalanen at gmail.com
Wed Jun 8 09:02:14 UTC 2016
On Wed, 8 Jun 2016 10:51:30 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:
> 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.
Agreed.
> >
> > 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.
Also agreed.
Furthermore, one cannot use any shell concepts in defining the
idle-inhibit extension, because idle-inhibit is a sibling, not a child
extension to shells. If you need to use those concepts, then you need
define them or make this a shell-extension-specific extension.
> 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.
I think sub-regions go too far.
Sub-surfaces however would be necessary to take into account somehow,
because you can construct a window from several non-overlapping
sub-surfaces, and hang parts of the window on different outputs.
Therefore I think inheritance is necessary. Overriding is possible only
in the direction of adding an inhibitor.
OTOH should we deny setting inhibitors on sub-surfaces? IOW, should we
deny based on the surface role? If yes, does a surface have to have a
role before creating an inhibitor? What if it doesn't and you make it a
sub-surface afterwards?
What if someone adds new surface roles in the future? A different kind
of sub-surface maybe because they hate the current interface?
I think the simplest solution here is:
- inherit inhibition
- do not forbid inhibitors based on surface role
However, should inheritance be limited to sub-surfaces only, that is,
based on role? What about parent-child relationships established in
e.g. xdg_shell? Ok, so inheriting becomes a bit hard too.
Rethinking this, should we actually not spec anything about inheritance
in this extension, and instead say it in sub-surface spec?
In more generic terms, where does extension interoperability
documentation belong? I don't think there is a single answer to that,
we have to go case by case with where it makes most sense.
To explain that inhibition could be inherited, I propose the
idle-inhibit spec specifies inheritance for sub-surfaces only. Then if
e.g. xdg_shell or something would like to do the same, it can refer to
the sub-surface case as a precedent.
Yes, this is overthinking things.
I stick to my simplest suggestion:
- inherit inhibition for sub-surfaces (by effectiveness, not by
pretending the child has its own copy of the inhibitor to be
evaluated separately)
- do not forbid inhibitors based on surface role
Otherwise we could discuss this to the death. Would this be an
acceptable compromise?
Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160608/3ec0a154/attachment.sig>
More information about the wayland-devel
mailing list