8087370: [odroid] Monocle: Touch is still broken on Odroid

[fwolter]

There are sometimes multitouch events detected, when only a single touch should be detected under certain conditions. This lead to touch events on previous touch positions.

The referenced bug is closed with "won't fix" with the justification it would be platform specific. But this is not correct. It affects all Linux platforms combined with a touch controller, which sends the touch events in an uncommon, but valid(!), order.

We encountered this problem with the touch controller ILI2511 with firmware V6000_V1 in the Tianma display TM070JVHG33. This patch fixes the problem. It is the same patch attached to above bug report.

---

Progress

• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue
• [ ] Change must be properly reviewed

Issue

• JDK-8087370: [odroid] Monocle: Touch is still broken on Odroid ⚠️ Issue is not open.

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jfx pull/641/head:pull/641
$ git checkout pull/641

Update a local copy of the PR:
$ git checkout pull/641
$ git pull https://git.openjdk.java.net/jfx pull/641/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 641

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jfx/pull/641.diff

[bridgekeeper[bot]]

Hi @fwolter, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user fwolter" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

[fwolter]

/signed

[bridgekeeper[bot]]

You are already a known contributor!

[mlbridge[bot]]

Webrevs

• 00: Full (746568b9)

[AlexanderScherbatiy]

The fix #746 for 8282702 also proposes to copy the touch state. The fix makes it in LinuxStatefulMultiTouchProcessor class. Do issues 8087370 and 8282702 have the same root?

[ea1111]

@AlexanderScherbatiy I think both issues address the same problem in quite the same way.

However, #746 solves the problem only for LinuxStatefulMultiTouchProcessor. This pull request implements the fix more generally in the TouchPipeline so that other touch processors (e. g. LinuxStatelessMultiTouchProcessor and LinuxSimpleTouchProcessor) can also benefit.

[AlexanderScherbatiy]

It's sounds reasonable to fix the issue generally on all available touch processors.

[fwolter]

Sounds reasonable to me, too. What a coincidence we both filed a PR to fix the same 8-year-old bug, now, after nobody cared for 8 years :)

[AlexanderScherbatiy]

The 8087370 includes LinuxStatefulMultiTouchProcessor-fix-for-1.patch to fix the first described case:

1. Consider you're touching the screen with two fingers, for example coordinates
of one point is ~100,100 and second point is ~200,200. While still touching around
the same points some events are reported with one or another point
to be either 100,200 or 200,100.

Is it reproduced on your system as well?

It relates to 8282104: Node zoom/rotate flickers on Raspberry Pi with Touchscreen with fix #737 which suggests to not take LinuxInput.ABS_X/Y events into account for non zero slots instead just skipping them for all slots.

[fwolter]

We didn't encounter the first point in this bug report as our application doesn't make use of multi touch. It was propably not the best idea of the original bug reporter 8 years ago to file two different bugs within the same bug report. Seems like you fixed two bugs which were already about to be fixed nearly a decade ago :/

[bridgekeeper[bot]]

@fwolter This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[fwolter]

Do you need any additional information to start reviewing this bug?

[kevinrushforth]

I reopened the JBS bug (it has been closed as "Won't fix" long ago).

@johanvos or @jperedadnr can decide whether they want to review and sponsor this.

[johanvos]

I'll have a look at it next week. I think this is an area where we would need more tests. Can you add a test for this that fails before the patch and succeeds after?

[bridgekeeper[bot]]

@fwolter This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[johanvos]

Hi @fwolter . I still think this would be a good fix, but it would be really helpful if there was a regression test for this. There have been a number of changes in how touch handling should be processed, and I want to make sure we don't run in circles (where patch A breaks the fix from patch B and vice versa). Do you think you can come up with a unit test for this?

[fwolter]

Unfortunately, I can't afford time to burrow into the issue again for writing tests at the moment. @AlexanderScherbatiy is there any chance you can provide a test case?

[AlexanderScherbatiy]

I can try to look at the test. Is the TouchEventLookaheadTest a good example for writing a TouchEvent test?
https://github.com/openjdk/jfx/blob/c20f6d0f0133a59a982959ee2e48809f30f3130b/tests/system/src/test/java/test/robot/com/sun/glass/ui/monocle/TouchEventLookaheadTest.java#L42

[bridgekeeper[bot]]

@fwolter This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper[bot]]

@fwolter This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.