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.

Sign up for GitHub

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

Jump to bottom

Identifiers for touch points are not constant in multi touch #698

Open
gebausim opened this issue Mar 1, 2024 · 5 comments
Open

Identifiers for touch points are not constant in multi touch #698

gebausim opened this issue Mar 1, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@gebausim
Copy link
Contributor

gebausim commented Mar 1, 2024 •
edited
Loading

Propably the best way to showcase this is via https://people.igalia.com/clopez/wkbug/pointer-events/paint.html page. Reproduced by simultaneously doing the snake move with one finger on the right and multiple lines with the other hand. What is expected is the first touch not to be influenced by the other ones and remain the same color/id (red) all the way.

Got:

Screenshot_20240301_164748

Expected:

Screenshot_20240301_165021

Same issue can also be observed in https://patrickhlauke.github.io/touch/tracker/multi-touch-tracker-index.html page. Where the identifier numbers rapidly change between all the currently active touch ids.

Can be reproduced on cog 0.18 and master with the wayland backend with WPEWebKit 2.42.5. It does not occur with WPEWebKit 2.40.5 or 2.38.6.

The effects of this are that any javascript pan or zoom gestures don't work properly, can be tested on https://elmarquis.github.io/Leaflet.GestureHandling/examples/. Zooming on the map is just very funky. Basically any online maps don't work.
@gebausim
Copy link
Contributor Author

gebausim commented Mar 5, 2024 •
edited
Loading

WebKit issue: https://bugs.webkit.org/show_bug.cgi?id=270516

@aperezdc aperezdc added the bug Something isn't working label Mar 27, 2024
@psaavedra
Copy link
Member

This is interesting. I wonder if you could try again with main/master but reverting WPE - Make TouchPoint ID unique - commit 057b735cbf912f620273106541f2196191c5aa53 and reporting back the results.

@psaavedra
Copy link
Member

This is interesting. I wonder if you could try again with main/master but reverting WPE - Make TouchPoint ID unique - commit 057b735cbf912f620273106541f2196191c5aa53 and reporting back the results.

update: From the https://bugs.webkit.org/show_bug.cgi?id=270516 link I see @gebausim you are pointing to the same direction. 🙏🏼

clopez added a commit to clopez/cog that referenced this issue Jul 18, 2024
@clopez
wl: do not set to null the surface and target at touch_on_up()
2b757bb 
On a multitouch screen, when several fingers are on the screen at
the same time, if we set to null surface and target on touch_on_up()
then we stop all the ongoing touch gestures and not only the one
of the finger that caused the up event. That is becasue
touch_on_motion() will refuse to process further events if surface
or target are null.

Instead of doing that do not set to null this pointers on the
touch_on_up() event. The value of those will be set again on
the next touch_on_down() event in any case.

Related: Igalia#698
Related: https://bugs.webkit.org/show_bug.cgi?id=270516
@clopez clopez mentioned this issue Jul 18, 2024
Merged
@clopez
Copy link
Contributor

clopez commented Jul 18, 2024

This two patches fix this issue for me:

@gebausim
Copy link
Contributor Author

Just tested with the patches applied and everything seems to work as expected, thanks!

clopez added a commit to clopez/cog that referenced this issue Aug 27, 2024
@clopez
wl: do not set to null the surface and target at touch_on_up()
f357aa0 
On a multitouch screen, when several fingers are on the screen at
the same time, if we set to null surface and target on touch_on_up()
then we stop all the ongoing touch gestures and not only the one
of the finger that caused the up event. That is becasue
touch_on_motion() will refuse to process further events if surface
or target are null.

Instead of doing that do not set to null this pointers on the
touch_on_up() event. The value of those will be set again on
the next touch_on_down() event in any case.

Also make seat->touch_target a weak reference, that way if the
CogViewport gets destroyed before the touch-move and/or touch-up
events arrive it will be set to NULL automatically and we avoid
use-after-free of a stale pointer.

Related: Igalia#698
Related: https://bugs.webkit.org/show_bug.cgi?id=270516
aperezdc pushed a commit that referenced this issue Aug 27, 2024
@clopez @aperezdc
wl: do not set to null the surface and target at touch_on_up()
edafaa0 
On a multitouch screen, when several fingers are on the screen at
the same time, if we set to null surface and target on touch_on_up()
then we stop all the ongoing touch gestures and not only the one
of the finger that caused the up event. That is becasue
touch_on_motion() will refuse to process further events if surface
or target are null.

Instead of doing that do not set to null this pointers on the
touch_on_up() event. The value of those will be set again on
the next touch_on_down() event in any case.

Also make seat->touch_target a weak reference, that way if the
CogViewport gets destroyed before the touch-move and/or touch-up
events arrive it will be set to NULL automatically and we avoid
use-after-free of a stale pointer.

Related: #698
Related: https://bugs.webkit.org/show_bug.cgi?id=270516
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@clopez @psaavedra @aperezdc @gebausim