On Fri, Oct 30, 2015 at 10:04:49PM +0100, Carlos Garnacho wrote:
> Weston now sends wl_data_source.drop_performed and .drag_finished in
> order to notify about the different phases of DnD.
> 
> wl_data_source.cancelled is also used as mentioned in the docs, being
> emitted also on DnD when the operation is meant to fail (eg. source
> and dest didn't agree on a mimetype).
> 
> The dnd demo is also fixed so the struct dnd_drag isn't leaked.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=91943
> https://bugs.freedesktop.org/show_bug.cgi?id=91944
> 
> Changes since v1:
>   - Updated to protocol v2.
> 
> Signed-off-by: Carlos Garnacho <[email protected]>
> ---
>  clients/dnd.c     | 39 +++++++++++++++++++++++++++++++--------
>  clients/window.c  |  2 +-
>  src/compositor.h  |  2 ++
>  src/data-device.c | 37 +++++++++++++++++++++++++++++++------
>  4 files changed, 65 insertions(+), 15 deletions(-)
> 
> diff --git a/clients/dnd.c b/clients/dnd.c
> index e6a0c39..6c2ed57 100644
> --- a/clients/dnd.c
> +++ b/clients/dnd.c
> @@ -318,14 +318,8 @@ data_source_send(void *data, struct wl_data_source 
> *source,
>  }
>  
>  static void
> -data_source_cancelled(void *data, struct wl_data_source *source)
> +destroy_dnd_drag(struct dnd_drag *dnd_drag)
>  {
> -     struct dnd_drag *dnd_drag = data;
> -
> -     /* The 'cancelled' event means that the source is no longer in
> -      * use by the drag (or current selection).  We need to clean
> -      * up the drag object created and the local state. */
> -
>       wl_data_source_destroy(dnd_drag->data_source);
>  
>       /* Destroy the item that has been dragged out */
> @@ -339,10 +333,39 @@ data_source_cancelled(void *data, struct wl_data_source 
> *source)
>       free(dnd_drag);
>  }
>  
> +static void
> +data_source_cancelled(void *data, struct wl_data_source *source)
> +{
> +     struct dnd_drag *dnd_drag = data;
> +
> +     /* The 'cancelled' event means that the source is no longer in
> +      * use by the drag (or current selection).  We need to clean
> +      * up the drag object created and the local state. */
> +        destroy_dnd_drag(dnd_drag);
> +}
> +
> +static void
> +data_source_drop_performed(void *data, struct wl_data_source *source)
> +{
> +}
> +
> +static void
> +data_source_drag_finished(void *data, struct wl_data_source *source)
> +{
> +     struct dnd_drag *dnd_drag = data;
> +
> +        /* The operation is already finished, we can destroy all
> +         * related data.
> +         */
> +     destroy_dnd_drag(dnd_drag);
> +}
> +
>  static const struct wl_data_source_listener data_source_listener = {
>       data_source_target,
>       data_source_send,
> -     data_source_cancelled
> +     data_source_cancelled,
> +     data_source_drop_performed,
> +     data_source_drag_finished,
>  };
>  
>  static cairo_surface_t *
> diff --git a/clients/window.c b/clients/window.c
> index 47628de..24aa517 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -5376,7 +5376,7 @@ registry_handle_global(void *data, struct wl_registry 
> *registry, uint32_t id,
>               d->shm = wl_registry_bind(registry, id, &wl_shm_interface, 1);
>               wl_shm_add_listener(d->shm, &shm_listener, d);
>       } else if (strcmp(interface, "wl_data_device_manager") == 0) {
> -             d->data_device_manager_version = MIN(version, 2);
> +             d->data_device_manager_version = MIN(version, 3);
>               d->data_device_manager =
>                       wl_registry_bind(registry, id,
>                                        &wl_data_device_manager_interface,
> diff --git a/src/compositor.h b/src/compositor.h
> index 4443c72..18266ef 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -304,6 +304,8 @@ struct weston_data_source {
>       struct wl_resource *resource;
>       struct wl_signal destroy_signal;
>       struct wl_array mime_types;
> +     struct weston_data_offer *offer;
> +     int accepted;

accepted is just being used as a flag here, so should be a bool.
  
>       void (*accept)(struct weston_data_source *source,
>                      uint32_t serial, const char *mime_type);
> diff --git a/src/data-device.c b/src/data-device.c
> index 1612091..4be392d 100644
> --- a/src/data-device.c
> +++ b/src/data-device.c
> @@ -62,12 +62,18 @@ data_offer_accept(struct wl_client *client, struct 
> wl_resource *resource,
>  {
>       struct weston_data_offer *offer = wl_resource_get_user_data(resource);
>  
> +     /* Protect against untimely calls from older data offers */
> +     if (!offer->source || offer != offer->source->offer)
> +             return;
> +
>       /* FIXME: Check that client is currently focused by the input
>        * device that is currently dragging this data source.  Should
>        * this be a wl_data_device request? */
>  
> -     if (offer->source)
> +     if (offer->source) {
>               offer->source->accept(offer->source, serial, mime_type);
> +             offer->source->accepted = mime_type != NULL;
> +     }
>  }
>  
>  static void
> @@ -76,7 +82,7 @@ data_offer_receive(struct wl_client *client, struct 
> wl_resource *resource,
>  {
>       struct weston_data_offer *offer = wl_resource_get_user_data(resource);
>  
> -     if (offer->source)
> +     if (offer->source && offer == offer->source->offer)
>               offer->source->send(offer->source, mime_type, fd);
>       else
>               close(fd);
> @@ -99,8 +105,18 @@ destroy_data_offer(struct wl_resource *resource)
>  {
>       struct weston_data_offer *offer = wl_resource_get_user_data(resource);
>  
> -     if (offer->source)
> +     if (offer->source) {
> +             if (offer->source->offer == offer) {
> +                     if (offer->source->accepted)
> +                             
> wl_data_source_send_dnd_finished(offer->source->resource);
> +                     else
> +                             
> wl_data_source_send_cancelled(offer->source->resource);
> +
> +                     offer->source->offer = NULL;
> +             }
> +
>               wl_list_remove(&offer->source_destroy_listener.link);
> +     }
>       free(offer);
>  }
>  
> @@ -147,6 +163,9 @@ weston_data_source_send_offer(struct weston_data_source 
> *source,
>       wl_array_for_each(p, &source->mime_types)
>               wl_data_offer_send_offer(offer->resource, *p);
>  
> +     source->offer = offer;
> +     source->accepted = 0;
> +
>       return offer->resource;
>  }
>  
> @@ -301,6 +320,7 @@ weston_drag_set_focus(struct weston_drag *drag,
>       serial = wl_display_next_serial(display);
>  
>       if (drag->data_source) {
> +             drag->data_source->accepted = 0;
>               offer = weston_data_source_send_offer(drag->data_source,
>                                                     resource);
>               if (offer == NULL)
> @@ -398,9 +418,12 @@ drag_grab_button(struct weston_pointer_grab *grab,
>       enum wl_pointer_button_state state = state_w;
>  
>       if (drag->base.focus_resource &&
> +         drag->base.data_source &&
>           pointer->grab_button == button &&
> -         state == WL_POINTER_BUTTON_STATE_RELEASED)
> +         state == WL_POINTER_BUTTON_STATE_RELEASED) {
>               wl_data_device_send_drop(drag->base.focus_resource);
> +             
> wl_data_source_send_dnd_performed(drag->base.data_source->resource);
> +     }
>  
>       if (pointer->button_count == 0 &&
>           state == WL_POINTER_BUTTON_STATE_RELEASED) {
> @@ -878,11 +901,13 @@ create_data_source(struct wl_client *client,
>       source->accept = client_source_accept;
>       source->send = client_source_send;
>       source->cancel = client_source_cancel;
> +     source->offer = NULL;
> +     source->accepted = 0;
>  
>       wl_array_init(&source->mime_types);
>  
>       source->resource =
> -             wl_resource_create(client, &wl_data_source_interface, 1, id);
> +             wl_resource_create(client, &wl_data_source_interface, 3, id);
>       wl_resource_set_implementation(source->resource, &data_source_interface,
>                                      source, destroy_data_source);
>  }
> @@ -958,7 +983,7 @@ WL_EXPORT int
>  wl_data_device_manager_init(struct wl_display *display)
>  {
>       if (wl_global_create(display,
> -                          &wl_data_device_manager_interface, 2,
> +                          &wl_data_device_manager_interface, 3,
>                            NULL, bind_manager) == NULL)
>               return -1;

Other than the boolean change, this lgtm.  Do I understand correctly
that this does not depend on any of the pending protocol changes you've
recently submitted, and can be landed independently of them?

Thanks,
Bryce

>  
> -- 
> 2.5.0
> 
> _______________________________________________
> wayland-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to