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 <[email protected]> > 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 <[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. > > 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 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
