[PATCH weston v7 2/5] data-device: Implement DnD actions

Jonas Ã…dahl jadahl at gmail.com
Thu Jan 14 23:17:10 PST 2016


Hey,

Found a couple of more issues. Comments inline.

On Thu, Jan 14, 2016 at 11:46:34PM +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 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>
> ---
>  clients/dnd.c     |  32 +++++++++--
>  clients/window.c  |  25 ++++++++
>  src/compositor.h  |   5 ++
>  src/data-device.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  4 files changed, 221 insertions(+), 10 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);

This call must be above the start_drag, and inside a version check.

>  
>  		dnd_drag->opaque =
>  			create_drag_icon(dnd_drag, item, x, y, 1);
> diff --git a/clients/window.c b/clients/window.c
> index c93edb1..db1fe57 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,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);
> +

Missing version check and stray newline.


Jonas

>  	} else {
>  		input->drag_offer = NULL;
>  		types_data = NULL;
> diff --git a/src/compositor.h b/src/compositor.h
> index 8d6b051..c53949e 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -311,6 +311,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 {
> @@ -320,6 +322,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 d2b89bb..18448bf 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)
> @@ -107,6 +111,89 @@ 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;
> +
> +	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 (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;
> +	}
> +
> +	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)
>  {
> @@ -126,6 +213,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;
> +	}
> +
>  	data_offer_notify_finish(offer);
>  }
>  
> @@ -134,6 +230,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
> @@ -183,7 +280,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;
> @@ -192,6 +290,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,
> @@ -204,6 +304,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;
>  }
> @@ -230,9 +331,53 @@ 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");
> +		return;
> +	}
> +
> +	source->dnd_actions = dnd_actions;
> +	source->actions_set = true;
> +
> +	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
> @@ -471,7 +616,8 @@ 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) >=
> @@ -895,13 +1041,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;
> +
>  	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)
> @@ -980,6 +1136,9 @@ create_data_source(struct wl_client *client,
>  	source->cancel = client_source_cancel;
>  	source->offer = NULL;
>  	source->accepted = false;
> +	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