On Tue, Dec 15, 2015 at 06:56:27PM +0100, Carlos Garnacho wrote: > In order to keep things simple, weston-dnd made a few choices that > turn out to be unrealistic, a few tweaks have been done to make it > less of a playground demo: > > - It now caters for copy/move operations, instead of just move, > which still remains the default nonetheless. > - As "move" operations are no longer assumed, the item isn't removed > on start_drag, instead it is made translucent until the drag > operation finishes (and we know whether the item is to be > removed after transfer or left as is) > - For the same reasons, "Drop nowhere to delete item" no longer > happens. Drag-and-drop is a failable operation and must not result > in data loss. > - As multiple actions are now allowed, we set the pointer icon > surface accordingly to the current operation. > > This makes weston-dnd a better example of what applications usually > want to do here. > > Signed-off-by: Carlos Garnacho <[email protected]>
Looks good to me, just have a couple of nits inline. With those fixed, consider this Reviewed-by: Jonas Ådahl <[email protected]> . > --- > clients/dnd.c | 85 > ++++++++++++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 64 insertions(+), 21 deletions(-) > > diff --git a/clients/dnd.c b/clients/dnd.c > index 4a046b1..dcb9ede 100644 > --- a/clients/dnd.c > +++ b/clients/dnd.c > @@ -36,6 +36,7 @@ > #include <sys/epoll.h> > #include <stdbool.h> > > +#include <linux/input.h> Is this needed? > #include <wayland-client.h> > #include <wayland-cursor.h> > > @@ -190,7 +191,7 @@ dnd_redraw_handler(struct widget *widget, void *data) > struct dnd *dnd = data; > struct rectangle allocation; > cairo_t *cr; > - cairo_surface_t *surface; > + cairo_surface_t *surface, *item_surface; > unsigned int i; > > surface = window_get_surface(dnd->window); > @@ -210,7 +211,13 @@ dnd_redraw_handler(struct widget *widget, void *data) > for (i = 0; i < ARRAY_LENGTH(dnd->items); i++) { > if (!dnd->items[i]) > continue; > - cairo_set_source_surface(cr, dnd->items[i]->surface, > + > + if (dnd->current_drag && dnd->items[i] == > dnd->current_drag->item) > + item_surface = dnd->current_drag->translucent; > + else > + item_surface = dnd->items[i]->surface; > + > + cairo_set_source_surface(cr, item_surface, > dnd->items[i]->x + allocation.x, > dnd->items[i]->y + allocation.y); > cairo_paint(cr); > @@ -266,6 +273,30 @@ dnd_get_item(struct dnd *dnd, int32_t x, int32_t y) > return NULL; > } > > +static int > +lookup_dnd_cursor(uint32_t dnd_action) > +{ > + if (dnd_action == WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE) > + return CURSOR_DND_MOVE; > + else if (dnd_action == WL_DATA_DEVICE_MANAGER_DND_ACTION_COPY) > + return CURSOR_DND_COPY; > + > + return CURSOR_DND_FORBIDDEN; > +} > + > +static void > +dnd_drag_update_cursor(struct dnd_drag *dnd_drag) > +{ > + int cursor; > + > + if (dnd_drag->mime_type == NULL) > + cursor = CURSOR_DND_FORBIDDEN; > + else > + cursor = lookup_dnd_cursor(dnd_drag->dnd_action); > + > + input_set_pointer_image(dnd_drag->input, cursor); > +} > + > static void > dnd_drag_update_surface(struct dnd_drag *dnd_drag) > { > @@ -293,6 +324,8 @@ data_source_target(void *data, > > dnd_drag->mime_type = mime_type; > dnd_drag_update_surface(dnd_drag); > + dnd_drag_update_cursor(dnd_drag); > + Stray newline. > } > > static void > @@ -326,13 +359,27 @@ data_source_send(void *data, struct wl_data_source > *source, > } > > static void > -destroy_dnd_drag(struct dnd_drag *dnd_drag) > +destroy_dnd_drag(struct dnd_drag *dnd_drag, bool delete_item) > { > + struct dnd *dnd = dnd_drag->dnd; > + unsigned int i; > + > wl_data_source_destroy(dnd_drag->data_source); > > - /* Destroy the item that has been dragged out */ > - cairo_surface_destroy(dnd_drag->item->surface); > - free(dnd_drag->item); > + if (delete_item) { > + for (i = 0; i < ARRAY_LENGTH(dnd->items); i++) { > + if (dnd_drag->item == dnd->items[i]) { > + dnd->items[i] = 0; While at it, can replace the 0 with NULL I suppose. > + break; > + } > + } > + > + /* Destroy the item that has been dragged out */ > + cairo_surface_destroy(dnd_drag->item->surface); > + free(dnd_drag->item); > + } > + > + dnd->current_drag = NULL; > > wl_surface_destroy(dnd_drag->drag_surface); > > @@ -345,11 +392,13 @@ static void > data_source_cancelled(void *data, struct wl_data_source *source) > { > struct dnd_drag *dnd_drag = data; > + struct dnd *dnd = dnd_drag->dnd; > > /* The 'cancelled' event means that the source is no longer in > * use by the drag (or current selection). We need to clean > * up the drag object created and the local state. */ > - destroy_dnd_drag(dnd_drag); > + destroy_dnd_drag(dnd_drag, false); > + window_schedule_redraw(dnd->window); > } > > static void > @@ -361,11 +410,13 @@ static void > data_source_drag_finished(void *data, struct wl_data_source *source) > { > struct dnd_drag *dnd_drag = data; > + struct dnd *dnd = dnd_drag->dnd; > > /* The operation is already finished, we can destroy all > * related data. > */ > - destroy_dnd_drag(dnd_drag); > + destroy_dnd_drag(dnd_drag, dnd_drag->dnd_action == > WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE); Quite a long line. Maybe make a temporary bool delete_item? > + window_schedule_redraw(dnd->window); > } > > static void > @@ -375,6 +426,7 @@ data_source_action(void *data, struct wl_data_source > *source, uint32_t dnd_actio > > dnd_drag->dnd_action = dnd_action; > dnd_drag_update_surface(dnd_drag); > + dnd_drag_update_cursor(dnd_drag); > } > > static const struct wl_data_source_listener data_source_listener = { > @@ -429,9 +481,9 @@ create_drag_source(struct dnd *dnd, > struct display *display; > struct wl_compositor *compositor; > struct wl_buffer *buffer; > - unsigned int i; > uint32_t serial; > cairo_surface_t *icon; > + uint32_t actions; > > widget_get_allocation(dnd->widget, &allocation); > item = dnd_get_item(dnd, x, y); > @@ -449,12 +501,8 @@ create_drag_source(struct dnd *dnd, > 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]){ > - dnd->items[i] = 0; > - break; > - } > - } > + actions = WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE | > + WL_DATA_DEVICE_MANAGER_DND_ACTION_COPY; > > display = window_get_display(dnd->window); > compositor = display_get_compositor(display); > @@ -481,8 +529,7 @@ 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); > + wl_data_source_set_actions(dnd_drag->data_source, actions); > > dnd_drag->opaque = > create_drag_icon(dnd_drag, item, x, y, 1); > @@ -556,7 +603,6 @@ dnd_button_handler(struct widget *widget, > if (state == WL_POINTER_BUTTON_STATE_PRESSED) { > input_ungrab(input); > if (create_drag_source(dnd, input, time, x, y) == 0) { > - input_set_pointer_image(input, CURSOR_DRAGGING); I think this will result in no image being set if the object version is only <= 2. Won't it also create an indefinite delay until the first action event is received? Meaning roundtrips to the destination client? Jonas > update_pointer_images_except(dnd, input); > } > } > @@ -587,8 +633,6 @@ dnd_enter_handler(struct widget *widget, > struct dnd *dnd = data; > struct pointer *new_pointer = malloc(sizeof *new_pointer); > > - dnd->current_drag = NULL; > - > if (new_pointer) { > new_pointer->input = input; > new_pointer->dragging = false; > @@ -699,7 +743,6 @@ dnd_drop_handler(struct window *window, struct input > *input, > message.x_offset = dnd->current_drag->x_offset; > message.y_offset = dnd->current_drag->y_offset; > dnd_receive_func(&message, sizeof message, x, y, dnd); > - dnd->current_drag = NULL; > } else { > fprintf(stderr, "ignoring drop from another client\n"); > } > -- > 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
