jfx icon indicating copy to clipboard operation
jfx copied to clipboard

8282703: Axis is not cached in the LinuxTouchTransform class

Open AlexanderScherbatiy opened this issue 3 years ago • 4 comments

An axis is not cached in the LinuxTouchTransform class.

To reproduce the issue I added System.out.printf("initTransform: axis: %d, index: %d%n", axis, index); log to the LinuxTouchTransform.initTransform() method: https://github.com/openjdk/jfx/blob/5112be957be70dd6521e6fb6ee64e669c148729c/modules/javafx.graphics/src/main/java/com/sun/glass/ui/monocle/LinuxTouchTransform.java#L117 run the JFXButtonExample sample and tapped the touch screen on a Raspberry Pi with Touchscreen.

The result was

initTransform: axis: 47, index: 0
initTransform: axis: 57, index: 0
initTransform: axis: 53, index: 0
initTransform: axis: 54, index: 0
initTransform: axis: 0, index: 0
initTransform: axis: 1, index: 0
initTransform: axis: 53, index: 0
initTransform: axis: 54, index: 0
initTransform: axis: 0, index: 0
initTransform: axis: 1, index: 0
initTransform: axis: 53, index: 0
initTransform: axis: 54, index: 0
initTransform: axis: 0, index: 0
initTransform: axis: 1, index: 0

The initTransform() is called several times for axis 0,1,47,53,54 and index is always set to zero.

The straight forward fix is to store the given axis in the axes array: "axes[index] = axis". This is the first commit for the current fix. Using this fix the output with printf from initTransform() method looks like:

initTransform: axis: 47, index: 0
initTransform: axis: 57, index: 1
initTransform: axis: 53, index: 2
initTransform: axis: 54, index: 4
initTransform: axis: 1, index: 5

Now all axes are printed only once and the index value is different for each axes.

However, the minimum/maximum values are retrieved and cached for ABS_X/Y and ABS_MT_POSITION_X/Y axes after the fist tap on the screen. The second commit improves this moving the ABS_X/Y and ABS_MT_POSITION_X/Y axes initialization into the LinuxTouchTransform constructor.

Now the touch logs look like:

// LinuxTouchTransform constructor
// device: /dev/input/mouse0
initTransform: axis: 0, index: 0
initTransform: axis: 1, index: 1
initTransform: axis: 53, index: 2
initTransform: axis: 54, index: 3

// LinuxTouchTransform constructor
// device: /dev/input/event2
initTransform: axis: 0, index: 0
initTransform: axis: 1, index: 1
initTransform: axis: 53, index: 2
initTransform: axis: 54, index: 3

// the first tap
initTransform: axis: 57, index: 4
initTransform: axis: 47, index: 5

The ABS_X/Y and ABS_MT_POSITION_X/Y axes and corresponding minimum/maximum values are initialized in the constructor. The other axes which stores only default values in translates and scalars arrays are initialized during touch events.


Progress

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

Issue

  • JDK-8282703: Axis is not cached in the LinuxTouchTransform class

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 747

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

Using diff file

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

AlexanderScherbatiy avatar Mar 05 '22 15:03 AlexanderScherbatiy

:wave: Welcome back alexsch! 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.

bridgekeeper[bot] avatar Mar 05 '22 15:03 bridgekeeper[bot]

Webrevs

mlbridge[bot] avatar Mar 05 '22 15:03 mlbridge[bot]

@AlexanderScherbatiy Did I miss something? Is touch now support with GTK? I thought the implementation is missing for GTK: https://github.com/openjdk/jfx/pull/457

chilliger avatar Mar 11 '22 15:03 chilliger

Did I miss something? Is touch now support with GTK? I thought the implementation is missing for GTK

This PR is for Monocle, which is not used for desktop Linux (it's for embedded systems like Raspberry Pi).

kevinrushforth avatar Mar 11 '22 15:03 kevinrushforth

@AlexanderScherbatiy 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] avatar Mar 31 '23 23:03 bridgekeeper[bot]

@AlexanderScherbatiy 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!

Could you review the fix?

AlexanderScherbatiy avatar Apr 24 '23 08:04 AlexanderScherbatiy

@AlexanderScherbatiy 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] avatar May 22 '23 15:05 bridgekeeper[bot]

@AlexanderScherbatiy 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.

bridgekeeper[bot] avatar Jun 19 '23 22:06 bridgekeeper[bot]