On Wed, Dec 16, 2015 at 02:50:59PM +0100, Carlos Garnacho wrote: > Hey Jonas! > > On Wed, Dec 16, 2015 at 2:40 AM, Jonas Ådahl <[email protected]> wrote: > > On Tue, Dec 15, 2015 at 06:56:24PM +0100, Carlos Garnacho wrote: > >> These 2 requests have been added: > >> > >> - wl_data_source.set_actions: Notifies the compositor of the available > >> actions on the data source. > >> - wl_data_offer.set_actions: Notifies the compositor of the available > >> actions on the destination side, plus the preferred action. > >> > >> Out of the data from these requests, the compositor can determine the > >> action > >> both parts agree on (and let the user play a role through eg. keyboard > >> modifiers). The chosen option will be notified to both parties > >> through the following two requests: > >> > >> - wl_data_source.action > >> - wl_data_offer.action > >> > >> In addition, the destination side can peek the source side actions through > >> wl_data_offer.source_actions. > >> > >> Compared to the XDND protocol, there's two notable changes: > >> > >> - XDND lets the source suggest an action, whereas wl_data_device lets > >> the destination prefer a given action. The difference is subtle here, > >> it comes off as convenience because it is the drag destination which > >> receives the motion events (unlike in X) and can perform action updates. > >> > >> The drag destination seems also in a better position to update the > >> preferred action based on things like the data being transferred, the > >> place being dropped, and whether the drag is client-local. > >> > >> - That same source-side preferred action is used in XDND to convey the > >> modifier-induced action to the drag destination, which would then ack > >> it, or reply with another action that's accepted (or none), this makes > >> the XdndPosition/XdndStatus messaging very verbose, and synchronous > >> because the drag source always needs to know the latest status/action > >> for every position+action sent. > >> > >> Here it's the compositor which takes care of modifiers and matching > >> available/accepted actions, this allows for the signaling to happen > >> only whenever the actions/modifiers change for real. > >> > >> Roughly based on previous work by Giulio Camuffo <[email protected]> > >> > >> Changes since v4: > >> - Minor rewording. > >> > >> Changes since v3: > >> - Splitted from DnD progress notification changes. > >> - Further rationales in commit log. > >> > >> Changes since v2: > >> - Renamed notify_actions to set_actions on both sides, seems more > >> consistent > >> with the rest of the protocol. > >> - Spelled out better which events may be triggered on the compositor side > >> by the requests, the circumstances in which events are emitted, and > >> what are events useful for in clients. > >> - Defined a minimal common ground wrt compositor-side action picking and > >> keybindings. > >> - Acknowledge the possibility of compositor/toolkit defined actions, even > >> though none are used at the moment. > >> Changes since v1: > >> - Added wl_data_offer.source_actions to let know of the actions offered > >> by a data source. > >> - Renamed wl_data_source.finished to "drag_finished" for clarity > >> - Improved wording as suggested by Bryce > >> > >> Signed-off-by: Carlos Garnacho <[email protected]> > >> Reviewed-by: Michael Catanzaro <[email protected]> > >> Reviewed-by: Mike Blumenkrantz <[email protected]> > > > > Sorry for taking my sweet time, but anyhow, overall I think it looks > > good, but obligatory bikeshedding follows. > > Thanks for the review :) > > > > >> --- > >> protocol/wayland.xml | 129 > >> +++++++++++++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 129 insertions(+) > >> > >> diff --git a/protocol/wayland.xml b/protocol/wayland.xml > >> index b54bcd0..ab64762 100644 > >> --- a/protocol/wayland.xml > >> +++ b/protocol/wayland.xml > >> @@ -468,6 +468,56 @@ > >> > >> <arg name="mime_type" type="string"/> > >> </event> > >> + > >> + <!-- Version 3 additions --> > > > > bikeshed: For consistency, I think added requests should always come before > > added events in the XML. > > Ah sure, didn't know there was a pattern here. > > > > >> + > >> + <event name="source_actions" since="3"> > >> + <description summary="notify the source-side available actions"> > >> + This event indicates the actions offered by the data source. It > >> + will be sent right after data_device.enter, or anytime the source > > > > wl_data_device.enter. > > > >> + side changes its offered actions through data_source.set_actions. > >> + </description> > >> + <arg name="source_actions" type="uint"/> > >> + </event> > >> + > >> + <event name="action" since="3"> > >> + <description summary="notify the selected action"> > >> + This event indicates the action selected by the compositor after > >> + matching the source/destination side actions. Only one action (or > >> + none) will be offered here. > >> + > >> + This event can be emitted multiple times during the drag-and-drop > >> + operation, mainly in response to source side changes (through > >> + data_source.set_actions), destination side changes (through > >> + data_offer.set_actions), and as pointer enters/leaves surfaces. > > > > wl_data_source.set_actions and wl_data_offer.set_actions. > > > >> + > >> + Compositors may also change the selected action on the fly, mainly > >> + in response to keyboard modifier changes during the drag-and-drop > >> + operation. > >> + > >> + The most recent action received is always the valid one. > > > > I think it would be good to mention the last action emitted before the > > wl_data_device.drop event is the action that should be used when the > > completing the operation. Also that no "action" event will be emitted > > after the "wl_data_device.drop" event. > > Hmm, although that may not be true. In "ask" operations, the drag > destination may switch either mimetype or action after .drop happened. > Examples I can think of: > - [Move|Copy|Link|Cancel] menu in a file manager (this one's not made > up, nautilus has this) > - [Paste as plain text|Paste with format] menu in some wysiwyg editor > > In these cases the final action will be possibly chosen well after > .drop happened, and will be different to the one happening at that > time (ask). But the drag source won't receive .drag_finished/cancelled > until that last action is chosen and dealt with, so it is that one > which would be honored. It is a good idea to explain the possible > "ask" behavior further here.
Ah right, makes sense. So should it instead be "the last message before wl_data_source.send"? I'd like to have it more strictly specified than "most recent" as well as at what point it will never be emitted again. > > > > >> + </description> > >> + <arg name="dnd_action" type="uint"/> > >> + </event> > >> + > >> + <request name="set_actions" since="3"> > >> + <description summary="set the available/preferred drag-and-drop > >> actions"> > >> + Sets the actions that the destination side client supports for > >> + this operation. This request may trigger the emission of > >> + data_source.action and data_offer.action events if the compositor > > > > wl_data_source.action and wl_data_offer.action. > > > >> + needs changing the selected action. > >> + > >> + This request can be called multiple times throughout the > >> + drag-and-drop operation, typically in response to data_device.enter > >> + or data_device.motion events. > > > > wl_data_device.enter and wl_data_device.motion. > > > >> + > >> + This request determines the final result of the drag-and-drop > >> + operation. If the end result is that no action is accepted, > >> + the drag source will receive drag_source.cancelled. > > > > wl_data_source.cancelled. > > > >> + </description> > >> + <arg name="dnd_actions" type="uint"/> > >> + <arg name="preferred_action" type="uint"/> > >> + </request> > >> </interface> > >> > >> <interface name="wl_data_source" version="3"> > >> @@ -524,6 +574,9 @@ > >> - The drag-and-drop operation was performed, but the drop destination > >> did not accept any of the mimetypes offered through > >> data_source.target. > > > > I know this is the wrong patch for commenting on this, but here was > > spaces instead of tabs as well (the same for the point below the added > > one). > > Meh, sorry about these. Too many different indentation schemes around, > I open the file with the wrong emacs window and boom. > > > > >> + - The drag-and-drop operation was performed, but the drop destination > >> + did not select any action present in the mask offered through > >> + data_source.action. > > > > Mixed tabs and spaces (indentation). > > > >> - The drag-and-drop operation was performed but didn't happen over a > >> surface. > >> - The compositor cancelled the drag-and-drop operation > >> @@ -558,8 +611,44 @@ > >> The drop destination finished interoperating with this data > >> source, the client is now free to destroy this data source and > >> free all associated data. > >> + > >> + Clients can also trigger the deletion of source-side data on > >> + "move" drag-and-drop operations. > > > > This wording seems a bit off to me. At first it sounded like I was back > > at the wl_data_offer side and it could trigger the deletion of client > > side things, but then I remembered what interface it was in. A > > suggestion: > > > > If the action used to perform the operation was "move", the > > source can now delete the transferred data. > > Right, I agree it sounds clearer. > > > > > A side thought; how is this supposed to work when move does not require > > any deletion? I.e. a file moved within a filesystem. > > In a file manager there's indeed special considerations around "move" > operations, rename(2) is only possible if files move across the same > filesystem, otherwise unlink(2) must happen after the file was fully > copied. Because at the DnD level everything they transfer is URIs, and > copy/move operations happen off-band, I'd expect the final delete to > happen off-band too. > > At the DnD level, what AFAICT file managers usually do is: > - Never explicitly delete as a result of "move" operations > - Disallow/ignore "move" operations on drags to a different process, > because in these cases you know nothing about the operation performed > afterwards, only that the URIs were transferred correctly. > > It makes some sense if you think of the DnD transfer being purely > about the URIs, the file manager transfers an URI list and an intent, > and it does succeed at that, regardless of the copy operation being > cancelled afterwards by the user, the destination partition being > full, the ssh connection dropping... in terms of DnD, the URI list > triggered something on the destination, anything that happens after > that belongs somewhere else. I see. Do you think any of this should be put in the protocol in any way? What the moved "data" means is a bit ambiguous > > > > >> + </description> > >> + </event> > >> + > >> + <event name="action" since="3"> > >> + <description summary="notify the selected action"> > >> + This event indicates the action selected by the compositor after > >> + matching the source/destination side actions. Only one action (or > >> + none) will be offered here. > >> + > >> + This event can be emitted multiple times during the drag-and-drop > >> + operation, mainly in response to source side changes (through > >> + data_source.set_actions), destination side changes (through > >> + data_offer.set_actions), and as pointer enters/leaves surfaces. > > > > wl_data_source.set-actions and wl_data_offer.set_actions. > > > > and > > > > "..., and as the pointer enter/leaves ...". > > Oops, thanks for spotting > > > > >> + > >> + Compositors may also change the selected action on the fly, mainly > >> + in response to keyboard modifier changes during the drag-and-drop > >> + operation. > >> + > >> + The most recent action received is always the valid one. > >> + > >> + Clients can trigger cursor surface changes from this point, so > >> + they reflect the current action. > > > > Here as well I think it'd be good to add a note about what action should > > be used to perform the operation, i.e. the last action emitted before > > the "dnd_drop_performed" event. Maybe even specify that no events will > > ever be emitted after the "dnd_drop_performed" event. > > > >> </description> > >> + <arg name="dnd_action" type="uint"/> > >> </event> > >> + > >> + <request name="set_actions" since="3"> > >> + <description summary="set the available drag-and-drop actions"> > >> + Sets the actions that the source side client supports for this > >> + operation. This request may trigger a data_source.action event and > >> + data_offer.action events if the compositor needs changing the > > > > wl_data_source.action and wl_data_offer.action. > > > >> + selected action. > >> + </description> > >> + <arg name="dnd_actions" type="uint"/> > >> + </request> > >> </interface> > >> > >> <interface name="wl_data_device" version="3"> > >> @@ -727,6 +816,46 @@ > >> <arg name="id" type="new_id" interface="wl_data_device"/> > >> <arg name="seat" type="object" interface="wl_seat"/> > >> </request> > >> + > >> + <!-- Version 3 additions --> > >> + > >> + <enum name="dnd_action" since="3"> > >> + <description summary="drag and drop actions"> > >> + This is a bitmask of the available/preferred actions in a > >> + drag-and-drop operation. > >> + > >> + The current reserved ranges are: > >> + > >> + 0x0000 - 0x00FF: Reserved for the wayland core protocol. > >> + 0x01FF - 0xFFFF: Reserved for future toolkit-specific use. Slots > >> + may be reserved. > > > > This means we'll have 8 possible core actions, and 24 that may be > > reserved. 8 I suppose is enough for core actions, but is 24 really > > enough for all eternity if they are meant to be allocated by different > > toolkits and applications? If we're to make it reservable it seems to be > > better to make it more long term sustainable, i.e. not making it a bit > > mask but a regular enum. > > > > This means that some of the events and requests would have to use array > > instead of uint, which doesn't seem that bad. > > Uhm, it looks like the matching of actions would be harder then... > > At the expense of sounding too optimistic here, I think this will be > mostly unused. XDND defines the following actions: > > - XdndActionCopy > - XdndActionMove > - XdndActionAsk > - XdndActionLink > - XdndActionPrivate > > It has stayed like that for ~17 years, and I dare say that the latter > 2 are mostly unused. In the end I think that the two things that > matter in the context of DnD are: > > - Whether the destination found the offered data satisfactory > - Whether the source has to signal deletion > > And none/move/copy cover all the combinations of these conditions we > care about (ask being a sort of extension point), everything else IMO > belongs to drag destination app-specific context (thus custom > mimetypes, local-only DnD, etc) > > In that scheme, I think slots should be reserved with real > actions/arguments behind every requested bit, and we should fold into > the core actions anything we see popular enough. Although I'd be just > as happy to drop the action extension blurbs and wait for others to > contend about extra actions in the future :) I think I'd vote for leaving the action extension parts and leave the rest to be undefined and meant for future core additions. In that way, I don't think using a uint is an issue. > > > > >> + > >> + In the compositor, the selected action comes out as a result of > >> + matching the actions offered by the source and destination sides, > > > > Full stop here. > > > >> + "action" events with a "none" action will be sent to both source > >> + and destination if there is no match. All further checks will > >> + effectively happen on (source_actions & dest_actions). > > > > s/&/∩/ (we're using UTF-8 so can just use the unicode character for > > intersection here). > > > > I also think there is no need variable name like things, i.e. I think it > > can just be (source actions ∩ destination actions). > > Sure, didn't know set algebra symbols were preferred here. I suppose it's a personal preference, but as the XML is usually read "raw" and & is hard to read, using immediately readable UTF-8 seems like an improvement. Jonas > > Cheers, > Carlos _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
