Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Wed, 11 Dec 2019 19:05:54 GMT, Thiago Milczarek Sayao wrote: >> With your latest patch, which reverts the changes to `GtkDnDClipboard.java`, >> I now get the DRAG_DONE event, but the following warning printed to the >> console in the case of a `DRAG_DONE` event with a null transfer mode: >> >> GOT A dragExit when dndGesture is null! >> >> This is coming from >> [Scene.java#L2925](https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/java/javafx/scene/Scene.java#L2925). > > My mistake. I saw that and thought it was from the test app. > > Will fix. All my testing looks good now. I tried both of the programs you listed in the description, as well as `tests/manual/dnd/DndTest.java`, on Ubuntu 16.04, Ubuntu 19.04, and Oracle Linux 7.7. I ran with the default gtk-3 implementation as well as gtk-2. I'll do a final code review, probably early next week. This will need a second reviewer. @pankaj-bansal can you also review this. - PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Wed, 11 Dec 2019 18:43:23 GMT, Kevin Rushforth wrote: >> I don't get an assertion error on Oracle Linux 7.7 -- so far only on Ubuntu >> 16.04. I will resume reviewing once you restore the `DRAG_DONE` behavior. > > With your latest patch, which reverts the changes to `GtkDnDClipboard.java`, > I now get the DRAG_DONE event, but the following warning printed to the > console in the case of a `DRAG_DONE` event with a null transfer mode: > > GOT A dragExit when dndGesture is null! > > This is coming from > [Scene.java#L2925](https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/java/javafx/scene/Scene.java#L2925). My mistake. I saw that and thought it was from the test app. Will fix. - PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Wed, 11 Dec 2019 00:21:20 GMT, Kevin Rushforth wrote: >>> Does it send the event on other platforms? I can test on Windows. >> >> Yes, it always sends the `DRAG_DONE` event on all platforms today (I tested >> it on Mac, Windows, and Linux). >> >>> Will look into the assertion error - it does not happen on 18.04. >> >> It doesn't happen on 19.10 either. Only 16.04 as far as I have tested. I'll >> also test on Oracle Linux 7.x. > > I don't get an assertion error on Oracle Linux 7.7 -- so far only on Ubuntu > 16.04. I will resume reviewing once you restore the `DRAG_DONE` behavior. With your latest patch, which reverts the changes to `GtkDnDClipboard.java`, I now get the DRAG_DONE event, but the following warning printed to the console in the case of a `DRAG_DONE` event with a null transfer mode: GOT A dragExit when dndGesture is null! This is coming from [Scene.java#L2925](https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/java/javafx/scene/Scene.java#L2925). - PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Tue, 10 Dec 2019 23:53:27 GMT, Kevin Rushforth wrote: >> @kevinrushforth I changed that behaviour. Since the drag does not complete i >> find it counter-intuitive to send the DRAG_DONE event. >> >> Does it send the event on other platforms? I can test on Windows. >> >> Will look into the assertion error - it does not happen on 18.04. > >> Does it send the event on other platforms? I can test on Windows. > > Yes, it always sends the `DRAG_DONE` event on all platforms today (I tested > it on Mac, Windows, and Linux). > >> Will look into the assertion error - it does not happen on 18.04. > > It doesn't happen on 19.10 either. Only 16.04 as far as I have tested. I'll > also test on Oracle Linux 7.x. I don't get an assertion error on Oracle Linux 7.7 -- so far only on Ubuntu 16.04. I will resume reviewing once you restore the `DRAG_DONE` behavior. - PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Tue, 10 Dec 2019 19:41:05 GMT, Thiago Milczarek Sayao wrote: >> I'm starting to look at this. I see at least one behavioral difference that >> will need to be addressed. With the existing implementation, if I drag / >> drop onto a target that set up to receive the drop, the transfer completes >> with a transfer mode of `null`. With your patch, the transfer never >> completes -- the `DRAG_DONE` event is never sent. >> >> This can be most easily seen by running the `DragDropWithControls` app and >> doing one of the following two things: >> >> 1. Drag from the source rectangle and drop somewhere _other than_ the >> target rectangle, for example, in the status text box at the bottom. >> 2. Change the source data format to `HTML` (uncheck `PLAINTEXT`), leaving >> the target at `PLAINTEXT`, then drag from the source rectangle and drop onto >> the target rectangle >> >> In addition to the drop not happening, I get the following assertion error >> on Ubuntu 16.04: >> >> (java:22328): Gdk-CRITICAL **: gdk_frame_clock_get_frame_time: assertion >> 'GDK_IS_FRAME_CLOCK (frame_clock)' failed >> >> I'm using the default GTK 3 library. > > @kevinrushforth I changed that behaviour. Since the drag does not complete i > find it counter-intuitive to send the DRAG_DONE event. > > Does it send the event on other platforms? I can test on Windows. > > Will look into the assertion error - it does not happen on 18.04. > Does it send the event on other platforms? I can test on Windows. Yes, it always sends the `DRAG_DONE` event on all platforms today (I tested it on Mac, Windows, and Linux). > Will look into the assertion error - it does not happen on 18.04. It doesn't happen on 19.10 either. Only 16.04 as far as I have tested. I'll also test on Oracle Linux 7.x. - PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Tue, 10 Dec 2019 16:13:06 GMT, Kevin Rushforth wrote: >> Just a reminder to take a look when time allows. > > I'm starting to look at this. I see at least one behavioral difference that > will need to be addressed. With the existing implementation, if I drag / drop > onto a target that set up to receive the drop, the transfer completes with a > transfer mode of `null`. With your patch, the transfer never completes -- the > `DRAG_DONE` event is never sent. > > This can be most easily seen by running the `DragDropWithControls` app and > doing one of the following two things: > > 1. Drag from the source rectangle and drop somewhere _other than_ the target > rectangle, for example, in the status text box at the bottom. > 2. Change the source data format to `HTML` (uncheck `PLAINTEXT`), leaving the > target at `PLAINTEXT`, then drag from the source rectangle and drop onto the > target rectangle > > In addition to the drop not happening, I get the following assertion error on > Ubuntu 16.04: > > (java:22328): Gdk-CRITICAL **: gdk_frame_clock_get_frame_time: assertion > 'GDK_IS_FRAME_CLOCK (frame_clock)' failed > > I'm using the default GTK 3 library. @kevinrushforth I changed that behaviour. Since the drag does not complete i find it counter-intuitive to send the DRAG_DONE event. Does it send the event on other platforms? I can test on Windows. Will look into the assertion error - it does not happen on 18.04. - PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Mon, 2 Dec 2019 11:09:05 GMT, Thiago Milczarek Sayao wrote: >> Preliminary review was here: >> [javafxports/openjdk-jfx/pull/490](https://github.com/javafxports/openjdk-jfx/pull/490) > > Just a reminder to take a look when time allows. I'm starting to look at this. I see at least one behavioral difference that will need to be addressed. With the existing implementation, if I drag / drop onto a target that set up to receive the drop, the transfer completes with a transfer mode of `null`. With your patch, the transfer never completes -- the `DRAG_DONE` event is never sent. This can be most easily seen by running the `DragDropWithControls` app and doing one of the following two things: 1. Drag from the source rectangle and drop somewhere _other than_ the target rectangle, for example, in the status text box at the bottom. 2. Change the source data format to `HTML` (uncheck `PLAINTEXT`), leaving the target at `PLAINTEXT`, then drag from the source rectangle and drop onto the target rectangle In addition to the drop not happening, I get the following assertion error on Ubuntu 16.04: (java:22328): Gdk-CRITICAL **: gdk_frame_clock_get_frame_time: assertion 'GDK_IS_FRAME_CLOCK (frame_clock)' failed I'm using the default GTK 3 library. - PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Fri, 4 Oct 2019 23:03:00 GMT, Kevin Rushforth wrote: > On Wed, 2 Oct 2019 20:02:22 GMT, Kevin Rushforth wrote: > >> On Wed, 2 Oct 2019 19:54:50 GMT, Thiago Milczarek Sayao >> wrote: >> >>> On Wed, 2 Oct 2019 19:54:50 GMT, Kevin Rushforth wrote: >>> On Wed, 2 Oct 2019 19:54:48 GMT, Thiago Milczarek Sayao wrote: > https://bugs.openjdk.java.net/browse/JDK-8225571 > > To run tests (on the root of the source tree): > ./gradlew apps > java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar > dragdrop.DragDropWithControls > java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar > dragdrop.DragDropColor > > java -Djdk.gtk.version=2 @build/run.args -cp > apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropWithControls > java -Djdk.gtk.version=2 @build/run.args -cp > apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropColor > > > > Commits: > - 3d650b2b: Gtk DND port for Linux > > Changes: https://git.openjdk.java.net/jfx/pull/4/files > Webrev: https://webrevs.openjdk.java.net/jfx/4/webrev.00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8225571 > Stats: 669 lines in 5 files changed: 81 ins; 442 del; 146 mod > Patch: https://git.openjdk.java.net/jfx/pull/4.diff > Fetch: git fetch https://git.openjdk.java.net/jfx pull/4/head:pull/4 @tsayao In your case `/covered` isn't what's needed. You have an OpenJDK ID so should do this instead: > If you already are an OpenJDK > [Author](https://openjdk.java.net/bylaws#author), > [Committer](https://openjdk.java.net/bylaws#committer) or > [Reviewer](https://openjdk.java.net/bylaws#reviewer), please click > [here](https://bugs.openjdk.java.net/secure/CreateIssue.jspa?pid=11300&issuetype=1) > to open a new issue so that we can record that fact. Please use "Add > GitHub user tsayao" as summary for the issue. >>> >>> Yes, noticed that while reading the comment again. I have opened an issue. >>> Thanks. >> >> @pankaj-bansal can you review this as well? > > Preliminary review was here: > [javafxports/openjdk-jfx/pull/490](https://github.com/javafxports/openjdk-jfx/pull/490) Just a reminder to take a look when time allows. PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Wed, 2 Oct 2019 20:02:22 GMT, Kevin Rushforth wrote: > On Wed, 2 Oct 2019 19:54:50 GMT, Thiago Milczarek Sayao > wrote: > >> On Wed, 2 Oct 2019 19:54:50 GMT, Kevin Rushforth wrote: >> >>> On Wed, 2 Oct 2019 19:54:48 GMT, Thiago Milczarek Sayao >>> wrote: >>> https://bugs.openjdk.java.net/browse/JDK-8225571 To run tests (on the root of the source tree): ./gradlew apps java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropWithControls java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropColor java -Djdk.gtk.version=2 @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropWithControls java -Djdk.gtk.version=2 @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropColor Commits: - 3d650b2b: Gtk DND port for Linux Changes: https://git.openjdk.java.net/jfx/pull/4/files Webrev: https://webrevs.openjdk.java.net/jfx/4/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8225571 Stats: 669 lines in 5 files changed: 81 ins; 442 del; 146 mod Patch: https://git.openjdk.java.net/jfx/pull/4.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/4/head:pull/4 >>> >>> @tsayao In your case `/covered` isn't what's needed. You have an OpenJDK ID >>> so should do this instead: >>> If you already are an OpenJDK [Author](https://openjdk.java.net/bylaws#author), [Committer](https://openjdk.java.net/bylaws#committer) or [Reviewer](https://openjdk.java.net/bylaws#reviewer), please click [here](https://bugs.openjdk.java.net/secure/CreateIssue.jspa?pid=11300&issuetype=1) to open a new issue so that we can record that fact. Please use "Add GitHub user tsayao" as summary for the issue. >> >> Yes, noticed that while reading the comment again. I have opened an issue. >> Thanks. > > @pankaj-bansal can you review this as well? Preliminary review was here: [javafxports/openjdk-jfx/pull/490](https://github.com/javafxports/openjdk-jfx/pull/490) PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Wed, 2 Oct 2019 19:54:50 GMT, Thiago Milczarek Sayao wrote: > On Wed, 2 Oct 2019 19:54:50 GMT, Kevin Rushforth wrote: > >> On Wed, 2 Oct 2019 19:54:48 GMT, Thiago Milczarek Sayao >> wrote: >> >>> https://bugs.openjdk.java.net/browse/JDK-8225571 >>> >>> To run tests (on the root of the source tree): >>> ./gradlew apps >>> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar >>> dragdrop.DragDropWithControls >>> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar >>> dragdrop.DragDropColor >>> >>> java -Djdk.gtk.version=2 @build/run.args -cp >>> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropWithControls >>> java -Djdk.gtk.version=2 @build/run.args -cp >>> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropColor >>> >>> >>> >>> Commits: >>> - 3d650b2b: Gtk DND port for Linux >>> >>> Changes: https://git.openjdk.java.net/jfx/pull/4/files >>> Webrev: https://webrevs.openjdk.java.net/jfx/4/webrev.00 >>> Issue: https://bugs.openjdk.java.net/browse/JDK-8225571 >>> Stats: 669 lines in 5 files changed: 81 ins; 442 del; 146 mod >>> Patch: https://git.openjdk.java.net/jfx/pull/4.diff >>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/4/head:pull/4 >> >> @tsayao In your case `/covered` isn't what's needed. You have an OpenJDK ID >> so should do this instead: >> >>> If you already are an OpenJDK >>> [Author](https://openjdk.java.net/bylaws#author), >>> [Committer](https://openjdk.java.net/bylaws#committer) or >>> [Reviewer](https://openjdk.java.net/bylaws#reviewer), please click >>> [here](https://bugs.openjdk.java.net/secure/CreateIssue.jspa?pid=11300&issuetype=1) >>> to open a new issue so that we can record that fact. Please use "Add >>> GitHub user tsayao" as summary for the issue. > > Yes, noticed that while reading the comment again. I have opened an issue. > Thanks. @pankaj-bansal can you review this as well? PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Wed, 2 Oct 2019 19:54:50 GMT, Kevin Rushforth wrote: > On Wed, 2 Oct 2019 19:54:48 GMT, Thiago Milczarek Sayao > wrote: > >> https://bugs.openjdk.java.net/browse/JDK-8225571 >> >> To run tests (on the root of the source tree): >> ./gradlew apps >> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar >> dragdrop.DragDropWithControls >> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar >> dragdrop.DragDropColor >> >> java -Djdk.gtk.version=2 @build/run.args -cp >> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropWithControls >> java -Djdk.gtk.version=2 @build/run.args -cp >> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropColor >> >> >> >> Commits: >> - 3d650b2b: Gtk DND port for Linux >> >> Changes: https://git.openjdk.java.net/jfx/pull/4/files >> Webrev: https://webrevs.openjdk.java.net/jfx/4/webrev.00 >> Issue: https://bugs.openjdk.java.net/browse/JDK-8225571 >> Stats: 669 lines in 5 files changed: 81 ins; 442 del; 146 mod >> Patch: https://git.openjdk.java.net/jfx/pull/4.diff >> Fetch: git fetch https://git.openjdk.java.net/jfx pull/4/head:pull/4 > > @tsayao In your case `/covered` isn't what's needed. You have an OpenJDK ID > so should do this instead: > >> If you already are an OpenJDK >> [Author](https://openjdk.java.net/bylaws#author), >> [Committer](https://openjdk.java.net/bylaws#committer) or >> [Reviewer](https://openjdk.java.net/bylaws#reviewer), please click >> [here](https://bugs.openjdk.java.net/secure/CreateIssue.jspa?pid=11300&issuetype=1) >> to open a new issue so that we can record that fact. Please use "Add GitHub >> user tsayao" as summary for the issue. Yes, noticed that while reading the comment again. I have opened an issue. Thanks. PR: https://git.openjdk.java.net/jfx/pull/4
Re: RFR: 8225571: Port DND source to use GTK instead of GDK
On Wed, 2 Oct 2019 19:54:48 GMT, Thiago Milczarek Sayao wrote: > https://bugs.openjdk.java.net/browse/JDK-8225571 > > To run tests (on the root of the source tree): > ./gradlew apps > java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar > dragdrop.DragDropWithControls > java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar > dragdrop.DragDropColor > > java -Djdk.gtk.version=2 @build/run.args -cp > apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropWithControls > java -Djdk.gtk.version=2 @build/run.args -cp > apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropColor > > > > Commits: > - 3d650b2b: Gtk DND port for Linux > > Changes: https://git.openjdk.java.net/jfx/pull/4/files > Webrev: https://webrevs.openjdk.java.net/jfx/4/webrev.00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8225571 > Stats: 669 lines in 5 files changed: 81 ins; 442 del; 146 mod > Patch: https://git.openjdk.java.net/jfx/pull/4.diff > Fetch: git fetch https://git.openjdk.java.net/jfx pull/4/head:pull/4 @tsayao In your case `/covered` isn't what's needed. You have an OpenJDK ID so should do this instead: > If you already are an OpenJDK > [Author](https://openjdk.java.net/bylaws#author), > [Committer](https://openjdk.java.net/bylaws#committer) or > [Reviewer](https://openjdk.java.net/bylaws#reviewer), please click > [here](https://bugs.openjdk.java.net/secure/CreateIssue.jspa?pid=11300&issuetype=1) > to open a new issue so that we can record that fact. Please use "Add GitHub > user tsayao" as summary for the issue. PR: https://git.openjdk.java.net/jfx/pull/4
