[PATCH weston v2] data-device: Implement DnD progress notification
Bryce Harrington
bryce at osg.samsung.com
Wed Nov 4 13:17:11 PST 2015
On Mon, Nov 02, 2015 at 09:15:15PM +0100, Carlos Garnacho wrote:
> Hi Bryce,
>
> On Mon, Nov 2, 2015 at 8:44 PM, Bryce Harrington <bryce at osg.samsung.com> wrote:
> > 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 <carlosg at gnome.org>
> >> ---
> >> 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.
>
> Indeed, changed locally.
>
> >
> >> 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?
>
> Unfortunately that's not completely the case... this patch depends on
> the first protocol patch I sent recently:
> http://lists.freedesktop.org/archives/wayland-devel/2015-October/025130.html
>
> The DnD action patches modify too close areas (eg. both features want
> to bump the data_device_manager version, extend wl_data_source, ...),
> so those only apply on top of these "progress notification" ones.
>
> Thanks for the review!
> Carlos
Ok sounds good, glad I asked. I see now the wl_data_offer version=3
requirement reflected in this patch.
(We've already bumped up the wayland-server dependency for weston to
1.9.90, so I'm assuming we're covered for build time dependency.)
Anyway, for the protocol changes, I'd like to see a couple more R-b's,
and/or at least one acked-by from one of the other wayland maintainers,
but in my looking over them I didn't spot anything wrong.
Bryce
More information about the wayland-devel
mailing list