After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 735888 - dnd drag cancellation animation missing
dnd drag cancellation animation missing
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
unspecified
Other Linux
: High critical
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on: 735850
Blocks:
 
 
Reported: 2014-09-02 13:34 UTC by Matthias Clasen
Modified: 2016-01-23 04:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gdk: Allow passing the start coordinates in drag_begin (11.45 KB, patch)
2015-12-09 19:36 UTC, Matthias Clasen
committed Details | Review
dnd: Pass start coordinates when creating the drag context (2.81 KB, patch)
2015-12-09 19:36 UTC, Matthias Clasen
committed Details | Review
x11: Store drag start coordinates (1.10 KB, patch)
2015-12-09 19:36 UTC, Matthias Clasen
committed Details | Review
gdk: Add gdk_drag_drop_done (2.30 KB, patch)
2015-12-09 19:37 UTC, Matthias Clasen
committed Details | Review
x11: Implement drag cancel animation (3.58 KB, patch)
2015-12-09 19:37 UTC, Matthias Clasen
committed Details | Review
dnd: Stop doing cancel animation in GTK+ (6.21 KB, patch)
2015-12-09 19:37 UTC, Matthias Clasen
committed Details | Review

Description Matthias Clasen 2014-09-02 13:34:44 UTC
under wayland, this would probably have to be done by the compositor.
Comment 1 Matthias Clasen 2015-12-03 15:26:09 UTC
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.
Comment 2 Matthias Clasen 2015-12-09 19:36:50 UTC
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.
Comment 3 Matthias Clasen 2015-12-09 19:36:55 UTC
Created attachment 317068 [details] [review]
dnd: Pass start coordinates when creating the drag context

This will be used in subsequent commits.
Comment 4 Matthias Clasen 2015-12-09 19:36:59 UTC
Created attachment 317069 [details] [review]
x11: Store drag start coordinates

These will be used in later commits.
Comment 5 Matthias Clasen 2015-12-09 19:37:04 UTC
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.
Comment 6 Matthias Clasen 2015-12-09 19:37:08 UTC
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.
Comment 7 Matthias Clasen 2015-12-09 19:37:13 UTC
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 8 Carlos Garnacho 2015-12-10 14:02:24 UTC
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.
Comment 9 Carlos Garnacho 2015-12-10 14:03:16 UTC
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.
Comment 10 Carlos Garnacho 2015-12-10 14:03:53 UTC
The other patches look alright to me, given the current doings.
Comment 11 Matthias Clasen 2015-12-10 15:22:29 UTC
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
Comment 12 Matthias Clasen 2015-12-13 21:06:39 UTC
I've merged the dnd-animation branch now. We can fix up the new api for GdkSeat when it lands.
Comment 13 Matthias Clasen 2015-12-14 13:25:08 UTC
Whats left here is still to get a drag end event from the compositor.
Comment 14 Matthias Clasen 2016-01-23 04:57:43 UTC
this has been fixed