On Sun, 12 Mar 2023 23:04:19 GMT, Thiago Milczarek Sayao <tsa...@openjdk.org> 
wrote:

>> This PR fixes 8273379.
>> 
>> I reverted back to use GDK (from 
>> [8225571](https://bugs.openjdk.org/browse/JDK-8225571)) to handle the 
>> events. 
>> 
>> It may also fix [8280383](https://bugs.openjdk.org/browse/JDK-8280383).
>> 
>> There's also some cleaup.
>> 
>> To do general testing (two tests were added):
>> `java @build/run.args -jar apps/toys/DragDrop/dist/DragDrop.jar`
>> 
>> Information for reviewing:
>> * Previously an offscreen window where used to pass events. Now it gets the 
>> window were Drag initially started  
>> (`WindowContextBase::sm_mouse_drag_window`);
>> * There's a new `DragSourceContext` instead of global variables;
>> * DragView were simplified;
>> * It handles `GDK_GRAB_BROKEN` events (I still need to figure it out a test 
>> case for this - It should happen when another window grabs the device during 
>> the drag);
>> * There's a special case for `GDK_BUTTON_RELEASE` because `WindowContext` 
>> will notify java about the button release and set `DnDGesture` to null 
>> before the end of the DND.
>> * `gdk_drag_find_window_for_screen` -> pass the DragView window to be 
>> ignored (as it would "steal" destination motion events);
>> * The Scenario were the drag source window closes during the drag is now 
>> covered;
>> * It does not rely on `gdk_threads_add_idle` because it may be inconsistent.
>> 
>> 
>> ![image](https://user-images.githubusercontent.com/30704286/213877115-18f274ff-18c9-4d38-acc4-449f24174ecc.png)
>> ![image](https://user-images.githubusercontent.com/30704286/213877140-1d24c293-d70f-46e6-b040-c49170d2aa9a.png)
>
> Thiago Milczarek Sayao has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Don't call gdk_device_manager_get_client_pointer() on event processing

modules/javafx.graphics/src/main/native-glass/gtk/glass_dnd.cpp line 780:

> 778:         dnd_set_performed_action(translate_gdk_action_to_glass(action));
> 779:     }
> 780:     gdk_threads_add_idle((GSourceFunc) dnd_destroy_drag_widget_callback, 
> NULL);

This could lead to inconsistent results.

modules/javafx.graphics/src/main/native-glass/gtk/glass_dnd.cpp line 789:

> 787: {
> 788:     
> dnd_set_performed_action(com_sun_glass_ui_gtk_GtkDnDClipboard_ACTION_NONE);
> 789:     gdk_threads_add_idle((GSourceFunc) dnd_destroy_drag_widget_callback, 
> NULL);

May cause problems.

modules/javafx.graphics/src/main/native-glass/gtk/glass_dnd.cpp line 834:

> 832:     // be destroyed on drag end
> 833:     drag_widget = gtk_window_new(GTK_WINDOW_POPUP);
> 834:     gtk_window_resize(GTK_WINDOW(drag_widget), 1, 1);

This was hacky.

modules/javafx.graphics/src/main/native-glass/gtk/glass_dnd.cpp line 879:

> 877:     if (cursor == NULL) {
> 878:         cursor = gdk_cursor_new_from_name(gdk_display_get_default(), 
> "dnd-none");
> 879:     }

Those cursor works on Ubuntu 16.04+ (probably before) and are consistent with 
the ones used on gnome.

modules/javafx.graphics/src/main/native-glass/gtk/glass_dnd.cpp line 937:

> 935:         ERROR0("Drag not started on source window.");
> 936:         return;
> 937:     }

Instead of using a offscreen window to pass events, it uses the window were the 
drag started, which seems the intended way.

modules/javafx.graphics/src/main/native-glass/gtk/glass_dnd.cpp line 981:

> 979:                                          "text-x-generic", w,
> 980:                                          GTK_ICON_LOOKUP_USE_BUILTIN,
> 981:                                          &error);

This adds the default dnd icon if no dragview is present.

modules/javafx.graphics/src/main/native-glass/gtk/glass_dnd.cpp line 1024:

> 1022:         case GDK_GRAB_BROKEN:
> 1023:             process_dnd_source_grab_broken(ctx, event);
> 1024:             break;

This was added to consider the case when a mouse grab is broken, I just could 
not think when it happens - but it's present on gdk managed drag and drop code.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/986#discussion_r1167668422
PR Review Comment: https://git.openjdk.org/jfx/pull/986#discussion_r1167668911
PR Review Comment: https://git.openjdk.org/jfx/pull/986#discussion_r1167668780
PR Review Comment: https://git.openjdk.org/jfx/pull/986#discussion_r1167668852
PR Review Comment: https://git.openjdk.org/jfx/pull/986#discussion_r1167668226
PR Review Comment: https://git.openjdk.org/jfx/pull/986#discussion_r1167668562
PR Review Comment: https://git.openjdk.org/jfx/pull/986#discussion_r1167668369

Reply via email to