GNOME Bugzilla – Bug 735888
dnd drag cancellation animation missing
Last modified: 2016-01-23 04:57:43 UTC
under wayland, this would probably have to be done by the compositor.
what is needed here on the gtk side is to move the client-side drag animation code from gtk+ into the gdk backends, so we can do it under X, but skip it under Wayland.
Created attachment 317067 [details] [review] gdk: Allow passing the start coordinates in drag_begin Add a variant of gdk_drag_begin that takes the start position in addition to the device. All backend implementation have been updated to accept (and ignore) the new arguments. Subsequent commits will make use of the data in some backends.
Created attachment 317068 [details] [review] dnd: Pass start coordinates when creating the drag context This will be used in subsequent commits.
Created attachment 317069 [details] [review] x11: Store drag start coordinates These will be used in later commits.
Created attachment 317070 [details] [review] gdk: Add gdk_drag_drop_done This will allow us to move the drag cancel animation to GDK. For now, it does nothing.
Created attachment 317071 [details] [review] x11: Implement drag cancel animation Showing the drag cancel animation can be done in the X11 drag context implementation now that we hold the drag window there, and have the start coordinates.
Created attachment 317072 [details] [review] dnd: Stop doing cancel animation in GTK+ Under Wayland, the compositor does it, so there is no need for us to move the window ourselves. For X11, we are now doing the animation from the X11 backend. Trigger that by calling gdk_drag_drop_done(). What changes here is that we have to keep the icon_window alive for as long as the drag context exists. Use a weak reference to do so.
Comment on attachment 317067 [details] [review] gdk: Allow passing the start coordinates in drag_begin The approach looks alright to me, I thought first about just passing a GdkEvent so we could eventually shuffle grabs into gdk and have all extra info readily available (device/seat, time/serial...), but that probably shouldn't be done implicitly if we want to maintain the gdk-level API compatible. However, the implicit grabbing behavior would belong into a gdk_drag_begin* kind of call, it might be worth to rush grabs to so we don't end up adding multiple calls here.
Review of attachment 317072 [details] [review]: ::: gtk/gtkdnd.c @@ +3202,3 @@ + /* keep the icon_window alive until the (possible) drag cancel animation is done */ + g_object_set_data_full (G_OBJECT (info->context), "former-gtk-source-info", info, (GDestroyNotify)gtk_drag_source_info_free); Been thinking on this and can't figure out a better way tbh... we can't add a post-animation vfunc to GdkDragContext because x11 is the only place we can know that, and hooking the GtkDragSourceInfo to the icon_window widget being destroyed relies on gdk knowing how to indirectly destroy the widget in the first place. That said, the GtkDragSourceInfo is already previously attached to the context as dest_info_quark, we might as well set up a GDestroyNotify when creating the source info and making gtk_drag_clear_source_info() steal the qdata.
The other patches look alright to me, given the current doings.
the reason I decided to pass x/y instead of the event is the same one that made us introduce gtk_drag_begin_with_coordinates: the event doesn't actually have the right x/y in it, since we've already moved from the starting point to exceed the drag threshold. I guess one could pass the event + offset x/y instead
I've merged the dnd-animation branch now. We can fix up the new api for GdkSeat when it lands.
Whats left here is still to get a drag end event from the compositor.
this has been fixed