[PATCH weston 1/5] data-device: Implement DnD progress notification
Jonas Ådahl
jadahl at gmail.com
Tue Dec 15 18:21:25 PST 2015
On Tue, Dec 15, 2015 at 06:56:21PM +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.
s/drop_performed/dnd_drop_performed, and s/drag_finished/dnd_finished/.
>
> 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 <carlosg at gnome.org>
> Reviewed-by: Michael Catanzaro <mcatanzaro at igalia.com>
> ---
> 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)
naming nit: dnd_drag_destroy().
> {
> - 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);
Tab instead of spaces.
> +}
> +
> +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.
> + */
Tab instead of spaces.
> + 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..96a7b0d 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;
> + bool accepted;
>
> 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..2b55f7b 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);
First, this needs to be inside a version checks. It should only send
wl_data_source_cancelled if the offer version is >= 3, and it should
only send the dnd_finished if the source version is >= 3.
This made me think of an issue with the progress notification. How will
a failed operation be communicated? For example, lets pretend we have
the actions and the progress protocol in place.
source.offer("abc")
source.set_actions(move)
data_device.enter
offer.accept("abc")
offer.set_actions(move)
source.action(move)
offer.action(move)
data_device.drop
*fails to perform move*
offer.destroy
source.dnd_finished
*removes data*
Shouldn't we not send source.dnd_finished here? How would the offer
communicate that, while it accepted the action and mime type? Looking
for the .receive request I suspect is not enough, because it can still
fail to write the file due to the filesystem beeing full etc.
> +
> + 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;
s/0/false/.
> +
> 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_drop_performed(drag->base.data_source->resource);
Needs to be inside a data_source version >= 3 check.
> + }
>
> 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;
s/0/false/.
>
> 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);
The 1 here was incorrect, so lets fix this while at it. It should be
source->resource =
wl_resource_create(client, &wl_data_source_interface,
wl_resource_get_version(resource), id);
Seems weston_data_source_send_offer has the same bug when creating the
offer object, so it's probably needed to fix that place as well in order
to be able to do the version checking.
Jonas
> 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;
>
> --
> 2.5.0
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list