[PATCH weston v3 6/8] ivi-shell: Support tracking active surfaces

Bryce Harrington bryce at osg.samsung.com
Sat Jun 4 02:24:40 UTC 2016


On Thu, May 26, 2016 at 06:02:00PM +0300, Pekka Paalanen wrote:
> On Thu,  7 Apr 2016 16:44:21 -0700
> Bryce Harrington <bryce at osg.samsung.com> wrote:
> 
> > Activity for ivi-shell follows either click or touch.  Only a single
> > surface can be active in the shell at a time.
> > 
> > Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> > ---
> >  Makefile.am           |  4 ++-
> >  configure.ac          |  2 ++
> >  ivi-shell/ivi-shell.c | 10 ++++++
> >  ivi-shell/ivi-shell.h |  2 ++
> >  src/compositor.c      | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  5 files changed, 107 insertions(+), 2 deletions(-)
> 
> Hi,
> 
> this looks like commit squashing accident, I would not expect to find
> the generic server implementation to be found in this patch. Please
> split.
> 
> > diff --git a/Makefile.am b/Makefile.am
> > index cbb3b57..aa3aa1b 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -126,7 +126,9 @@ nodist_weston_SOURCES =					\
> >  	protocol/scaler-protocol.c			\
> >  	protocol/scaler-server-protocol.h		\
> >  	protocol/linux-dmabuf-unstable-v1-protocol.c	\
> > -	protocol/linux-dmabuf-unstable-v1-server-protocol.h
> > +	protocol/linux-dmabuf-unstable-v1-server-protocol.h	\
> > +	protocol/idle-inhibit-unstable-v1-protocol.c	\
> > +	protocol/idle-inhibit-unstable-v1-server-protocol.h
> >  
> >  BUILT_SOURCES += $(nodist_weston_SOURCES)
> >  
> > diff --git a/configure.ac b/configure.ac
> > index bba8050..d4817a9 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -4,6 +4,8 @@ m4_define([weston_micro_version], [90])
> >  m4_define([weston_version],
> >            [weston_major_version.weston_minor_version.weston_micro_version])
> >  
> > +# Note: Inhibition patchset requires inhibition protocol in wayland-protocol
> > +
> >  AC_PREREQ([2.64])
> >  AC_INIT([weston],
> >          [weston_version],
> > diff --git a/ivi-shell/ivi-shell.c b/ivi-shell/ivi-shell.c
> > index 59f5656..9739014 100644
> > --- a/ivi-shell/ivi-shell.c
> > +++ b/ivi-shell/ivi-shell.c
> > @@ -434,11 +434,16 @@ static void
> >  click_to_activate_binding(struct weston_pointer *pointer, uint32_t time,
> >  			  uint32_t button, void *data)
> >  {
> > +	struct ivi_shell *shell = data;
> > +
> >  	if (pointer->grab != &pointer->default_grab)
> >  		return;
> >  	if (pointer->focus == NULL)
> >  		return;
> >  
> > +	if (shell->active_surface != NULL)
> > +		weston_surface_deactivate(shell->active_surface);
> > +
> >  	activate_binding(pointer->seat, pointer->focus);
> >  }
> >  
> > @@ -446,11 +451,16 @@ static void
> >  touch_to_activate_binding(struct weston_touch *touch, uint32_t time,
> >  			  void *data)
> >  {
> > +	struct ivi_shell *shell = data;
> > +
> >  	if (touch->grab != &touch->default_grab)
> >  		return;
> >  	if (touch->focus == NULL)
> >  		return;
> >  
> > +	if (shell->active_surface != NULL)
> > +		weston_surface_deactivate(shell->active_surface);
> > +
> >  	activate_binding(touch->seat, touch->focus);
> >  }
> >  
> > diff --git a/ivi-shell/ivi-shell.h b/ivi-shell/ivi-shell.h
> > index 9a05eb2..87bce3a 100644
> > --- a/ivi-shell/ivi-shell.h
> > +++ b/ivi-shell/ivi-shell.h
> > @@ -55,6 +55,8 @@ struct ivi_shell
> >  		struct wl_resource *binding;
> >  		struct wl_list surfaces;
> >  	} input_panel;
> > +
> > +	struct weston_surface *active_surface;
> 
> I suspect this probably should be per-seat... it was the same as
> keyboard focus.
> 
> >  };
> >  
> >  struct weston_view *
> > diff --git a/src/compositor.c b/src/compositor.c
> > index 8e01d38..c89e443 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -51,6 +51,8 @@
> >  #include <time.h>
> >  #include <errno.h>
> >  
> > +#include <idle-inhibit-unstable-v1-server-protocol.h>
> 
> Use "" as this header is local, not from an installed location.
> 
> > +
> >  #include "timeline.h"
> >  
> >  #include "compositor.h"
> > @@ -2412,7 +2414,7 @@ weston_output_inhibited_outputs(struct weston_compositor *compositor)
> >  			continue;
> >  
> >  		/* Does the view's surface inhibit this output? */
> > -		if (!view->surface->inhibit_screensaving)
> > +		if (!view->surface->inhibit_idling)
> 
> Wait, there is a typo in an earlier patch?
> Please squash this hunk in the appropriate patch, otherwise I think
> there is a commit that won't build.
> 
> >  			continue;
> >  
> >  		inhibited_outputs_mask |= view->output_mask;
> > @@ -4631,6 +4633,89 @@ compositor_bind(struct wl_client *client,
> >  				       compositor, NULL);
> >  }
> >  
> > +struct weston_idle_inhibitor {
> > +	struct weston_surface *surface;
> > +};
> > +
> > +static void
> > +destroy_idle_inhibitor(struct wl_resource *resource)
> > +{
> > +	struct weston_idle_inhibitor *inhibitor = wl_resource_get_user_data(resource);
> > +
> > +	inhibitor->surface = NULL;
> > +	free(inhibitor);
> > +}
> > +
> > +static void
> > +idle_inhibitor_destroy(struct wl_client *client, struct wl_resource *resource)
> > +{
> > +	struct weston_idle_inhibitor *inhibitor = wl_resource_get_user_data(resource);
> > +
> > +	assert(inhibitor);
> > +
> > +	inhibitor->surface->inhibit_idling = false;
> 
> What if the surface had several inhibitor objects? Here destroying just
> the first one would cause the inhibit to go away, while other inhibitor
> objects remained.

Why would a surface have several inhibitor objects?

> I think you need to maintain a list or a refcount instead.
> 
> The destruction is missing.
> 
> > +}
> > +
> > +static const struct zwp_idle_inhibitor_v1_interface idle_inhibitor_interface = {
> > +	idle_inhibitor_destroy
> > +};
> > +
> > +static void
> > +idle_inhibit_manager_destroy(struct wl_client *client, struct wl_resource *resource)
> > +{
> 
> Missing the destruction.
> 
> > +}
> > +
> > +static void
> > +idle_inhibit_manager_create_inhibitor(struct wl_client *client, struct wl_resource *resource,
> > +				      uint32_t id, struct wl_resource *surface_resource)
> > +{
> > +	struct weston_surface *surface = wl_resource_get_user_data(surface_resource);
> > +	struct weston_idle_inhibitor *inhibitor;
> > +	struct wl_resource *cr;
> > +
> > +	cr = wl_resource_create(client, &zwp_idle_inhibitor_v1_interface,
> > +				wl_resource_get_version(resource), id);
> > +	if (cr == NULL) {
> > +		wl_client_post_no_memory(client);
> > +		return;
> > +	}
> > +
> > +	inhibitor = zalloc(sizeof *inhibitor);
> > +	if (inhibitor == NULL) {
> > +		wl_client_post_no_memory(client);
> > +		return;
> > +	}
> > +
> > +	inhibitor->surface = surface;
> > +	inhibitor->surface->inhibit_idling = true;
> > +
> > +	wl_resource_set_implementation(cr, &idle_inhibitor_interface,
> > +				       inhibitor, destroy_idle_inhibitor);
> > +}
> > +
> > +static const struct zwp_idle_inhibit_manager_v1_interface idle_inhibit_manager_interface = {
> > +	idle_inhibit_manager_create_inhibitor,
> > +	idle_inhibit_manager_destroy
> 
> Odd, this should cause a build failure as the XML spec does not have a
> destructor. It seems this patch was ahead of the spec. :-)
> 
> > +};
> > +
> > +static void
> > +bind_idle_inhibit_manager(struct wl_client *client,
> > +			  void *data, uint32_t version, uint32_t id)
> > +{
> > +	struct wl_resource *resource;
> > +
> > +	resource = wl_resource_create(client, &zwp_idle_inhibit_manager_v1_interface,
> > +				      version, id);
> > +	if (resource == NULL) {
> > +		wl_client_post_no_memory(client);
> > +		return;
> > +	}
> > +
> > +	wl_resource_set_implementation(resource, &idle_inhibit_manager_interface,
> > +				       NULL, NULL);
> > +}
> > +
> > +
> >  WL_EXPORT int
> >  weston_environment_get_fd(const char *env)
> >  {
> > @@ -4723,6 +4808,10 @@ weston_compositor_create(struct wl_display *display, void *user_data)
> >  			      ec, bind_presentation))
> >  		goto fail;
> >  
> > +	if (!wl_global_create(ec->wl_display, &zwp_idle_inhibit_manager_v1_interface, 1,
> > +			      ec, bind_idle_inhibit_manager))
> > +		goto fail;
> > +
> >  	wl_list_init(&ec->view_list);
> >  	wl_list_init(&ec->plane_list);
> >  	wl_list_init(&ec->layer_list);
> 
> 
> Thanks,
> pq

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
> 
> iQIVAwUBV0cP6CNf5bQRqqqnAQixow/9FlnZFHRfdA3/w55jMr8tvn0vTDPNcObJ
> 0IOJGXNuc1LqtlnZvJfa/pMCqXhXW3mfaj3EOhNyAKMfBpEcnEzoAn5OlqRJDJpX
> kmmQUhXiqR4lHdjWCEkjGtma8TjLDX4z5/rEdVuM48+HhIFqs0CbbTiEGCJxeL4p
> cc/BkWoW+WGqvgiA3aWVH67xgl9aOkNAR+DHDJowoHtF98kCEcPu1zq3dtpwZ1O4
> D6IHQ3/K3SQQaiWCpTLMGQ3RVVKv2mbD4yUMr9EcgjBA3YFyvBzOpUSFikKlBXqI
> 4h+JpD2ScMFr4qOjpxMI28844dtQMuzLGdmPRUAmU64EW5tqt8Bd5TcJ2NgvIFS3
> lYSbh7fkPfMVAQuoEGOr2jIEUndDRQabUzzppkc8395olGMen2vchnRApLD9ZZhk
> 1BVe54QVQ8RaRKJLrAadWCdFtC1w0OWsGFKRLT6//kIJfI8IXlL5TrMnp65OHfdE
> efQr6Ph2Vy572WPE2CQlsE8XUYVRGjKJf4qfI7hI0YtfTCfdBDxeem9e+QYwnPPX
> sQN0GCn2lJh6C8omOXDZNXxeZwuj/9jR07WEOk3vDhL6JRVbdBHfZ4TgoVv0eZUu
> kqVTpmqiEd3mB3hAIDv0Y5R/X6ePVFuO8Sn/CARiue3DY327DTDa1XpLjjI0Slj2
> TU2n89cTofo=
> =0sob
> -----END PGP SIGNATURE-----



More information about the wayland-devel mailing list