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