Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-02-25 Thread Jose Pereda
On Sat, 24 Feb 2024 17:34:54 GMT, Thiago Milczarek Sayao  
wrote:

>> I've just reverted the previous changes, and just applied the touch mask to 
>> the `gdk_pointer_grab` function. Tests should be green now.
>
> @jperedadnr Would you confirm that scroll on a touch screen still works on 
> Xorg ?
> 
> My tests looks all good (manual and system/robot).

@tsayao yes, I've double checked again. On Wayland and Xorg, I can scroll with 
touch events on a touch screen with this fix.
The only issue I still see on Wayland: the mouse cursor location doesn't get 
updated from touch events, only from mouse events (I guess this can be a 
follow-up issue).

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1963041414


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-02-24 Thread Thiago Milczarek Sayao
On Tue, 20 Feb 2024 12:13:31 GMT, Jose Pereda  wrote:

>> Jose Pereda has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Add compile-time checks to GdkSeat
>
> I've just reverted the previous changes, and just applied the touch mask to 
> the `gdk_pointer_grab` function. Tests should be green now.

@jperedadnr Would you confirm that scroll on a touch screen still works on Xorg 
?

My tests looks all good (manual and system/robot).

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1962431721


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-02-22 Thread Kevin Rushforth
On Mon, 19 Feb 2024 11:45:04 GMT, Thiago Milczarek Sayao  
wrote:

>> Jose Pereda has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Add compile-time checks to GdkSeat
>
> The rationale was:
> 
> This tells which events get delivered to the window while grabbing. XWayland 
> might be sensitive to `GDK_TOUCH_MASK` while Xorg is not.
> 
> So the Idea was to keep the current way (with `gdk_pointer_grab` or 
> `gdk_device_grab`, and adding the "deliver TOUCH events to me" might fix it.
> 
> Another place to investigate is:
> 
> 
> #define GDK_FILTERED_EVENTS_MASK 
> static_cast(GDK_ALL_EVENTS_MASK \
> & ~GDK_TOUCH_MASK)
> 
> 
> 
> It seems that Xorg converts touch events to regular mouse events, but 
> XWayland might be different.

@tsayao can you re-review?

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-195959


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-02-20 Thread Jose Pereda
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

I've just reverted the previous changes, and just applied the touch mask to the 
`gdk_pointer_grab` function. Tests should be green now.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1954087066


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-02-19 Thread Jose Pereda
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

I see... so removing all the changes from this PR, and using this diff from 
head instead:


diff --git 
a/modules/javafx.graphics/src/main/native-glass/gtk/glass_general.cpp 
b/modules/javafx.graphics/src/main/native-glass/gtk/glass_general.cpp
index e7a230b2b6..e89db2a55f 100644
--- a/modules/javafx.graphics/src/main/native-glass/gtk/glass_general.cpp
+++ b/modules/javafx.graphics/src/main/native-glass/gtk/glass_general.cpp
@@ -605,7 +605,8 @@ glass_gdk_mouse_devices_grab_with_cursor(GdkWindow 
*gdkWindow, GdkCursor *cursor
 | GDK_BUTTON2_MOTION_MASK
 | GDK_BUTTON3_MOTION_MASK
 | GDK_BUTTON_PRESS_MASK
-| GDK_BUTTON_RELEASE_MASK),
+| GDK_BUTTON_RELEASE_MASK
+   | GDK_TOUCH_MASK),
 NULL, cursor, GDK_CURRENT_TIME);
 
 return (status == GDK_GRAB_SUCCESS) ? TRUE : FALSE;


it looks like dragging works on a touch enabled display and Wayland. (No 
changes needed in `GDK_FILTERED_EVENTS_MASK`).

Tested this change on X11 as well, I don't see any issue.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1952469103


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-02-19 Thread Thiago Milczarek Sayao
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

The rationale was:

This tells which events get delivered to the window while grabbing. XWayland 
might be sensitive to `GDK_TOUCH_MASK` while Xorg is not.

So the Idea was to keep the current way (with `gdk_pointer_grab` or 
`gdk_device_grab`, and adding the "deliver TOUCH events to me" might fix it.

Another place to investigate is:


#define GDK_FILTERED_EVENTS_MASK static_cast(GDK_ALL_EVENTS_MASK \
& ~GDK_TOUCH_MASK)



It seems that Xorg converts touch events to regular mouse events, but XWayland 
might be different.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1952278279


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-02-19 Thread Jose Pereda
On Mon, 19 Feb 2024 00:35:48 GMT, Thiago Milczarek Sayao  
wrote:

>> Jose Pereda has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Add compile-time checks to GdkSeat
>
> A shot in the dark since I don't own a touch enabled monitor:
> 
> Test 1:
> 
>  Add `GDK_SCROLL_MASK` on the original `gdk_pointer_grab` function;
> 
> Test 2:
> 
> This PR uses `GDK_SEAT_CAPABILITY_ALL_POINTING` which includes the touch 
> masks.
> 
> So the equivalent would include `GDK_TOUCH_MASK` on `gdk_pointer_grab`.
> 
> 
> I would bet on option 2.

Thanks @tsayao, I can definitely test those suggestions.

If I get it right, you are proposing this change:


status = gdk_pointer_grab(gdkWindow, owner_events, (GdkEventMask)
(GDK_POINTER_MOTION_MASK
| GDK_POINTER_MOTION_HINT_MASK
+| GDK_TOUCH_MASK // or 
GDK_SCROLL_MASK
| GDK_BUTTON_MOTION_MASK
| GDK_BUTTON1_MOTION_MASK
| GDK_BUTTON2_MOTION_MASK
| GDK_BUTTON3_MOTION_MASK
| GDK_BUTTON_PRESS_MASK
| GDK_BUTTON_RELEASE_MASK
NULL, cursor, GDK_CURRENT_TIME);


But I don't see how this would fix the issue of the failing test on Linux that 
happens also without a touch enabled display. I tried both, the test still 
fails (two out of 5 times or so). Also, this is called only if there is no 
gdk_seat_grab function (which happens before GTK 3.20).

However, if I use `GDK_SEAT_CAPABILITY_NONE` instead of 
`GDK_SEAT_CAPABILITY_ALL_POINTING`, it passes 10 out of 10, but that would 
cancel pointer/touch events, wouldn't it?

Is there any chance that the robot implementation (via XTest, that uses 
`XTestGrabControl`) might not work fine with the new seat_grab approach after 
GTK 3.20+?

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1952237235


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-02-18 Thread Thiago Milczarek Sayao
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

A shot in the dark since I don't own a touch enabled monitor:

Test 1:

 Add `GDK_SCROLL_MASK` on the original `gdk_pointer_grab` function;

Test 2:

This PR uses `GDK_SEAT_CAPABILITY_ALL_POINTING` which includes the touch masks.

So the equivalent would include `GDK_TOUCH_MASK` on `gdk_pointer_grab`.


I would bet on option 2.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1951510764


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-01-23 Thread Thiago Milczarek Sayao
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

Maybe this: https://docs.gtk.org/gdk3/enum.CrossingMode.html

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1906879038


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-01-23 Thread Jose Pereda
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

Yes, for some reason, after applying the changes from this PR, there are Leave 
events.

When it works:

loaded gdk_seat_grab
loaded gdk_display_get_default_seat
GlassApplication::process_events: GDK_ENTER_NOTIFY
found schema 'org.gnome.desktop.interface' and key 'scaling-factor'
GlassApplication::process_events: GDK_ENTER_NOTIFY
GlassApplication::process_events: GDK_MOTION_NOTIFY
GlassApplication::process_events: GDK_BUTTON_PRESS
GlassApplication::process_mouse_button: pressed
GlassApplication::process_events: GDK_BUTTON_RELEASE
GlassApplication::process_mouse_button: not pressed
...

When it fails:

loaded gdk_seat_grab
loaded gdk_display_get_default_seat
GlassApplication::process_events: GDK_LEAVE_NOTIFY
GlassApplication::process_events: GDK_ENTER_NOTIFY
GlassApplication::process_events: GDK_LEAVE_NOTIFY
GlassApplication::process_events: GDK_ENTER_NOTIFY
found schema 'org.gnome.desktop.interface' and key 'scaling-factor'
GlassApplication::process_events: GDK_LEAVE_NOTIFY
GlassApplication::process_events: GDK_MOTION_NOTIFY
GlassApplication::process_events: GDK_BUTTON_PRESS
GlassApplication::process_mouse_button: pressed
Java_com_sun_glass_ui_gtk_GtkWindow__1ungrabFocus()
ungrab focus()
...

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1906355019


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-01-23 Thread Thiago Milczarek Sayao
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

It seems when it fails, button press is reported, but button release is not.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1906263617


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-01-23 Thread Thiago Milczarek Sayao
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

It seems when it fails it's because `GtkWindow._ungrabFocus` is called before 
the mouse release event.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1906198790


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-01-23 Thread Thiago Milczarek Sayao
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

For me it passes on XWayland and fails on Xorg (with Ubuntu 22.04).

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1906056674


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-01-22 Thread Jose Pereda
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

So it seems definitely related to the changes in this PR. 

Running with `GDK_DEBUG=nograbs` works (10 out of 10).

Adding some debug to the native code, and running with grabs, I can see:

When it works:

DatePickerTest STANDARD_OUT
Glass GTK library to load is glassgtk3

loaded gdk_seat_grab
loaded gdk_display_get_default_seat
grab
grab
found schema 'org.gnome.desktop.interface' and key 'scaling-factor'
loaded gdk_seat_ungrab
ungrab
grab
ungrab
found schema 'org.gnome.desktop.interface' and key 'scaling-factor'
ungrab
grab

> Task :systemTests:test
...

(so test starts after window is grabbed and focused, as it should be).

When it fails:

DatePickerTest STANDARD_OUT
Glass GTK library to load is glassgtk3

loaded gdk_seat_grab
loaded gdk_display_get_default_seat
grab
grab
found schema 'org.gnome.desktop.interface' and key 'scaling-factor'
loaded gdk_seat_ungrab
ungrab

> Task :systemTests:test
...

so window is not grabbed and not focused, and therefore native mouse clicks go 
elsewhere.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1905121653


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-01-22 Thread Jose Pereda
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

I'm testing on my local Linux Intel machine (Ubuntu 20.04), and running the 
test from head, it passes 5 out of 5 times.
After applying this patch, it fails with your same stacktrace 2 out of 5 times.

With some printouts, I can see that when the test fails, in 
`DatePickerTest::clickDatePickerCalendarPopup` the call to `mouseClick` 
processes correctly the mouse move at the correct coordinates and calls mouse 
press and mouse release, but what it should trigger the datePicker setOnAction 
event (line 168), fails to do so. For some reason, the event doesn't make it to 
the control.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1904746485


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-01-22 Thread Kevin Rushforth
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

This dropped off my radar.

The only pending issue is the test failure I noted in an earlier comment:

> I then did a full test run on our headful test systems, and there is one new 
> test failure -- it seems to be intermittent, although fails pretty 
> consistently on our Ubuntu 22.04 and Ubuntu 20.04 test systems. I can 
> reproduce it locally on a VM running Ubuntu 22.04, where it fails about 1/2 
> the time with this patch applied (it fails more often on our physical test 
> systems).
> 
> ```
> DatePickerTest > testDatePickerSceneChange FAILED
> java.lang.AssertionError: Timeout: Failed to receive onAction call.
> at org.junit.Assert.fail(Assert.java:89)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at test.util.Util.waitForLatch(Util.java:400)
> at 
> test.robot.javafx.scene.DatePickerTest.clickDatePickerCalendarPopup(DatePickerTest.java:90)
> at 
> test.robot.javafx.scene.DatePickerTest.testDatePickerSceneChange(DatePickerTest.java:123)
> ```
> 
> Not sure what to make of this. I am not aware of any problems with this test, 
> but it's possible that your fix has exposed a latent issue either in the test 
> or somewhere else.

@jperedadnr Can you try it on your system and see if you can reproduce this 
failure?

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1904662936


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2024-01-21 Thread Jose Pereda
On Mon, 18 Dec 2023 22:00:28 GMT, Kevin Rushforth  wrote:

>> Jose Pereda has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Add compile-time checks to GdkSeat
>
> The addition of the compile-time flags looks OK.
> 
> I did a build with GTK 3.22 (so it compiles the new code, does the dlsym, and 
> does the runtime check) and verified that there are no regressions when 
> running on an older system (Ubuntu 16.04).
> 
> I then did a full test run on our headful test systems, and there is one new 
> test failure -- it seems to be intermittent, although fails pretty 
> consistently on our Ubuntu 22.04 and Ubuntu 20.04 test systems. I can 
> reproduce it locally on a VM running Ubuntu 22.04, where it fails about 1/2 
> the time with this patch applied (it fails more often on our physical test 
> systems).
> 
> 
> DatePickerTest > testDatePickerSceneChange FAILED
> java.lang.AssertionError: Timeout: Failed to receive onAction call.
> at org.junit.Assert.fail(Assert.java:89)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at test.util.Util.waitForLatch(Util.java:400)
> at 
> test.robot.javafx.scene.DatePickerTest.clickDatePickerCalendarPopup(DatePickerTest.java:90)
> at 
> test.robot.javafx.scene.DatePickerTest.testDatePickerSceneChange(DatePickerTest.java:123)
> 
> 
> Not sure what to make of this. I am not aware of any problems with this test, 
> but it's possible that your fix has exposed a latent issue either in the test 
> or somewhere else.

@kevinrushforth is there anything else to be done before we can move this PR 
forward?

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1902776414


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2023-12-24 Thread Thiago Milczarek Sayao
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

Looks good.

-

Marked as reviewed by tsayao (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1305#pullrequestreview-1795582193


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2023-12-23 Thread Thiago Milczarek Sayao
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

I did compile (updating gcc) and test on Ubuntu 16.04 and Ubuntu 23.10 with 
Xorg and Wayland. All OK.
Code LGTM.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1868362953


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2023-12-19 Thread Johan Vos
On Mon, 18 Dec 2023 22:00:28 GMT, Kevin Rushforth  wrote:

> I did a build with GTK 3.22 (so it compiles the new code, does the dlsym, and 
> does the runtime check) and verified that there are no regressions when 
> running on an older system (Ubuntu 16.04).

That sounds good.

> If we decide that is a reasonable thing to do, we might want the minimum 
> _build-time_ version of GTK to be higher than the minimum version we support 
> at _runtime_.

That makes sense. We already do this with libc. When building (using the devkit 
based on the OpenJDK devkit), we use later versions of certain libraries, but 
we make sure the binaries still work on systems with older versions of the 
libraries. We should clearly state the minimum runtime requirements for some 
libraries (rather than for distributions, as it is always possible to hack new 
versions of libraries into old releases) .
What I really want to avoid is having 2 (or more) versions of the binaries for 
the same platform/arch/os combination.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1862376357


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2023-12-18 Thread Jose Pereda
> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
> 3.8+), and fixes the dragging issue on Wayland.

Jose Pereda has updated the pull request incrementally with one additional 
commit since the last revision:

  Add compile-time checks to GdkSeat

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1305/files
  - new: https://git.openjdk.org/jfx/pull/1305/files/99230ca6..f8ffe87f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=1305&range=02
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1305&range=01-02

  Stats: 8 lines in 1 file changed: 8 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jfx/pull/1305.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1305/head:pull/1305

PR: https://git.openjdk.org/jfx/pull/1305


Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]

2023-12-18 Thread Kevin Rushforth
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda  wrote:

>> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and 
>> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and 
>> wrapped functions for GTK 3.20+ (so systems without it still run with GTK 
>> 3.8+), and fixes the dragging issue on Wayland.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add compile-time checks to GdkSeat

The addition of the compile-time flags looks OK.

I did a build with GTK 3.22 (so it compiles the new code, does the dlsym, and 
does the runtime check) and verified that there are no regressions when running 
on an older system (Ubuntu 16.04).

I then did a full test run on our headful test systems, and there is one new 
test failure -- it seems to be intermittent, although fails pretty consistently 
on our Ubuntu 22.04 and Ubuntu 20.04 test systems. I can reproduce it locally 
on a VM running Ubuntu 22.04, where it fails about 1/2 the time with this patch 
applied (it fails more often on our physical test systems).


DatePickerTest > testDatePickerSceneChange FAILED
java.lang.AssertionError: Timeout: Failed to receive onAction call.
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.assertTrue(Assert.java:42)
at test.util.Util.waitForLatch(Util.java:400)
at 
test.robot.javafx.scene.DatePickerTest.clickDatePickerCalendarPopup(DatePickerTest.java:90)
at 
test.robot.javafx.scene.DatePickerTest.testDatePickerSceneChange(DatePickerTest.java:123)


Not sure what to make of this. I am not aware of any problems with this test, 
but it's possible that your fix has exposed a latent issue either in the test 
or somewhere else.

-

PR Comment: https://git.openjdk.org/jfx/pull/1305#issuecomment-1861759137