jfx
jfx copied to clipboard
openjdk
8262023: Scrolled button is pressed using Monocle on Raspberry Pi with Touchscreen
The issue is reproduced on Raspberry Pi 3 B+ with Touchscreen display.
To reproduce the issue run the ScrollPaneSample with Monocle:
sudo jdk/bin/java -Dprism.verbose=true -Djavafx.platform=monocle -Dembedded=monocle -Dglass.platform=Monocle ScrollPaneSample
An application consists of a ScrollPane with buttons. if a button is touched by a finger, moved up/down and released, the button is scrolled and the button's action is fired.
This happens because Monocle generates mouse pressed, mouse dragged, scroll, mouse released events when touch events are received. Even a button is scrolled on a ScrollPane it still fires the button's action on the synthesized mouse release event.
My first attempt was to add a scroll event listener to a ButtonBehavior class and disarm the button when the scroll event is received. This does work not in the case where buttons are small and scrolling buttons leads that a finger is released on the next button (the scrolling process is remained a slightly behind the touched finger so the finger is touched on one button and released on another). In this case all scroll events goes to the first button and the second button still fires its action (it does not disarmed because it does not receive scroll events).
The current fix adds drag event listener to ButtonBehavior to disarm the button. Drag events goes to the touched and released buttons.
Than I checked the fix on the same Raspberry Pi using GTK with touchscreen.
sudo jdk/bin/java -Dprism.verbose=true -Djavafx.platform=gtk ScrollPaneSample
I have not seen scroll events using GTK (even using -Dgtk.com.sun.javafx.gestures.scroll=true option), but GTK sends mouse drag events on a button touch. The mouse drag event between a button touch and release events would disarm the button in the proposed ButtonBehavior drag event handler. So I added the check if the mouse drag is synthesized. If the mouse drag is synthesized (Monocle case on touchscreen) it disarms the button, otherwise (GTK case) not.
I checked the fix for the following controls placed on a ScrollPane (see ScrollPaneControlsSample sample) :
- Fixed in corresponding behavior classes or its parents: Button, ToggleButton, CheckBox, ComboBox, ChoiceBox, ColorPicker, DatePicker, RadioButton
- Works because an action is not fired on mouse release event: TextField
- Does not work: Slider
The Slider control does not work with the fix because it reacts not only on mouse release event but on mouse drag event as well. It requires a separate fix.
I checked the Ensemble8 sample with the fix. It works with Monocle on Raspberry Pi 3B+ on Touchscreen. Scrolling the main page by a finger does not makes it to be pressed.
The Ensemble8 sample does not work with GTK on Raspberry Pi 3B+ with Touchscreen. I see it generates scroll events ( it has its own ScrollEventSynthesizer) and action events and it can makes the Ensemble8 buttons on a ScrollPane to be pressed.
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-8262023: Scrolled button is pressed using Monocle on Raspberry Pi with Touchscreen
Reviewers
- @m7snxx (no known github.com user name / role)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx pull/406/head:pull/406
$ git checkout pull/406
Update a local copy of the PR:
$ git checkout pull/406
$ git pull https://git.openjdk.org/jfx pull/406/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 406
View PR using the GUI difftool:
$ git pr show -t 406
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/406.diff
: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](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
Can you provide an automated test for this?
Since this is touching common code, what testing have you done to ensure no regressions on other platforms when not using Monocle?
This will need careful review and testing.
/reviewers 2
@kevinrushforth The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).
I have Touchscreen only on Raspberry Pi so I checked the touch events only on JavaFX on arm with Monocle and GTK.
I also checked the fix with ScrollPaneControlsSample on Linux and Windows with ordinary screen and using only mouse (press, release, scroll) and the sample works with and without the fix.
I run all but webkit automated tests with the fix on Ubuntu gradle test and they passed.
I will look the way to provide an automated test.
I am interested if there is a better way to fix this. Handling scroll event would be straightforward (because the ScrollPane is used) but unfortunately it does not work when two controls are scrolled.
Some more details about handled events in ScrollPaneSample Monocle.
Touch and release a button (press a button by touching the screen)
[button behavior] MOUSE_ENTERED, Button: 2
[button behavior] MOUSE_PRESSED, Button: 2
[button behavior] MOUSE_RELEASED, Button: 2
Scroll a button:
[button behavior] MOUSE_PRESSED, Button: 3
[button behavior] SCROLL, Button: 3
[button behavior] MOUSE_DRAGGED, Button: 3
[button behavior] SCROLL, Button: 3
[button behavior] MOUSE_DRAGGED, Button: 3
[button behavior] SCROLL, Button: 3
[button behavior] MOUSE_DRAGGED, Button: 3
[button behavior] SCROLL, Button: 3
[button behavior] MOUSE_DRAGGED, Button: 3
[button behavior] MOUSE_RELEASED, Button: 3
Scroll a button that the next button appears under the finger:
[button behavior] MOUSE_ENTERED, Button: 5
[button behavior] MOUSE_PRESSED, Button: 5
[button behavior] SCROLL, Button: 4
[button behavior] MOUSE_DRAGGED, Button: 5
[button behavior] SCROLL, Button: 4
[button behavior] MOUSE_DRAGGED, Button: 5
[button behavior] SCROLL, Button: 4
[button behavior] MOUSE_DRAGGED, Button: 5
[button behavior] SCROLL, Button: 4
[button behavior] MOUSE_DRAGGED, Button: 5
[button behavior] SCROLL, Button: 4
[button behavior] MOUSE_DRAGGED, Button: 5
[button behavior] SCROLL, Button: 4
[button behavior] MOUSE_DRAGGED, Button: 5
[button behavior] SCROLL, Button: 4
[button behavior] MOUSE_DRAGGED, Button: 5
[button behavior] MOUSE_RELEASED, Button: 5
Note: all scroll events go to Button 4 but mouse is released on Button 5.
GTK Touch and release a button (press a button by touching the screen)
[button behavior] MOUSE_PRESSED, Button: 1
[button behavior] MOUSE_DRAGGED, Button: 1
[button behavior] MOUSE_DRAGGED, Button: 1
[button behavior] MOUSE_RELEASED, Button: 1
Note: mouse drag events are generated
Scroll buttons by mouse (scrolling by touch does not work for me on GTK even with -Dgtk.com.sun.javafx.gestures.scroll=true option)
[button behavior] MOUSE_EXITED, Button: 3
[button behavior] MOUSE_ENTERED, Button: 4
[button behavior] SCROLL, Button: 4
[button behavior] SCROLL, Button: 4
[button behavior] SCROLL, Button: 4
[button behavior] SCROLL, Button: 4
[button behavior] MOUSE_EXITED, Button: 4
[button behavior] MOUSE_ENTERED, Button: 5
May be it has sense to add a drag event handler (which disarms the corresponding button) to ButtonBehavior only if javafx.platform is set to monocle to localize the fix only for Monocle? Or add a separate property and set it by default to true on Monocle and to false otherwise?
That would certainly be a safer (more targeted) fix. I'd like to hear from @jperedadnr and @johanvos on this.
In case it helps, I reproduced the problem on a Kobo Touch e-reader with an e-paper display. The expected result isn't explicit in the bug report, so just to confirm, there should be no button action events received at all when you touch any of the buttons only for the purpose of scrolling the entire pane, right?
$ sudo $HOME/opt/jdk-15.0.2+7/bin/java \
--module-path=$HOME/lib/armv6hf-sdk/lib --add-modules=javafx.controls \
-Dglass.platform=Monocle -Dmonocle.platform=EPD -Dprism.order=sw \
-Dmonocle.input.18/0/0/0.minX=0 -Dmonocle.input.18/0/0/0.maxX=800 \
-Dmonocle.input.18/0/0/0.minY=0 -Dmonocle.input.18/0/0/0.maxY=600 ScrollPaneSample
Button is pressed: 3
Button is pressed: 3
Button is pressed: 5
Button is pressed: 5
Button is pressed: 5
It's subtle, though. With a bit of practice, I can scroll the pane without causing the action events by swiping my finger quickly over the buttons. It's only when I linger for a fraction of a second too long on one of the buttons that one or more of the events are fired.
The expected result isn't explicit in the bug report, so just to confirm, there should be no button action events received at all when you touch any of the buttons only for the purpose of scrolling the entire pane, right?
Could you run Ensemble8 demo on the a Kobo Touch e-reader?
On my Raspberry Pi with Touchscreen I run Ensemble8, slide a main page to scroll and the program opens the sample where my finger is released. It prevents me to scroll Ensemble8 main page to bottom. To avoid this I need to use places without samples to scroll the main page down.
Could you run Ensemble8 demo on the Kobo Touch e-reader?
Yes, I see the problem you describe using the Ensemble8 app on my Kobo e-reader. Thanks. And thank you for having the courage to fix this! :smile:
I also tested the changes in this pull request. I notice an improvement in that there are less "Button is pressed" action events received, but I still see the error. I added calls to System.err.println right before any calls to arm(), fire(), or disarm() in each of the mouse event handlers in ButtonBehavior.java. I swiped the buttons four times to scroll the pane. The output is shown below. The first three worked. The fourth one failed by firing Button 12 twice.
The new method is marked with the prefix ***.
$ sudo $HOME/opt/jdk-15.0.2+7/bin/java \
--module-path=$HOME/lib/armv6hf-sdk/lib --add-modules=javafx.controls \
-Dglass.platform=Monocle -Dmonocle.platform=EPD -Dprism.order=sw \
-Dmonocle.input.18/0/0/0.minX=0 -Dmonocle.input.18/0/0/0.maxX=800 \
-Dmonocle.input.18/0/0/0.minY=0 -Dmonocle.input.18/0/0/0.maxY=600 \
ScrollPaneSample
--> Arming button (mouse pressed) ...
--> Disarming button (mouse exited) ...
--> Arming button (mouse pressed) ...
--> Disarming button (mouse exited) ...
--> Arming button (mouse pressed) ...
*** Disarming button (mouse dragged) ...
--> Arming button (mouse pressed) ...
--> Firing and disarming button (mouse released) ...
Button is pressed: 12
--> Arming button (mouse pressed) ...
--> Firing and disarming button (mouse released) ...
Button is pressed: 12
--> Arming button (mouse pressed) ...
*** Disarming button (mouse dragged) ...
The trace below shows a similar test but this time with the system property monocle.input.traceEvents set to true. (There is also the property monocle.input.traceEvents.verbose, but I don't find the extra information helpful.) In this case, the first two swipes worked, and the third one failed.
$ sudo $HOME/opt/jdk-15.0.2+7/bin/java \
--module-path=$HOME/lib/armv6hf-sdk/lib --add-modules=javafx.controls \
-Dglass.platform=Monocle -Dmonocle.platform=EPD -Dprism.order=sw \
-Dmonocle.input.18/0/0/0.minX=0 -Dmonocle.input.18/0/0/0.maxX=800 \
-Dmonocle.input.18/0/0/0.minY=0 -Dmonocle.input.18/0/0/0.maxY=600 \
-Dmonocle.input.traceEvents=true ScrollPaneSample
traceEvent: Set MouseState[x=400,y=300,wheel=0,buttonsPressed=IntSet[]]
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=238,y=448]]
traceEvent: Set MouseState[x=238,y=448,wheel=0,buttonsPressed=IntSet[212]]
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=253,y=110]]
traceEvent: Set MouseState[x=253,y=110,wheel=0,buttonsPressed=IntSet[212]]
traceEvent: Set TouchState[0]
traceEvent: Set MouseState[x=253,y=110,wheel=0,buttonsPressed=IntSet[]]
--> Arming button (mouse pressed) ...
--> Disarming button (mouse exited) ...
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=209,y=450]]
traceEvent: Set MouseState[x=209,y=450,wheel=0,buttonsPressed=IntSet[212]]
--> Arming button (mouse pressed) ...
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=209,y=450]]
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=229,y=262]]
traceEvent: Set MouseState[x=229,y=262,wheel=0,buttonsPressed=IntSet[212]]
*** Disarming button (mouse dragged) ...
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=235,y=238]]
traceEvent: Set MouseState[x=235,y=238,wheel=0,buttonsPressed=IntSet[212]]
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=265,y=109]]
traceEvent: Set MouseState[x=265,y=109,wheel=0,buttonsPressed=IntSet[212]]
traceEvent: Set TouchState[0]
traceEvent: Set MouseState[x=265,y=109,wheel=0,buttonsPressed=IntSet[]]
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=260,y=403]]
traceEvent: Set MouseState[x=260,y=403,wheel=0,buttonsPressed=IntSet[212]]
--> Arming button (mouse pressed) ...
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=260,y=403]]
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=260,y=403]]
traceEvent: Set TouchState[0]
traceEvent: Set MouseState[x=260,y=403,wheel=0,buttonsPressed=IntSet[]]
--> Firing and disarming button (mouse released) ...
Button is pressed: 10
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=260,y=400]]
traceEvent: Set MouseState[x=260,y=400,wheel=0,buttonsPressed=IntSet[212]]
traceEvent: Set TouchState[1,TouchState.Point[id=1,x=260,y=142]]
traceEvent: Set MouseState[x=260,y=142,wheel=0,buttonsPressed=IntSet[212]]
traceEvent: Set TouchState[0]
traceEvent: Set MouseState[x=260,y=142,wheel=0,buttonsPressed=IntSet[]]
--> Arming button (mouse pressed) ...
*** Disarming button (mouse dragged) ...
I find that any movement after touching a button will cause it to fire, even without lifting my finger. I don't think the button action should fire unless I lift my finger off the button from the same location or very near to where I first touched it.
Can you reproduce the error I'm seeing? Hold your finger on a button. Then roll your finger back and forth while keeping contact. Then release the button. Android does nothing when I do that on a button, and it seems to fire an event only when I release my finger very close to where I first touched it. With Monocle, though, I can fire a hundred action events just by rolling my finger back and forth on the button without ever releasing it.
It could be the sensitivity of my touch panel. The panel itself is double the resolution of the display, which is why I have to add all those monocle.input properties to set the correct 800 × 600 touch points to match the pixel resolution.
There is similar behavior on Android (using Monocle too), when scrolling through a list, as that may result in the list item to be "selected". The scrolled node receives a button_pressed event as well. The main difficulty I have is: What is the expected behavior? I would like to seen consistent behavior between the GTK and Monocle implementations, but the hardest part to me is getting a clear definition on what we would expect (as that is then what we can test).
What is the expected behavior?
I think that's why I never raised the issue. I let the bug train me to avoid it! For better or for worse, I think the only answer to your question is whatever Android and iOS do. After playing around with the JavaFX test buttons on the touchscreen, I finally tried the same thing on my Android phone, and there it worked just as one would expect. I didn't have to be careful at all.
As far as I can tell, on Android the rule is simple: the button fires only when it's released within a very small radius of where it was first touched. Furthermore, the button never fires at all if it was ever dragged at all. I'll try on an old iPad next.
Edit: In case it's unclear, by "Android" I mean an Android native app, like the Android Settings app, for example (and not JavaFX on Android). Sorry for any confusion.
What is the expected behavior?
I think that's why I never raised the issue. I let the bug train me to avoid it! For better or for worse, I think the only answer to your question is whatever Android and iOS do.
I agree it's good looking at how it is done on other platforms. It's still not clear to me (as in: I can't put it in an algorithm): if you say that the button is "fired" only when the release location is very close to the touch-start location, then it occurs to me that somehow there could have been other events that caused the location to change slightly, but don't these events lead to DRAG events? That would then contradict the second statement: whenever a drag is detected, there is no button-fired action at all.
Mailing list message from Michael Strauß on openjfx-dev:
Android exposes the "touch slop" value, which is the distance which a touch pointer has to travel before it is considered to be a drag event. It is important to also consider the case where a touch pointer wanders off to another location, and then without releasing, returns back to the start location. In this case, you would not want to interpret that as a "click" event.
See: https://developer.android.com/reference/android/view/ViewConfiguration.html#getScaledTouchSlop()
Am So., 18. Apr. 2021 um 12:57 Uhr schrieb Johan Vos <jvos at openjdk.java.net>:
Running ScrollPaneSample on my Pi 4 with the proposed changes works fine. Scrolling works and the buttons are not fired unless you stop scrolling and press one of them. Without the changes, the button that was selected when scrolling started gets fired right after scrolling finishes.
There is a minor issue: when you start scrolling, the button that gets touched (but not fired) in the first place gets focused (but this is not happening as a consequence of such changes).
There is also another issue, this time related to the changes: if you have a very small input touch radius (-Dmonocle.input.touchRadius=1) it is really hard to do a regular click on a button: with very small radius it is easier that the touch press event turns into a drag event, and then the button will be disarmed. While setting this tiny radius on a Pi seems unnecessary (due to low resolution), this is a usual value on Android, where the issue will happen as well.
Running ScrollPaneControlsSample works as well: with the proposed changes it works fine. Scrolling works and the controls are not fired unless you stop scrolling and press one of them. Without the changes, the control that was selected when scrolling started gets fired right after scrolling finishes.
Same issues as well: When you start scrolling, the control that gets touched (but not fired) in the first place gets focused. This has the side effect on a TextField that the keyboard (if enabled) shows up when scrolling finishes. Ideally, this shouldn't happen if you are finishing scrolling, unless you tap again on it to enable edit mode. Click issue: firing a control is now harder for the same reason mentioned earlier.
Both TextField (TextArea) and Slider issues will require a follow-up issue.
May be it has sense to add a drag event handler (which disarms the corresponding button) to ButtonBehavior only if javafx.platform is set to monocle to localize the fix only for Monocle? Or add a separate property and set it by default to true on Monocle and to false otherwise?
Following up on this: have you given this any more thought? The current fix runs the risk of regression on desktop platforms, so will need additional testing unless you decide to limit the fix to Monocle platforms.
May be it has sense to add a drag event handler (which disarms the corresponding button) to ButtonBehavior only if javafx.platform is set to monocle to localize the fix only for Monocle? Or add a separate property and set it by default to true on Monocle and to false otherwise?
Following up on this: have you given this any more thought? The current fix runs the risk of regression on desktop platforms, so will need additional testing unless you decide to limit the fix to Monocle platforms.
I am personally not in favor of adding a check on Monocle. That sounds like we care less about quality when talking about Monocle. We do more experimental things in Monocle, but I think we simply need to get this issue right. I fully agree it needs more testing. I think it would be good to see some scenario's that contain numbers and expected behavior (e.g. in case this sequence of low-level events is generated, what high-level events do we expect).
We do more experimental things in Monocle, but I think we simply need to get this issue right.
Yes, this is the better approach. If we proceed with this PR, we should make sure that what it does is valid and tested on all platforms. We might do something different based on whether or not an event is a touch event, but making it Monocle-only doesn't seem the best idea.
@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!
@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.