On Wed, 8 Jun 2016 10:51:30 +0800 Jonas Ådahl <jad...@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 <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, > > > > > > 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. ? > > > > 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
pgpbYP3cEK3Is.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
