On Thu,  7 Apr 2016 16:44:21 -0700
Bryce Harrington <br...@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 <br...@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.

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

Attachment: pgpv7SiBCf6Co.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to