Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8217955: Problems with touch input and JavaFX 11 #457

Closed

Conversation

@jperedadnr
Copy link
Collaborator

@jperedadnr jperedadnr commented Apr 5, 2021

This PR filters out GDK_TOUCH_EVENT_MASK from GDK_ALL_EVENTS_MASK to prevent touch events from being used instead of regular mouse events on Linux platforms. Note that the touch events will be delivered as mouse pressed/dragged events.

Since we still compile with GTK 2.x the bit flag (that was introduced in GTK 3.4) needs to be defined.

This PR has been tested on Ubuntu 20.04 with a touch enabled display.


Progress

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

Issue

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 457

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

Using diff file

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

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Apr 5, 2021

👋 Welcome back jpereda! 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 openjdk bot added the rfr label Apr 5, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Apr 5, 2021

Webrevs

@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented Apr 5, 2021

@pankaj-bansal It looks OK to me, but can you also take a look at this?

@kevinrushforth kevinrushforth self-requested a review Apr 5, 2021
@tsayao
Copy link
Collaborator

@tsayao tsayao commented Apr 6, 2021

If anyone would implement touch events, the change would need to be rolled back?

If you catch and consume touch events, would it also work?

I mean on GlassApplication.cpp -> process_events:

case GDK_TOUCH_BEGIN:
case GDK_TOUCH_UPDATE:
case GDK_TOUCH_END:
case GDK_TOUCH_CANCEL:
    gtk_main_do_event(event);

I don't own a touch device and GTK_DEBUG=touchscreen seems to work only with gtk signals (not gdk events).

@johanvos
Copy link
Collaborator

@johanvos johanvos commented Apr 7, 2021

If anyone would implement touch events, the change would need to be rolled back?

Yes, if touch events would be implemented properly at a native level, this change will need to be rolled back. This can be a follow-up issue, but it seems less urgent to me than this particular fix.

@Torbuntu
Copy link
Contributor

@Torbuntu Torbuntu commented Apr 7, 2021

I've started working on trying to get the touch support available on Linux, and have a branch started here: https://github.com/torbuntu/jfx/tree/touch-support

Still trying to get everything wired up properly.

Copy link
Member

@kevinrushforth kevinrushforth left a comment

I think we can proceed with this PR. It can be adjusted (or reverted) if touch support is added in the future.

@openjdk
Copy link

@openjdk openjdk bot commented Apr 7, 2021

@jperedadnr 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:

8217955: Problems with touch input and JavaFX 11

Reviewed-by: kcr, jvos

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 3 new commits pushed to the master branch:

  • 9796a83: 8263322: Calling Application.launch on FX thread should throw IllegalStateException, but causes deadlock
  • 5898858: 8263169: [macos] JavaFX windows open as tabs when system preference for documents is set
  • e63931e: 8262366: Update glib to version 2.66.7

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

@openjdk openjdk bot added the ready label Apr 7, 2021
Copy link
Collaborator

@johanvos johanvos left a comment

This fixes a current issue, and while a solution that adds native touch-event support is highly encouraged, this fix will already improve the current situation.

@jperedadnr
Copy link
Collaborator Author

@jperedadnr jperedadnr commented Apr 7, 2021

/integrate

@openjdk openjdk bot closed this Apr 7, 2021
@openjdk openjdk bot added integrated and removed ready rfr labels Apr 7, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Apr 7, 2021

@jperedadnr Since your change was applied there have been 3 commits pushed to the master branch:

  • 9796a83: 8263322: Calling Application.launch on FX thread should throw IllegalStateException, but causes deadlock
  • 5898858: 8263169: [macos] JavaFX windows open as tabs when system preference for documents is set
  • e63931e: 8262366: Update glib to version 2.66.7

Your commit was automatically rebased without conflicts.

Pushed as commit cc94e96.

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

@Torbuntu
Copy link
Contributor

@Torbuntu Torbuntu commented Apr 9, 2021

This fixes a current issue, and while a solution that adds native touch-event support is highly encouraged, this fix will already improve the current situation.

What would be the protocol for requesting guidance on this? Since as I mentioned I am trying to implement touch-event support so that JavaFX can be used out of the box on devices such as PinePhone and PineTab running aarch64 linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants