[PATCH weston 2/5] data-device: Implement DnD actions
Bill Spitzak
spitzak at gmail.com
Mon Jan 11 11:26:01 PST 2016
Can you please explain in detail the difference between the actions being
"COPY + MOVE" and being "COPY + MOVE + ASK". I do not at all understand the
purpose of "ASK". It might also help to specify what should happen if ASK
is combined with less than 2 other actions.
On Mon, Jan 11, 2016 at 12:30 AM, Jonas Ådahl <jadahl at gmail.com> wrote:
> On Thu, Dec 24, 2015 at 02:00:38AM +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.
> >
>
> Mostly looks good; but I have some questions/comments related to how
> 'ask' is implemented.
>
> > 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>
> > ---
> > clients/dnd.c | 32 ++++++++++---
> > clients/window.c | 25 +++++++++++
> > src/compositor.h | 4 ++
> > src/data-device.c | 131
> ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > 4 files changed, 183 insertions(+), 9 deletions(-)
> >
> > diff --git a/clients/dnd.c b/clients/dnd.c
> > index 48111d9..ddf3fcc 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_drag_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_drop_performed,
> > data_source_drag_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]){
> > @@ -461,6 +481,8 @@ create_drag_source(struct dnd *dnd,
> >
> window_get_wl_surface(dnd->window),
> > dnd_drag->drag_surface,
> > serial);
> > + wl_data_source_set_actions(dnd_drag->data_source,
> > +
> WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE);
> >
> > dnd_drag->opaque =
> > create_drag_icon(dnd_drag, item, x, y, 1);
> > diff --git a/clients/window.c b/clients/window.c
> > index db96185..9c67b91 100644
> > --- a/clients/window.c
> > +++ b/clients/window.c
> > @@ -3362,6 +3362,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;
> > };
> >
> > @@ -3375,8 +3377,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
> > @@ -3440,6 +3460,11 @@ data_device_enter(void *data, struct
> wl_data_device *data_device,
> > *p = NULL;
> >
> > types_data = input->drag_offer->types.data;
> > + 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;
> > diff --git a/src/compositor.h b/src/compositor.h
> > index 83d6f7f..2956f98 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -298,6 +298,8 @@ 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;
> > };
> >
> > struct weston_data_source {
> > @@ -307,6 +309,8 @@ struct weston_data_source {
> > struct weston_data_offer *offer;
> > struct weston_seat *seat;
> > bool accepted;
> > + 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 9942a66..ab81ba9 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)
> > @@ -101,7 +105,8 @@ data_offer_notify_finish(struct weston_data_offer
> *offer)
> >
> > if (wl_resource_get_version(offer->source->resource) >=
> > WL_DATA_SOURCE_DND_FINISHED_SINCE_VERSION) {
> > - if (offer->source->accepted)
> > + if (offer->source->accepted &&
> > + offer->source->current_dnd_action)
> >
> wl_data_source_send_dnd_finished(offer->source->resource);
> > else
> >
> wl_data_source_send_cancelled(offer->source->resource);
> > @@ -110,6 +115,86 @@ data_offer_notify_finish(struct weston_data_offer
> *offer)
> > offer->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;
> > +
> > + action = data_offer_choose_action(offer);
> > +
> > + if (offer->source->current_dnd_action == action)
> > + return;
> > +
> > + offer->source->current_dnd_action = action;
> > +
> > + 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);
>
> Should we still send action after an 'ask' was dropped and the
> destination chose a final 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;
> > + }
>
> Should we not error if a destination set more than one action after an
> 'ask' was performed?
>
> > +
> > + 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)
> > {
> > @@ -134,6 +219,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
> > @@ -146,7 +232,7 @@ destroy_data_offer(struct wl_resource *resource)
> > * won't be called, so do this here as a safety net,
> because
> > * we still want the version >=3 drag source to be happy.
> > */
> > - if (wl_resource_get_version (offer->source->resource) <
> > + if (wl_resource_get_version(offer->source->resource) <
>
> Seems this change leaked over from the previous patch.
>
>
> Jonas
>
> > WL_DATA_SOURCE_DND_FINISHED_SINCE_VERSION)
> > data_offer_notify_finish(offer);
> > else
> > @@ -181,7 +267,8 @@ weston_data_source_send_offer(struct
> weston_data_source *source,
> >
> > offer->resource =
> > wl_resource_create(wl_resource_get_client(target),
> > - &wl_data_offer_interface, 1, 0);
> > + &wl_data_offer_interface,
> > +
> wl_resource_get_version(source->resource), 0);
> > if (offer->resource == NULL) {
> > free(offer);
> > return NULL;
> > @@ -190,6 +277,8 @@ weston_data_source_send_offer(struct
> weston_data_source *source,
> > wl_resource_set_implementation(offer->resource,
> &data_offer_interface,
> > offer, destroy_data_offer);
> >
> > + 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,
> > @@ -202,6 +291,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;
> > }
> > @@ -228,9 +318,40 @@ 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 (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->dnd_actions == dnd_actions)
> > + return;
> > +
> > + source->dnd_actions = dnd_actions;
> > +
> > + if (source->offer) {
> > + if (wl_resource_get_version(source->offer->resource) >=
> > + WL_DATA_OFFER_SOURCE_ACTIONS_SINCE_VERSION) {
> > +
> wl_data_offer_send_source_actions(source->offer->resource,
> > + dnd_actions);
> > + }
> > + data_offer_update_action(source->offer);
> > + }
> > +}
> > +
> > static struct wl_data_source_interface data_source_interface = {
> > data_source_offer,
> > - data_source_destroy
> > + data_source_destroy,
> > + data_source_set_actions
> > };
> >
> > static void
> > @@ -951,6 +1072,8 @@ create_data_source(struct wl_client *client,
> > source->cancel = client_source_cancel;
> > source->offer = NULL;
> > source->accepted = 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
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20160111/e55c3488/attachment-0001.html>
More information about the wayland-devel
mailing list