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

[Carlos Garnacho]

Hey,

On Wed, Dec 16, 2015 at 3:37 AM, Jonas Ådahl <jadahl at gmail.com> wrote:
> On Tue, Dec 15, 2015 at 06:56:23PM +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 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 | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>  4 files changed, 128 insertions(+), 8 deletions(-)
>>
>> diff --git a/clients/dnd.c b/clients/dnd.c
>> index 6c2ed57..4a046b1 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)
>>       destroy_dnd_drag(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;
>
> This made me think of another issue. Here we default to the "move"
> action. Are we guaranteed to always receive an wl_data_source.action
> event before the drop is performed? What if the sides don't have enough
> time to set their supported actions, what will we always get a "none"
> action sent?

The first wl_data_source.set_actions call should happen before or
together with .start_drag(), and the first wl_data_offer.set_actions
should happen in response to wl_data_device.enter, AFAICT there's no
room for that to happen, either the surface doesn't have the pointer
focus yet, or it was focused and has acknowledged a set of actions. As
a side effect, an .action event on both sides is expected to happen
soon after the data device enters a surface.

But yes, if either side fails to report the allowed actions, the
resulting effect should be "none" because (source_actions &
dest_actions) == 0

>
> I guess this should be specified in the protocol, and reflected here. I
> assume "move" is the default for version < 3 objects though.

Yup, exactly. I'll make it apply to <3 only, now that I'm at versioning.

Cheers,
  Carlos