Hey, On Wed, Dec 23, 2015 at 3:07 AM, Jonas Ådahl <jad...@gmail.com> wrote: > On Tue, Dec 22, 2015 at 04:46:37PM +0100, Carlos Garnacho wrote: >> Hey, >> >> On Tue, Dec 22, 2015 at 2:55 PM, Jonas Ådahl <jad...@gmail.com> wrote: >> > On Tue, Dec 22, 2015 at 01:33:08PM +0100, Carlos Garnacho wrote: >> >> Hey!, >> >> >> >> On Tue, Dec 22, 2015 at 3:26 AM, Jonas Ådahl <jad...@gmail.com> wrote: >> >> > On Tue, Dec 22, 2015 at 02:33:32AM +0100, Carlos Garnacho wrote: >> >> >> Currently, there's no means for the DnD origin to know whether the >> >> >> destination is actually finished with the DnD transaction, short of >> >> >> finalizing it after the first transfer finishes, or leaking it forever. >> >> >> >> >> >> But this poses other interoperation problems, drag destinations might >> >> >> be requesting several mimetypes at once, might be just poking to find >> >> >> out the most suitable format, might want to defer the action to a >> >> >> popup, >> >> >> might be poking contents early before the selection was dropped... >> >> >> >> >> >> In addition, data_source.cancelled is suitable for the situations where >> >> >> the DnD operation fails (not on a drop target, no matching mimetypes, >> >> >> etc..), but seems undocumented for that use (and unused in weston's >> >> >> DnD). >> >> >> >> >> >> In order to improve the situation, the drag source should be notified >> >> >> of all stages of DnD. In addition to documenting the "cancelled" event >> >> >> for DnD purposes, The following 2 events have been added: >> >> >> >> >> >> - wl_data_source.dnd_drop_performed: Happens when the operation has >> >> >> been >> >> >> physically finished (eg. the button is released), it could be the >> >> >> right >> >> >> place to reset the pointer cursor back and undo any other state >> >> >> resulting >> >> >> from the initial button press. >> >> >> - wl_data_source.dnd_finished: Happens when the destination side >> >> >> destroys >> >> >> the wl_data_offer, at this point the source can just forget all data >> >> >> related to the DnD selection as well, plus optionally deleting the >> >> >> data >> >> >> on move operations. >> >> >> >> >> >> Changes since v4: >> >> >> - Applied rewording suggestions from Jonas Ådahl. Added new >> >> >> wl_data_offer.finish request to allow explicit finalization on the >> >> >> destination side. >> >> >> >> >> >> Changes since v3: >> >> >> - Renamed dnd_performed to a more descriptive dnd_drop_performed, >> >> >> documented backwards compatible behavior on wl_data_offer.accept >> >> >> and >> >> >> wl_data_source.cancelled. >> >> >> >> >> >> Changes since v2: >> >> >> - Minor rewording. >> >> >> >> >> >> Changes since v1: >> >> >> - Renamed events to have a common "dnd" namespace. Made >> >> >> dnd_performed to >> >> >> happen invariably, data_device.cancelled may still happen >> >> >> afterwards. >> >> >> >> >> >> Signed-off-by: Carlos Garnacho <carl...@gnome.org> >> >> >> Reviewed-by: Michael Catanzaro <mcatanz...@igalia.com> >> >> >> Reviewed-by: Jonas Ådahl <jad...@gmail.com> >> >> >> Reviewed-by: Mike Blumenkrantz <zm...@samsung.com> >> >> > >> >> > Looks pretty good now. Btw, if you put the "Changes since ..." >> >> > underneath the "---" below after doing git-format-patch you don't have >> >> > to keep the patch change log in the actual commit message. >> >> >> >> Ah sure, I thought the kernel style of keeping patch changelog for >> >> posteriority was preferred. >> > >> > Actually I don't know what is preferred. Personally I put the changelog >> > outside of the commit message and thought that was what was preferred >> > but I might be wrong. >> > >> >> >> >> > >> >> > One comment and a couple of nits below. >> >> > >> >> >> --- >> >> >> protocol/wayland.xml | 75 >> >> >> +++++++++++++++++++++++++++++++++++++++++++++++----- >> >> >> 1 file changed, 69 insertions(+), 6 deletions(-) >> >> >> >> >> >> diff --git a/protocol/wayland.xml b/protocol/wayland.xml >> >> >> index df8ed19..ae5ef21 100644 >> >> >> --- a/protocol/wayland.xml >> >> >> +++ b/protocol/wayland.xml >> >> >> @@ -408,7 +408,7 @@ >> >> >> </interface> >> >> >> >> >> >> >> >> >> - <interface name="wl_data_offer" version="1"> >> >> >> + <interface name="wl_data_offer" version="3"> >> >> >> <description summary="offer to transfer data"> >> >> >> A wl_data_offer represents a piece of data offered for transfer >> >> >> by another client (the source client). It is used by the >> >> >> @@ -423,7 +423,17 @@ >> >> >> Indicate that the client can accept the given mime type, or >> >> >> NULL for not accepted. >> >> >> >> >> >> - Used for feedback during drag-and-drop. >> >> >> + For objects of version 2 or older, this request is used by the >> >> >> + client to give feedback whether the client can receive the given >> >> >> + mime type, or NULL if none is accepted; the feedback does not >> >> >> + determine whether drag-and-drop operation succeeds or not. >> >> >> + >> >> >> + For objects of version 3 or newer, this request determines the >> >> >> + final result of the drag-and-drop operation. If the end result >> >> >> + is that no mime types was accepted, the drag-and-drop operation >> >> >> + will be cancelled and the corresponding drag source will receive >> >> >> + wl_data_source.cancelled. Clients may still use this event in >> >> >> + conjunction with wl_data_source.action for feedback. >> >> >> </description> >> >> >> >> >> >> <arg name="serial" type="uint"/> >> >> >> @@ -461,9 +471,24 @@ >> >> >> >> >> >> <arg name="mime_type" type="string"/> >> >> >> </event> >> >> >> + >> >> >> + <!-- Version 3 additions --> >> >> >> + >> >> >> + <request name="finish" since="3"> >> >> >> + <description summary="the offer will no longer be used"> >> >> >> + Notifies the compositor that the data offer will no longer be >> >> >> used. >> >> >> + Upon receiving this request, the compositor will emit >> >> >> + wl_data_source.drag_finished or wl_data_source.cancelled on the >> >> >> drag >> >> >> + source client depending on the most recent wl_data_offer.accept >> >> >> and >> >> >> + wl_data_offer.set_actions requests received from this offer. >> >> > >> >> > Hmm. What I had in mind is that .finish is called after the transfer is >> >> > completed, i.e. the accept and actions (nit: which btw is not introduced >> >> > in this patch so I suppose should not really be referenced yet) have >> >> >> >> Oops indeed. >> >> >> >> > already been ensured to be compatible. So, I don't see how >> >> > wl_data_source.cancelled could ever be emitted with well behaving >> >> > clients. >> >> >> >> Two well behaved clients can disagree on the actions (eg. source only >> >> offering "copy", dest only allowing "move"), the resulting action will >> >> be "none", and wl_data_source.cancelled would be emitted. Same for >> >> dropping places that are not really a drag destination, the dest >> >> client should .set_actions(0) if dropping is not allowed there. >> > >> > Yes, but if the resulting action is "none", the destination side would >> > never ever call "finish" to begin with since it has nothing that it can >> > finish. I thought the point of "finish" was that an already successfully >> > negotiated data transfer had finished transferring. >> >> The resulting action may end up as none after wl_data_device.drop (eg. >> cancelling a menu popped up on "ask"), and the dest client would need >> to wl_data_offer.finish() it. > > Why would it wl_data_offer.finish() it and not wl_data_offer.destroy() > it? It didn't finish anything, since the action is "none". Or do you > want to special case "none" where "finish" is the same as "destroy"? > >> >> There's also the small race condition pointed out in >> http://lists.freedesktop.org/archives/wayland-devel/2015-October/025107.html, >> which means that button release may not be a proper time to decide >> about wl_drag_source.cancelled vs .dnd_drop_performed (and eventually >> .dnd_finished), which means drop sites can still potentially do a last >> .set_actions() change soon after .drop, and that means they have to >> .finish() too. > > Yes, but I thought wl_data_source.cancelled would either come before or > after .dnd_drop_performed, and may very well come as a result of > wl_data_offer.destroy much later than .dnd_drop_performed.
Hmm, right. If it's "I disallow DnD" what the drag dest wants, destroying would be just as effective. In my local changes I have in data_offer_destroy (pseudocode): if (offer->source) { if (offer_version < 3) data_offer_notify_finish(offer); else if (source_version >= 3) wl_data_source_send_cancelled(offer->source->resource); } That should cater for what you mention here. Cheers, Carlos _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel