From 833b56494692aa683144259852dd960691f6f033 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sat, 22 Feb 2020 17:44:36 -0500 Subject: [PATCH 1/5] x11: Avoid crashes in dnd We were forgetting to clean up the ::xevent signal handler in some error cases. Move the signal connection later, when we know the drag is going forward, and use g_signal_connect_object to make sure the signal handler is not forgotten. --- gdk/x11/gdkdrag-x11.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gdk/x11/gdkdrag-x11.c b/gdk/x11/gdkdrag-x11.c index c29304e818..48ad2b6862 100644 --- a/gdk/x11/gdkdrag-x11.c +++ b/gdk/x11/gdkdrag-x11.c @@ -1714,8 +1714,8 @@ gdk_x11_drag_default_output_handler (GOutputStream *stream, static gboolean gdk_x11_drag_xevent (GdkDisplay *display, - const XEvent *xevent, - gpointer data) + const XEvent *xevent, + gpointer data) { GdkDrag *drag = GDK_DRAG (data); GdkX11Drag *x11_drag = GDK_X11_DRAG (drag); @@ -1876,8 +1876,8 @@ gdk_x11_drag_release_selection (GdkDrag *drag) } static void -gdk_x11_drag_drop_done (GdkDrag *drag, - gboolean success) +gdk_x11_drag_drop_done (GdkDrag *drag, + gboolean success) { GdkX11Drag *x11_drag = GDK_X11_DRAG (drag); GdkDragAnim *anim; @@ -2072,8 +2072,6 @@ _gdk_x11_surface_drag_begin (GdkSurface *surface, NULL); x11_drag = GDK_X11_DRAG (drag); - g_signal_connect (display, "xevent", G_CALLBACK (gdk_x11_drag_xevent), drag); - precache_target_list (drag); gdk_device_get_position (device, &px, &py); @@ -2115,6 +2113,8 @@ _gdk_x11_surface_drag_begin (GdkSurface *surface, return NULL; } + g_signal_connect_object (display, "xevent", G_CALLBACK (gdk_x11_drag_xevent), drag, 0); + return drag; } From f93d0f8fb5e6660126e5b4c35b2b9d8d5f189816 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sat, 22 Feb 2020 18:32:37 -0500 Subject: [PATCH 2/5] x11: Keep a ref on GdkDrag objects It is expected that backends keep a ref on the GdkDrag objects that they create as long as the drag is ongoing. --- gdk/x11/gdkdrag-x11.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/gdk/x11/gdkdrag-x11.c b/gdk/x11/gdkdrag-x11.c index 48ad2b6862..c2bd36682a 100644 --- a/gdk/x11/gdkdrag-x11.c +++ b/gdk/x11/gdkdrag-x11.c @@ -900,16 +900,12 @@ gdk_x11_drag_handle_finished (GdkDisplay *display, if (drag) { - g_object_ref (drag); - drag_x11 = GDK_X11_DRAG (drag); if (drag_x11->version == 5) drag_x11->drop_failed = xevent->xclient.data.l[1] == 0; g_signal_emit_by_name (drag, "dnd-finished"); gdk_drag_drop_done (drag, !drag_x11->drop_failed); - - g_object_unref (drag); } } @@ -1896,6 +1892,7 @@ gdk_x11_drag_drop_done (GdkDrag *drag, if (success) { gdk_surface_hide (x11_drag->drag_surface); + g_object_unref (drag); return; } @@ -1928,6 +1925,7 @@ gdk_x11_drag_drop_done (GdkDrag *drag, gdk_drag_anim_timeout, anim, (GDestroyNotify) gdk_drag_anim_destroy); g_source_set_name_by_id (id, "[gtk] gdk_drag_anim_timeout"); + g_object_unref (drag); } static gboolean @@ -2097,7 +2095,7 @@ _gdk_x11_surface_drag_begin (GdkSurface *surface, g_object_unref (drag); return NULL; } - + move_drag_surface (drag, x_root, y_root); x11_drag->timestamp = gdk_display_get_last_seen_time (display); @@ -2113,7 +2111,10 @@ _gdk_x11_surface_drag_begin (GdkSurface *surface, return NULL; } + g_signal_connect_object (display, "xevent", G_CALLBACK (gdk_x11_drag_xevent), drag, 0); + /* backend holds a ref until gdk_drag_drop_done is called */ + g_object_ref (drag); return drag; } From 14122d1acbcbc43e093758df17409b1564dc81bb Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sat, 22 Feb 2020 19:09:18 -0500 Subject: [PATCH 3/5] x11: Export gdk_x11_surface_get_root_coords privately This lets us avoid a roundtrip through the surface vfuncs. --- gdk/x11/gdkprivate-x11.h | 6 ++++++ gdk/x11/gdksurface-x11.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/gdk/x11/gdkprivate-x11.h b/gdk/x11/gdkprivate-x11.h index 0f8ca87765..69a1248b8a 100644 --- a/gdk/x11/gdkprivate-x11.h +++ b/gdk/x11/gdkprivate-x11.h @@ -230,6 +230,12 @@ GdkDrag * _gdk_x11_surface_drag_begin (GdkSurface *window, gint dx, gint dy); +void gdk_x11_surface_get_root_coords (GdkSurface *surface, + gint x, + gint y, + gint *root_x, + gint *root_y); + GdkGrabStatus _gdk_x11_convert_grab_status (gint status); cairo_surface_t * _gdk_x11_display_create_bitmap_surface (GdkDisplay *display, diff --git a/gdk/x11/gdksurface-x11.c b/gdk/x11/gdksurface-x11.c index 7c4ce1199e..8d052e9d3d 100644 --- a/gdk/x11/gdksurface-x11.c +++ b/gdk/x11/gdksurface-x11.c @@ -2402,7 +2402,7 @@ gdk_x11_surface_get_geometry (GdkSurface *surface, } } -static void +void gdk_x11_surface_get_root_coords (GdkSurface *surface, gint x, gint y, From 7c1cfc5533d68655b001936168d8a5c2a6f7622b Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sat, 22 Feb 2020 18:44:55 -0500 Subject: [PATCH 4/5] x11: Fix dnd coordinate handling We were not properly converting the coordinates we got to root coordinates. This was showing up as offsets between the actual drop target and the area where drops can happen, e.g. when dragging over a stack switcher to switch pages. --- gdk/x11/gdkdrag-x11.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gdk/x11/gdkdrag-x11.c b/gdk/x11/gdkdrag-x11.c index c2bd36682a..04fa753fa3 100644 --- a/gdk/x11/gdkdrag-x11.c +++ b/gdk/x11/gdkdrag-x11.c @@ -2073,8 +2073,12 @@ _gdk_x11_surface_drag_begin (GdkSurface *surface, precache_target_list (drag); gdk_device_get_position (device, &px, &py); - x_root = round (px) + dx; - y_root = round (py) + dy; + + gdk_x11_surface_get_root_coords (surface, + round (px) + dx, + round (py) + dy, + &x_root, + &y_root); x11_drag->start_x = x_root; x11_drag->start_y = y_root; From ec383a238842b5e50a4e8008c6e59ab5b7158dc7 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sat, 22 Feb 2020 18:50:28 -0500 Subject: [PATCH 5/5] Add detail to gdk_drag_begin docs Mention that GTK keeps a reference while the drag operation is ongoing. --- gdk/gdksurface.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gdk/gdksurface.c b/gdk/gdksurface.c index 399e40c368..13b0dec48d 100644 --- a/gdk/gdksurface.c +++ b/gdk/gdksurface.c @@ -3587,6 +3587,10 @@ gdk_surface_register_dnd (GdkSurface *surface) * probably want to set up the drag icon using the surface returned * by gdk_drag_get_drag_surface(). * + * This function returns a reference to the GdkDrag object, but GTK + * keeps its own reference as well, as long as the DND operation is + * going on. + * * Note: if @actions include %GDK_ACTION_MOVE, you need to listen for * the #GdkDrag::dnd-finished signal and delete the data at the source * if gdk_drag_get_selected_action() returns %GDK_ACTION_MOVE.