8289115: Touch events is not dispatched after upgrade to JAVAFX17+

[mstr2]

trafficstars

This PR fixes a bug (JDK-8289115) that was introduced with #394, which changed the following line in the NotifyTouchInput function (ViewContainer.cpp):

- const bool isDirect = true;
+ const bool isDirect = IsTouchEvent();

IsTouchEvent is a small utility function that uses the Windows SDK function GetMessageExtraInfo to distinguish between mouse events and pen events as described in this article: https://learn.microsoft.com/en-us/windows/win32/tablet/system-events-and-mouse-messages

I think that using this function to distinguish between touchscreen events and pen events is erroneous. The linked article states: "When your application receives a mouse message (such as WM_LBUTTONDOWN) [...]"

But we are not receiving a mouse message in NotifyTouchInput, we are receiving a WM_TOUCH message. I couldn't find any information on whether this API usage is also supported for WM_TOUCH messages, but my testing shows that that's probably not the case (I tested this with a XPS 15 touchscreen laptop, where GetMessageExtraInfo returns 0 for WM_TOUCH messages).

My suggestion is to check the TOUCHEVENTF_PEN flag of the TOUCHINPUT structure instead:

const bool isDirect = (ti->dwFlags & TOUCHEVENTF_PEN) == 0;

---

Progress

• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue
• [ ] Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8289115: Touch events is not dispatched after upgrade to JAVAFX17+ (Bug - P4)

Reviewers

• Jose Pereda (@jperedadnr - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1459/head:pull/1459
$ git checkout pull/1459

Update a local copy of the PR:
$ git checkout pull/1459
$ git pull https://git.openjdk.org/jfx.git pull/1459/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1459

View PR using the GUI difftool:
$ git pr show -t 1459

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1459.diff

Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back mstrauss! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk[bot]]

@mstr2 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8289115: Touch events is not dispatched after upgrade to JAVAFX17+

Reviewed-by: jpereda, kcr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 24 new commits pushed to the master branch:

• 1fb8755dc6452bdb07685c02272ecfc578fb1eba: 8198830: BarChart: auto-range of CategoryAxis not working on dynamically setting data
• ca04c87d307c36591162af8cd6298ede17812834: 8334713: WebKit build failed on LoongArch64 because currentStackPointer is undefined
• 0d2a0a462e500b9bb7ac4fc4f45beff9a378ea60: 8334161: Enable -Werror for javac tasks to fail on any warnings
• b80b106df9eee12fc42d85cf9386975dc56c5a57: 8314754: Minor ticks are not getting updated both the axes in LineChart
• d9c88630fad0e9cd4a93ac0901141f4ae7939be0: 8311124: [Windows] User installed font 8281327 fix does not work for all cases
• 101e5175ff429828de25204bc3e5f216d383060b: 8334657: Enable binary check
• 83c8dcc91a892c557c5b40064e94274caa89ce6f: 8327255: javac lint warnings: removal, missing-explicit-ctor
• e9b10ab816c2bf7b3bb88bf6c9c0ccb826bff34c: 8334731: GHA: build on macOS / aarch64
• 17c2dba0d5ab7608e7df6673c67ace7212a60040: 8334739: XYChart and (Stacked)AreaChart properties return incorrect beans
• c0216ae94862c860e238b251a5e51aa7c83b9a1d: 8088923: IOOBE when adding duplicate categories to the BarChart
• ... and 14 more: https://git.openjdk.org/jfx/compare/b5fe362286056e516be1d26f5d1cdda12eb20a4c...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[mlbridge[bot]]

Webrevs

• 00: Full (90612809)

[mstr2]

@jperedadnr Maybe you could test this PR with the pen digitizer that you used for #394 to ensure that this change doesn't introduce yet another regression.

[jperedadnr]

Yes, I'll do that, I still have it around.

[kevinrushforth]

Reviewer: @jperedadnr

/reviewers 2

[openjdk[bot]]

@kevinrushforth The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[FlorianKirmaier]

I can confirm that this change fixes a Problem for more users. Actually, i have a built with exactly the same change. So if this doesn't break anything, it would be great to see this integrated.

[mstr2]

I can confirm that this change fixes a Problem for more users. Actually, i have a built with exactly the same change. So if this doesn't break anything, it would be great to see this integrated.

Feel free to approve if you think the patch is good to go.

[drmarmac]

I attempted to test this with the sample app in JDK-8249737 and Microsoft.Windows.Simulator using the simulator's "Basic touch mode".

The "Too many touch points reported" exception seems to be thrown consistently with const bool isDirect = true; (original before #394) and const bool isDirect = (ti->dwFlags & TOUCHEVENTF_PEN) == 0; (this PR)

I didn't see the exception with const bool isDirect = IsTouchEvent(); (#394) and const bool isDirect = false (which is probably what IsTouchEvent() returns here).

Not sure what this simulator actually sends, but someone more knowledgeable might want to have a look at what's happening.

[mlbridge[bot]]

Mailing list message from Jurgen Doll on openjfx-dev:

Thank you Michael, Jose, Florian, and Markus

In my mind the "Too many touch points reported" exception fix (#394)
appears to not be the correct fix for that issue and is what caused this
regression.

There seems to be two possible paths forward, either revert #394 or
integrate this. In both cases the "Too many touch points reported" issue
will need to be readdressed.

It would be nice to see this move forward somehow.

Thanks again, Jurgen

On Tue, 04 Jun 2024 18:28:47 +0200, Markus Mack <mmack at openjdk.org> wrote:

[mstr2]

/integrate

[openjdk[bot]]

Going to push as commit 6c3fb5f557597a5a226d4a7bd41d84b7feb4a162. Since your change was applied there have been 24 commits pushed to the master branch:

• 1fb8755dc6452bdb07685c02272ecfc578fb1eba: 8198830: BarChart: auto-range of CategoryAxis not working on dynamically setting data
• ca04c87d307c36591162af8cd6298ede17812834: 8334713: WebKit build failed on LoongArch64 because currentStackPointer is undefined
• 0d2a0a462e500b9bb7ac4fc4f45beff9a378ea60: 8334161: Enable -Werror for javac tasks to fail on any warnings
• b80b106df9eee12fc42d85cf9386975dc56c5a57: 8314754: Minor ticks are not getting updated both the axes in LineChart
• d9c88630fad0e9cd4a93ac0901141f4ae7939be0: 8311124: [Windows] User installed font 8281327 fix does not work for all cases
• 101e5175ff429828de25204bc3e5f216d383060b: 8334657: Enable binary check
• 83c8dcc91a892c557c5b40064e94274caa89ce6f: 8327255: javac lint warnings: removal, missing-explicit-ctor
• e9b10ab816c2bf7b3bb88bf6c9c0ccb826bff34c: 8334731: GHA: build on macOS / aarch64
• 17c2dba0d5ab7608e7df6673c67ace7212a60040: 8334739: XYChart and (Stacked)AreaChart properties return incorrect beans
• c0216ae94862c860e238b251a5e51aa7c83b9a1d: 8088923: IOOBE when adding duplicate categories to the BarChart
• ... and 14 more: https://git.openjdk.org/jfx/compare/b5fe362286056e516be1d26f5d1cdda12eb20a4c...master

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@mstr2 Pushed as commit 6c3fb5f557597a5a226d4a7bd41d84b7feb4a162.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.