From 1234b88410357ebd6cb1a32444e73345480b079b Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Sat, 9 Jul 2022 03:52:58 +0200 Subject: [PATCH] Issue #247 - [GTK][Wayland] Drag&drop gets stuck The problem was introduced in patch for Bug 541635, which accidentally ignored calculated `dragging` variable, causing drag to begin immediately when mouse is moved. Also, it introduced a crazy loop, which, if it proceeds to second iteration, will loop forever, because GTK events can't be processed when main thread is busy running the infinite loop. Luckily, it seems that the loop always exited on first iteration, and its meaning was simply to `return false` when mouse is not down. The reason why drag&drop can become stuck is not exactly clear, but I'm confident that it was related to very short drag&drop mouse move threshold (basically, 1px). With the regular threshold, the bug may still be present, but it's almost impossible to reproduce. The real case behind this bug is that when actual human double-clicks a TreeItem, sometimes mouse will be moved very slightly when doing so, which started a drag&drop, which got stuck and caused further problems in applications which were puzzled by never ending drag&drop. Also, it results in `TreeDragSourceEffect` image leak, because it never receives the drag end event. Signed-off-by: Alexandr Miloslavskiy --- .../gtk/org/eclipse/swt/widgets/Control.java | 44 ++++------ .../Issue0247_WaylandDragDropGetsStuck.java | 80 +++++++++++++++++++ 2 files changed, 97 insertions(+), 27 deletions(-) create mode 100644 tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Issue0247_WaylandDragDropGetsStuck.java diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java index 6845bff3a60..c00d34d81e8 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java @@ -113,7 +113,6 @@ public abstract class Control extends Widget implements Drawable { * is currently pressed or released for DND. */ static boolean mouseDown; - boolean dragBegun; /** * Flag to check the scale factor upon the first drawing of this Control. @@ -2671,36 +2670,36 @@ boolean dragDetect (int button, int count, int stateMask, int x, int y) { } boolean dragDetect (int x, int y, boolean filter, boolean dragOnTimeout, boolean [] consume) { - boolean dragging = false; /* * Feature in GTK: In order to support both X.11/Wayland, GTKGestures are used * as of GTK3.14 in order to acquire mouse position offsets to decide on dragging. * See Bug 503431. */ if (!OS.isX11()) { // Wayland + // Don't drag if mouse is not down. This condition is not as + // trivial as it seems, see Bug 541635 where drag is tested + // after drag already completed and mouse is released. + if (!mouseDown) { + return false; + } + double [] offsetX = new double[1]; double [] offsetY = new double [1]; double [] startX = new double[1]; double [] startY = new double [1]; - if (GTK.gtk_gesture_drag_get_start_point(dragGesture, startX, startY)) { - GTK.gtk_gesture_drag_get_offset(dragGesture, offsetX, offsetY); - if (GTK3.gtk_drag_check_threshold(handle, (int)startX[0], (int) startY[0], (int) startX[0] - + (int) offsetX[0], (int) startY[0] + (int) offsetY[0])) { - dragging = true; - } - } else { + if (!GTK.gtk_gesture_drag_get_start_point(dragGesture, startX, startY)) { return false; } - // Block until mouse was released or drag was detected, see Bug 515396. - while (true) { - if (!mouseDown) { - return false; - } - if (dragBegun) { - return true; - } + + GTK.gtk_gesture_drag_get_offset(dragGesture, offsetX, offsetY); + if (GTK3.gtk_drag_check_threshold(handle, (int)startX[0], (int) startY[0], (int) startX[0] + + (int) offsetX[0], (int) startY[0] + (int) offsetY[0])) { + return true; } + + return false; } else { + boolean dragging = false; boolean quit = false; //428852 DND workaround for GTK3. //Gtk3 no longer sends motion events on the same control during thread sleep @@ -2801,8 +2800,8 @@ boolean dragDetect (int x, int y, boolean filter, boolean dragOnTimeout, boolean } gdk_event_free (eventPtr); } + return dragging; } - return dragging; } boolean filterKey (long event) { @@ -3429,7 +3428,6 @@ void gtk_style_context_get_border (long context, int state, GtkBorder padding) { @Override void gtk_gesture_press_event (long gesture, int n_press, double x, double y, long event) { mouseDown = true; - dragBegun = false; int eventButton = GDK.gdk_button_event_get_button(event); int eventTime = GDK.gdk_event_get_time(event); @@ -3473,7 +3471,6 @@ long gtk_button_press_event (long widget, long event) { long gtk_button_press_event (long widget, long event, boolean sendMouseDown) { mouseDown = true; - dragBegun = false; double [] eventX = new double [1]; double [] eventY = new double [1]; @@ -4109,10 +4106,6 @@ long gtk_mnemonic_activate (long widget, long arg1) { @Override void gtk4_motion_event(long controller, double x, double y, long event) { - if (mouseDown) { - dragBegun = true; - } - if (this == display.currentControl && (hooks(SWT.MouseHover) || filters(SWT.MouseHover))) { display.addMouseHoverTimeout(handle); } @@ -4143,9 +4136,6 @@ void gtk4_motion_event(long controller, double x, double y, long event) { @Override long gtk_motion_notify_event (long widget, long event) { int result; - if (mouseDown) { - dragBegun = true; - } double[] eventX = new double[1]; double[] eventY = new double[1]; diff --git a/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Issue0247_WaylandDragDropGetsStuck.java b/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Issue0247_WaylandDragDropGetsStuck.java new file mode 100644 index 00000000000..5b4ebce1084 --- /dev/null +++ b/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Issue0247_WaylandDragDropGetsStuck.java @@ -0,0 +1,80 @@ +/******************************************************************************* + * Copyright (c) 2022 Syntevo and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Syntevo - initial API and implementation + *******************************************************************************/ + +package org.eclipse.swt.tests.manual; + +import org.eclipse.swt.SWT; +import org.eclipse.swt.dnd.*; +import org.eclipse.swt.layout.*; +import org.eclipse.swt.widgets.*; + +public class Issue0247_WaylandDragDropGetsStuck { + public static void main(String[] args) { + final Display display = new Display (); + + final Shell shell = new Shell (display); + shell.setLayout (new GridLayout (1, true)); + + Label hint = new Label (shell, 0); + hint.setText ( + "1) Use Linux with Wayland\n" + + "2) Drag TreeItem\n" + + "3) Issue 0247: Dragging begins immediately when mouse is moved without any move threshold\n" + + "4) Move mouse over TreeItem slowly and double-click once in a while\n" + + "5) Issue 0247: Sometimes, counter will grow, indicating that there's an unfinished drag.\n" + + " You will also see it by DRAGGING indicator being stuck.\n" + ); + + Tree control = new Tree (shell, SWT.BORDER); + for (int i = 0; i < 5; i++) { + new TreeItem(control, 0).setText("TreeItem #" + i); + } + + Label labelIsDrag = new Label(shell, 0); + labelIsDrag.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); + + final int[] numActiveDrags = new int[1]; + DragSource dragSource = new DragSource (control, DND.DROP_MOVE | DND.DROP_COPY); + dragSource.setTransfer (TextTransfer.getInstance ()); + dragSource.addDragListener (new DragSourceListener() { + @Override + public void dragStart(DragSourceEvent event) { + labelIsDrag.setText("DRAGGING"); + numActiveDrags[0]++; + } + + @Override + public void dragSetData(DragSourceEvent event) { + event.data = "Data"; + } + + @Override + public void dragFinished(DragSourceEvent event) { + numActiveDrags[0]--; + labelIsDrag.setText("" + numActiveDrags[0]); + } + }); + + shell.pack (); + shell.open (); + + while (!shell.isDisposed ()) { + if (!display.readAndDispatch ()) { + display.sleep (); + } + } + + display.dispose (); + } +} \ No newline at end of file