[PATCH weston v5 5/7] libweston-desktop: Add listener and API to drop the idle inhibitor

[Pekka Paalanen]

On Tue, 30 Aug 2016 12:46:16 +0200
Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:

> On 30/08/2016 03:34, Bryce Harrington wrote:
> > Listen for the drop_idle_inhibitor signal from libweston, and propagate
> > the call to a corresponding libweston-desktop API.
> >
> > Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>  
> 
> In the current form, this patch is useless. Details inside.
> 
> 
> > ---
> >  libweston-desktop/internal.h          |  3 +++
> >  libweston-desktop/libweston-desktop.c |  8 ++++++++
> >  libweston-desktop/libweston-desktop.h |  2 ++
> >  libweston-desktop/surface.c           | 20 ++++++++++++++++++++
> >  4 files changed, 33 insertions(+)
> >

> >  static void
> > +weston_desktop_surface_drop_idle_inhibitor(struct wl_listener *listener,
> > +					   void *data)
> > +{
> > +	struct weston_desktop_surface *surface =
> > +		wl_container_of(listener, surface, surface_drop_idle_inhibitor_listener);
> > +	struct weston_desktop *desktop = surface->desktop;
> > +
> > +	printf("weston_desktop_surface_drop_idle_inhibitor\n");
> > +	weston_desktop_api_surface_drop_idle_inhibitor(desktop, surface);  
> 
> This …
> 
> > +	// TODO: Need to call shell.c's desktop_surface_drop_idle_inhibitor
> > +	//shell_desktop_api.surface_drop_idle_inhibitor(surface, NULL /*data?*/);  
> 
> … is exactly that, but safer and fitting the existing wrappers design.
> 
> This code does basically nothing that the compositor couldn’t do 
> directly by listening to the signal. Or rather, it makes the compositor 
> crash.
> 
> libweston-desktop uses "weston_desktop_surface" for two kind of 
> surfaces: toplevel surfaces, and non-toplevel surfaces.
> Here, you expose surfaces that the shell doesn’t know about, so with no 
> associated "shell_surface".
> 
> Pekka, Jonas, how will we interface idle_inhibitor with wl_shell and 
> xdg_shell? Do we allow inhibitors on popup/transient?

Hi,

idle-inhibit-unstable-v1.xml should have the answer to all those
questions.

It lists no restrictions and applies on wl_surface, so it is allowed
for any and all kinds of wl_surfaces.

> If so, libweston-desktop will have to somehow proxy these in a nice way, 
> as well as inheritance and stuff.
> If not, the shell will listen for the destroy signal, and 
> libweston-desktop need a way to hook on inhibitor creation, so it can 
> error the client.

There should be no reason to require any such proxying for surface
types the shells have not cared about before, or even at all.
Inhibition effect comes and goes per output (as computed from the
per-surface inhibitors) and that behaviour is common to all shells (not
only desktop). What is the reason why the shell is interested in
individual inhibiting surfaces?


Thanks,
pq

PS. Impressive testing as explained in the cover letter, very good!


> Xwayland surfaces are safe, I guess, so override-redirect surfaces don’t 
> need a special case here.
> 
> Cheers,
> 
> 
> > +}
> > +
> > +static void
> >  weston_desktop_surface_committed(struct weston_surface *wsurface,
> >  				 int32_t sx, int32_t sy)
> >  {
> > @@ -277,6 +293,10 @@ weston_desktop_surface_create(struct weston_desktop *desktop,
> >  		weston_desktop_surface_surface_destroyed;
> >  	wl_signal_add(&surface->surface->destroy_signal,
> >  		      &surface->surface_destroy_listener);
> > +	surface->surface_drop_idle_inhibitor_listener.notify =
> > +		weston_desktop_surface_drop_idle_inhibitor;
> > +	wl_signal_add(&surface->surface->drop_idle_inhibitor_signal,
> > +		      &surface->surface_drop_idle_inhibitor_listener);
> >
> >  	wl_list_init(&surface->client_link);
> >  	wl_list_init(&surface->resource_list);
> >  
> 
> 

-------------- 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/20160831/32500245/attachment.sig>