[PATCH weston 2/5] data-device: Implement DnD actions
Jonas Ådahl
jadahl at gmail.com
Sat Jan 16 07:47:24 PST 2016
On Fri, Jan 15, 2016 at 09:14:24PM +0100, Carlos Garnacho wrote:
> The policy in weston in order to determine the chosen DnD action is
> deliberately simple, and is probably the minimals that any compositor
> should be doing here.
>
> Besides honoring the set_actions requests on both wl_data_source and
> wl_data_offer, weston now will emit the newly added "action" events
> notifying both source and dest of the chosen action.
>
> The "dnd" client has been updated too (although minimally), so it
> notifies the compositor of a "move" action on both sides.
>
> Changes since v7:
> - Fixes spotted during review. Add client-side version checks.
> Implement .action emission as specified in protocol patch v11.
>
> Changes since v6:
> - Emit errors as defined in DnD actions patch v10.
>
> Changes since v5:
> - Use enum types and values for not-a-bitfield stored values.
> handle errors when finding unexpected dnd_actions values.
>
> Changes since v4:
> - Added compositor-side version checks. Spaces vs tabs fixes.
> Fixed resource versioning. Initialized new weston_data_source/offer
> fields.
>
> Changes since v3:
> - Put data_source.action to use in the dnd client, now updates
> the dnd surface like data_source.target events do.
>
> Changes since v2:
> - Split from DnD progress notification changes.
>
> Changes since v1:
> - Updated to v2 of DnD actions protocol changes, implement
> wl_data_offer.source_actions.
> - Fixed coding style issues.
>
> Signed-off-by: Carlos Garnacho <carlosg at gnome.org>
> Reviewed-by: Michael Catanzaro <mcatanzaro at igalia.com>
Except for couple of minor issues and a couple of nits, this patch is
now Reviewed-by: Jonas Ådahl <jadahl at gmail.com>.
> ---
> clients/dnd.c | 36 ++++++++++--
> clients/window.c | 34 +++++++++++
> clients/window.h | 3 +
> src/compositor.h | 6 ++
> src/data-device.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 5 files changed, 242 insertions(+), 9 deletions(-)
>
> diff --git a/clients/dnd.c b/clients/dnd.c
> index 974429b..d32655d 100644
> --- a/clients/dnd.c
> +++ b/clients/dnd.c
> @@ -72,6 +72,7 @@ struct dnd_drag {
> struct item *item;
> int x_offset, y_offset;
> int width, height;
> + uint32_t dnd_action;
> const char *mime_type;
>
> struct wl_surface *drag_surface;
> @@ -266,16 +267,13 @@ dnd_get_item(struct dnd *dnd, int32_t x, int32_t y)
> }
>
> static void
> -data_source_target(void *data,
> - struct wl_data_source *source, const char *mime_type)
> +dnd_drag_update_surface(struct dnd_drag *dnd_drag)
> {
> - struct dnd_drag *dnd_drag = data;
> struct dnd *dnd = dnd_drag->dnd;
> cairo_surface_t *surface;
> struct wl_buffer *buffer;
>
> - dnd_drag->mime_type = mime_type;
> - if (mime_type)
> + if (dnd_drag->mime_type && dnd_drag->dnd_action)
> surface = dnd_drag->opaque;
> else
> surface = dnd_drag->translucent;
> @@ -288,6 +286,16 @@ data_source_target(void *data,
> }
>
> static void
> +data_source_target(void *data,
> + struct wl_data_source *source, const char *mime_type)
> +{
> + struct dnd_drag *dnd_drag = data;
> +
> + dnd_drag->mime_type = mime_type;
> + dnd_drag_update_surface(dnd_drag);
> +}
> +
> +static void
> data_source_send(void *data, struct wl_data_source *source,
> const char *mime_type, int32_t fd)
> {
> @@ -360,12 +368,22 @@ data_source_dnd_finished(void *data, struct wl_data_source *source)
> dnd_drag_destroy(dnd_drag);
> }
>
> +static void
> +data_source_action(void *data, struct wl_data_source *source, uint32_t dnd_action)
> +{
> + struct dnd_drag *dnd_drag = data;
> +
> + dnd_drag->dnd_action = dnd_action;
> + dnd_drag_update_surface(dnd_drag);
> +}
> +
> static const struct wl_data_source_listener data_source_listener = {
> data_source_target,
> data_source_send,
> data_source_cancelled,
> data_source_dnd_drop_performed,
> data_source_dnd_finished,
> + data_source_action,
> };
>
> static cairo_surface_t *
> @@ -428,6 +446,8 @@ create_drag_source(struct dnd *dnd,
> dnd_drag->item = item;
> dnd_drag->x_offset = x - item->x;
> dnd_drag->y_offset = y - item->y;
> + dnd_drag->dnd_action = WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE;
> + dnd_drag->mime_type = NULL;
>
> for (i = 0; i < ARRAY_LENGTH(dnd->items); i++) {
> if (item == dnd->items[i]){
> @@ -456,6 +476,12 @@ create_drag_source(struct dnd *dnd,
> text_mime_type);
> }
>
> + if (display_get_data_device_manager_version(display) >=
> + WL_DATA_SOURCE_SET_ACTIONS_SINCE_VERSION) {
> + wl_data_source_set_actions(dnd_drag->data_source,
> + WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE);
> + }
> +
> wl_data_device_start_drag(input_get_data_device(input),
> dnd_drag->data_source,
> window_get_wl_surface(dnd->window),
> diff --git a/clients/window.c b/clients/window.c
> index 2baf9ea..3e5885e 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -3323,6 +3323,8 @@ struct data_offer {
> int fd;
> data_func_t func;
> int32_t x, y;
> + uint32_t dnd_action;
> + uint32_t source_actions;
> void *user_data;
> };
>
> @@ -3336,8 +3338,26 @@ data_offer_offer(void *data, struct wl_data_offer *wl_data_offer, const char *ty
> *p = strdup(type);
> }
>
> +static void
> +data_offer_source_actions(void *data, struct wl_data_offer *wl_data_offer, uint32_t source_actions)
> +{
> + struct data_offer *offer = data;
> +
> + offer->source_actions = source_actions;
> +}
> +
> +static void
> +data_offer_action(void *data, struct wl_data_offer *wl_data_offer, uint32_t dnd_action)
> +{
> + struct data_offer *offer = data;
> +
> + offer->dnd_action = dnd_action;
> +}
> +
> static const struct wl_data_offer_listener data_offer_listener = {
> data_offer_offer,
> + data_offer_source_actions,
> + data_offer_action
> };
>
> static void
> @@ -3401,6 +3421,14 @@ data_device_enter(void *data, struct wl_data_device *data_device,
> *p = NULL;
>
> types_data = input->drag_offer->types.data;
> +
> + if (input->display->data_device_manager_version >=
> + WL_DATA_OFFER_SET_ACTIONS_SINCE_VERSION) {
> + wl_data_offer_set_actions(offer,
> + WL_DATA_DEVICE_MANAGER_DND_ACTION_COPY |
> + WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE,
> + WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE);
> + }
> } else {
> input->drag_offer = NULL;
> types_data = NULL;
> @@ -5837,6 +5865,12 @@ display_exit(struct display *display)
> display->running = 0;
> }
>
> +int
> +display_get_data_device_manager_version(struct display *display)
> +{
> + return display->data_device_manager_version;
> +}
> +
> void
> keysym_modifiers_add(struct wl_array *modifiers_map,
> const char *name)
> diff --git a/clients/window.h b/clients/window.h
> index b92d10c..74a2c72 100644
> --- a/clients/window.h
> +++ b/clients/window.h
> @@ -175,6 +175,9 @@ display_run(struct display *d);
> void
> display_exit(struct display *d);
>
> +int
> +display_get_data_device_manager_version(struct display *d);
> +
> enum cursor_type {
> CURSOR_BOTTOM_LEFT,
> CURSOR_BOTTOM_RIGHT,
> diff --git a/src/compositor.h b/src/compositor.h
> index 8d6b051..e07179f 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -311,6 +311,9 @@ struct weston_data_offer {
> struct wl_resource *resource;
> struct weston_data_source *source;
> struct wl_listener source_destroy_listener;
> + uint32_t dnd_actions;
> + enum wl_data_device_manager_dnd_action preferred_dnd_action;
> + bool in_ask;
> };
>
> struct weston_data_source {
> @@ -320,6 +323,9 @@ struct weston_data_source {
> struct weston_data_offer *offer;
> struct weston_seat *seat;
> bool accepted;
> + bool actions_set;
> + uint32_t dnd_actions;
> + enum wl_data_device_manager_dnd_action current_dnd_action;
>
> 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 1cf0321..8820c93 100644
> --- a/src/data-device.c
> +++ b/src/data-device.c
> @@ -56,6 +56,10 @@ struct weston_touch_drag {
> struct weston_touch_grab grab;
> };
>
> +#define ALL_ACTIONS (WL_DATA_DEVICE_MANAGER_DND_ACTION_COPY | \
> + WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE | \
> + WL_DATA_DEVICE_MANAGER_DND_ACTION_ASK)
> +
> static void
> data_offer_accept(struct wl_client *client, struct wl_resource *resource,
> uint32_t serial, const char *mime_type)
> @@ -95,6 +99,13 @@ data_offer_destroy(struct wl_client *client, struct wl_resource *resource)
> static void
> data_source_notify_finish(struct weston_data_source *source)
> {
> + if (source->offer->in_ask &&
> + wl_resource_get_version(source->resource) >=
> + WL_DATA_SOURCE_ACTION_SINCE_VERSION) {
> + wl_data_source_send_action(source->resource,
> + source->current_dnd_action);
> + }
> +
> if (wl_resource_get_version(source->resource) >=
> WL_DATA_SOURCE_DND_FINISHED_SINCE_VERSION) {
> wl_data_source_send_dnd_finished(source->resource);
> @@ -103,6 +114,92 @@ data_source_notify_finish(struct weston_data_source *source)
> source->offer = NULL;
> }
>
> +static uint32_t
> +data_offer_choose_action(struct weston_data_offer *offer)
> +{
> + uint32_t available_actions, preferred_action = 0;
> + uint32_t source_actions, offer_actions;
> +
> + if (wl_resource_get_version(offer->resource) >=
> + WL_DATA_OFFER_ACTION_SINCE_VERSION) {
> + offer_actions = offer->dnd_actions;
> + preferred_action = offer->preferred_dnd_action;
> + } else {
> + offer_actions = WL_DATA_DEVICE_MANAGER_DND_ACTION_COPY;
> + }
> +
> + if (wl_resource_get_version(offer->source->resource) >=
> + WL_DATA_SOURCE_ACTION_SINCE_VERSION)
> + source_actions = offer->source->dnd_actions;
> + else
> + source_actions = WL_DATA_DEVICE_MANAGER_DND_ACTION_COPY;
> +
> + available_actions = offer_actions & source_actions;
> +
> + if (!available_actions)
> + return WL_DATA_DEVICE_MANAGER_DND_ACTION_NONE;
> +
> + /* If the dest side has a preferred DnD action, use it */
> + if ((preferred_action & available_actions) != 0)
> + return preferred_action;
> +
> + /* Use the first found action, in bit order */
> + return 1 << (ffs(available_actions) - 1);
> +}
> +
> +static void
> +data_offer_update_action(struct weston_data_offer *offer)
> +{
> + uint32_t action;
> +
> + if (!offer->source)
> + return;
> +
> + action = data_offer_choose_action(offer);
> +
> + if (offer->source->current_dnd_action == action)
> + return;
> +
> + offer->source->current_dnd_action = action;
> +
> + if (offer->in_ask)
> + return;
> +
> + if (wl_resource_get_version(offer->source->resource) >=
> + WL_DATA_SOURCE_ACTION_SINCE_VERSION)
> + wl_data_source_send_action(offer->source->resource, action);
> +
> + if (wl_resource_get_version(offer->resource) >=
> + WL_DATA_OFFER_ACTION_SINCE_VERSION)
> + wl_data_offer_send_action(offer->resource, action);
> +}
> +
> +static void
> +data_offer_set_actions(struct wl_client *client,
> + struct wl_resource *resource,
> + uint32_t dnd_actions, uint32_t preferred_action)
> +{
> + struct weston_data_offer *offer = wl_resource_get_user_data(resource);
> +
> + if (dnd_actions & ~ALL_ACTIONS) {
> + wl_resource_post_error(offer->resource,
> + WL_DATA_OFFER_ERROR_INVALID_ACTION_MASK,
> + "invalid action mask %x", dnd_actions);
> + return;
> + }
> +
> + if (__builtin_popcount(preferred_action) > 1) {
> + wl_resource_post_error(offer->resource,
> + WL_DATA_OFFER_ERROR_INVALID_ACTION,
> + "invalid action %x", preferred_action);
> + return;
> + }
I guess we should also error if preferred_action is not one of the
defined actions.
> +
> + offer->dnd_actions = dnd_actions;
> + offer->preferred_dnd_action = preferred_action;
> + data_offer_update_action(offer);
> +}
> +
> static void
> data_offer_finish(struct wl_client *client, struct wl_resource *resource)
> {
> @@ -122,6 +219,15 @@ data_offer_finish(struct wl_client *client, struct wl_resource *resource)
> return;
> }
>
> + if (!offer->source->current_dnd_action ||
> + offer->source->current_dnd_action ==
> + WL_DATA_DEVICE_MANAGER_DND_ACTION_ASK) {
> + wl_resource_post_error(offer->resource,
> + WL_DATA_OFFER_ERROR_INVALID_OFFER,
> + "offer finished with an invalid action");
> + return;
> + }
nit: I think this would be more clear if it would be a switch, i.e.
switch (offer->source->current_dnd_action) {
case WL_DATA_DEVICE_MANAGER_DND_ACTION_NONE:
case WL_DATA_DEVICE_MANAGER_DND_ACTION_ASK:
wl_resource_post_error(...);
return;
default:
break;
}
> +
> data_source_notify_finish(offer->source);
> }
>
> @@ -130,6 +236,7 @@ static const struct wl_data_offer_interface data_offer_interface = {
> data_offer_receive,
> data_offer_destroy,
> data_offer_finish,
> + data_offer_set_actions,
> };
>
> static void
> @@ -196,6 +303,9 @@ weston_data_source_send_offer(struct weston_data_source *source,
> wl_resource_set_implementation(offer->resource, &data_offer_interface,
> offer, destroy_data_offer);
>
> + offer->in_ask = false;
> + offer->dnd_actions = 0;
> + offer->preferred_dnd_action = WL_DATA_DEVICE_MANAGER_DND_ACTION_NONE;
> offer->source = source;
> offer->source_destroy_listener.notify = destroy_offer_data_source;
> wl_signal_add(&source->destroy_signal,
> @@ -208,6 +318,7 @@ weston_data_source_send_offer(struct weston_data_source *source,
>
> source->offer = offer;
> source->accepted = false;
> + data_offer_update_action(offer);
>
> return offer->resource;
> }
> @@ -234,9 +345,44 @@ data_source_destroy(struct wl_client *client, struct wl_resource *resource)
> wl_resource_destroy(resource);
> }
>
> +static void
> +data_source_set_actions(struct wl_client *client,
> + struct wl_resource *resource,
> + uint32_t dnd_actions)
> +{
> + struct weston_data_source *source =
> + wl_resource_get_user_data(resource);
> +
> + if (source->actions_set) {
> + wl_resource_post_error(source->resource,
> + WL_DATA_SOURCE_ERROR_INVALID_ACTION_MASK,
> + "cannot set actions more than once");
> + return;
> + }
> +
> + if (dnd_actions & ~ALL_ACTIONS) {
> + wl_resource_post_error(source->resource,
> + WL_DATA_SOURCE_ERROR_INVALID_ACTION_MASK,
> + "invalid action mask %x", dnd_actions);
> + return;
> + }
> +
> + if (source->seat) {
> + wl_resource_post_error(source->resource,
> + WL_DATA_SOURCE_ERROR_INVALID_ACTION_MASK,
> + "invalid action change after "
> + "wl_data_source.dnd_drop_performed");
Error message needs updating.
Jonas
> + return;
> + }
> +
> + source->dnd_actions = dnd_actions;
> + source->actions_set = true;
> +}
> +
> static struct wl_data_source_interface data_source_interface = {
> data_source_offer,
> - data_source_destroy
> + data_source_destroy,
> + data_source_set_actions
> };
>
> static void
> @@ -475,13 +621,18 @@ drag_grab_button(struct weston_pointer_grab *grab,
> pointer->grab_button == button &&
> state == WL_POINTER_BUTTON_STATE_RELEASED) {
> if (drag->base.focus_resource &&
> - data_source->accepted) {
> + data_source->accepted &&
> + data_source->current_dnd_action) {
> wl_data_device_send_drop(drag->base.focus_resource);
>
> if (wl_resource_get_version(data_source->resource) >=
> WL_DATA_SOURCE_DND_DROP_PERFORMED_SINCE_VERSION)
> wl_data_source_send_dnd_drop_performed(data_source->resource);
>
> + data_source->offer->in_ask =
> + data_source->current_dnd_action ==
> + WL_DATA_DEVICE_MANAGER_DND_ACTION_ASK;
> +
> data_source->seat = NULL;
> } else if (wl_resource_get_version(data_source->resource) >=
> WL_DATA_SOURCE_DND_FINISHED_SINCE_VERSION) {
> @@ -900,13 +1051,23 @@ data_device_set_selection(struct wl_client *client,
> struct wl_resource *resource,
> struct wl_resource *source_resource, uint32_t serial)
> {
> + struct weston_data_source *data_source;
> +
nit: data_source -> source
> if (!source_resource)
> return;
>
> + data_source = wl_resource_get_user_data(source_resource);
> +
> + if (data_source->actions_set) {
> + wl_resource_post_error(source_resource,
> + WL_DATA_SOURCE_ERROR_INVALID_SOURCE,
> + "cannot set drag-and-drop source as selection");
> + return;
> + }
> +
> /* FIXME: Store serial and check against incoming serial here. */
> weston_seat_set_selection(wl_resource_get_user_data(resource),
> - wl_resource_get_user_data(source_resource),
> - serial);
> + data_source, serial);
> }
> static void
> data_device_release(struct wl_client *client, struct wl_resource *resource)
> @@ -986,6 +1147,9 @@ create_data_source(struct wl_client *client,
> source->offer = NULL;
> source->accepted = false;
> source->seat = NULL;
> + source->actions_set = false;
> + source->dnd_actions = 0;
> + source->current_dnd_action = WL_DATA_DEVICE_MANAGER_DND_ACTION_NONE;
>
> wl_array_init(&source->mime_types);
>
> --
> 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