[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