Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

audgui: Make drag and drop work on Wayland. Closes: #1423 #1425

Merged

Conversation

radioactiveman
Copy link
Member

Calling both gtk_drag_get_data() and gtk_drag_finish() leads to drag and drop not working on Wayland and this error: "error reading selection buffer: Operation was cancelled"

See also: https://gitlab.gnome.org/GNOME/gimp/-/merge_requests/927

@radioactiveman
Copy link
Member Author

radioactiveman commented Jul 8, 2024

@jlindgren90: Do you consider this a valid fix? At least it works on X11 and Wayland. I have also tried to call gtk_drag_finish() in drag_data_received() (as mentioned in the GTK docs). But it looks like we still need to call it in drag_drop() for the "dragging within same list" if-branch. So also no clean solution.

Edit: The patch below looks cleaner to me and works too. The only confusing thing is that we can't use dropped = true; in the "dragging within same list" if-branch.

diff --git a/src/libaudgui/list.cc b/src/libaudgui/list.cc
index 6bbece5a7..141e03196 100644
--- a/src/libaudgui/list.cc
+++ b/src/libaudgui/list.cc
@@ -487,7 +487,7 @@ static gboolean drag_drop (GtkWidget * widget, GdkDragContext * context, int x,
 {
     g_signal_stop_emission_by_name (widget, "drag-drop");
 
-    gboolean success = true;
+    gboolean dropped = false;
     int row = audgui_list_row_at_point_rounded (widget, x, y);
 
     if (model->dragging && MODEL_HAS_CB (model, shift_rows))
@@ -495,8 +495,6 @@ static gboolean drag_drop (GtkWidget * widget, GdkDragContext * context, int x,
         /* dragging within same list */
         if (model->clicked_row >= 0 && model->clicked_row < model->rows)
             model->cbs->shift_rows (model->user, model->clicked_row, row);
-        else
-            success = false;
     }
     else if (MODEL_HAS_CB (model, data_type) && MODEL_HAS_CB (model, receive_data))
     {
@@ -504,14 +502,12 @@ static gboolean drag_drop (GtkWidget * widget, GdkDragContext * context, int x,
         model->receive_row = row;
         gtk_drag_get_data (widget, context, gdk_atom_intern
          (model->cbs->data_type, false), time);
+        dropped = true;
     }
-    else
-        success = false;
 
-    gtk_drag_finish (context, success, false, time);
     gtk_tree_view_set_drag_dest_row ((GtkTreeView *) widget, nullptr, (GtkTreeViewDropPosition) 0);
     stop_autoscroll (model, widget);
-    return true;
+    return dropped;
 }
 
 static void drag_data_received (GtkWidget * widget, GdkDragContext * context, int x,
@@ -524,11 +520,14 @@ static void drag_data_received (GtkWidget * widget, GdkDragContext * context, in
 
     auto data = (const char *) gtk_selection_data_get_data (sel);
     int length = gtk_selection_data_get_length (sel);
+    gboolean success = (data && length);
 
-    if (data && length)
+    if (success)
         model->cbs->receive_data (model->user, model->receive_row, data, length);
 
     model->receive_row = -1;
+
+    gtk_drag_finish (context, success, false, time);
 }
 
 /* ==== PUBLIC FUNCS ==== */

@jlindgren90
Copy link
Member

jlindgren90 commented Jul 8, 2024

I don't really understand why the existing code doesn't work. Nevertheless, how about this:

diff --git a/src/libaudgui/list.cc b/src/libaudgui/list.cc
index 6bbece5a7..7e465c6b6 100644
--- a/src/libaudgui/list.cc
+++ b/src/libaudgui/list.cc
@@ -487,16 +487,16 @@ static gboolean drag_drop (GtkWidget * widget, GdkDragContext * context, int x,
 {
     g_signal_stop_emission_by_name (widget, "drag-drop");
 
-    gboolean success = true;
     int row = audgui_list_row_at_point_rounded (widget, x, y);
 
     if (model->dragging && MODEL_HAS_CB (model, shift_rows))
     {
         /* dragging within same list */
-        if (model->clicked_row >= 0 && model->clicked_row < model->rows)
+        gboolean valid = (model->clicked_row >= 0 && model->clicked_row < model->rows);
+        if (valid)
             model->cbs->shift_rows (model->user, model->clicked_row, row);
-        else
-            success = false;
+
+        gtk_drag_finish (context, valid, false, time);
     }
     else if (MODEL_HAS_CB (model, data_type) && MODEL_HAS_CB (model, receive_data))
     {
@@ -504,11 +504,14 @@ static gboolean drag_drop (GtkWidget * widget, GdkDragContext * context, int x,
         model->receive_row = row;
         gtk_drag_get_data (widget, context, gdk_atom_intern
          (model->cbs->data_type, false), time);
+        /* gtk_drag_finish() is called from drag_data_received() */
     }
     else
-        success = false;
+    {
+        /* invalid */
+        gtk_drag_finish (context, false, false, time);
+    }
 
-    gtk_drag_finish (context, success, false, time);
     gtk_tree_view_set_drag_dest_row ((GtkTreeView *) widget, nullptr, (GtkTreeViewDropPosition) 0);
     stop_autoscroll (model, widget);
     return true;
@@ -524,10 +527,12 @@ static void drag_data_received (GtkWidget * widget, GdkDragContext * context, in
 
     auto data = (const char *) gtk_selection_data_get_data (sel);
     int length = gtk_selection_data_get_length (sel);
+    gboolean valid = (data && length);
 
-    if (data && length)
+    if (valid)
         model->cbs->receive_data (model->user, model->receive_row, data, length);
 
+    gtk_drag_finish (context, valid, false, time);
     model->receive_row = -1;
 }
 

…layer#1423

Calling both gtk_drag_get_data() and gtk_drag_finish()
leads to drag and drop not working on Wayland and this error:
"error reading selection buffer: Operation was cancelled"

Move the gtk_drag_finish() call to the "drag-data-received"
handler which gets triggered by calling gtk_drag_get_data()
to fix this.

See also:
- https://gitlab.gnome.org/GNOME/gimp/-/merge_requests/927
- https://docs.gtk.org/gtk3/signal.Widget.drag-drop.html

Co-authored-by: John Lindgren <[email protected]>
@radioactiveman
Copy link
Member Author

Thanks John, your suggestion works as well. 👍

@radioactiveman radioactiveman merged commit 005eae0 into audacious-media-player:master Jul 8, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants