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 <[email protected]> > Reviewed-by: Michael Catanzaro <[email protected]>
Except for couple of minor issues and a couple of nits, this patch is now Reviewed-by: Jonas Ådahl <[email protected]>. > --- > 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
