Bug 1348 - Add X11 Touch Event Support to Generate NEWT Mouse/Pointer Events
Summary: Add X11 Touch Event Support to Generate NEWT Mouse/Pointer Events
Status: VERIFIED FIXED
Alias: None
Product: Newt
Classification: JogAmp
Component: x11 (show other bugs)
Version: 2.4.0
Hardware: All linux
: P4 enhancement
Assignee: Sven Gothel
URL:
Depends on:
Blocks:
 
Reported: 2016-11-17 13:25 CET by danny.koernig
Modified: 2019-03-28 18:37 CET (History)
1 user (show)

See Also:
Type: FEATURE
SCM Refs:
jogl 92006e4baef57f1f3fb647dd307aed5989fd4c8d jogl 746383476aa449e9cab4a25df27be85b61aa074b jogl b32541efc1bef773c4f1bbd06d0885ee79821865 jogl ec4721c5b81ca39355f660294bf45edc0a1584da
Workaround: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description danny.koernig 2016-11-17 13:25:53 CET
X11 server API supports touch events, which could be supported by NEWT
to generate NEWT's Mouse/Pointer events.

More Information can be found here: http://www.x.org/wiki/Development/Documentation/Multitouch/

I've patched this into the jogl version 2.3.2 already. I will make a branch on github for my changes and send a pull request.
Comment 1 danny.koernig 2016-11-17 14:50:31 CET
Here is my pull request: https://github.com/sgothel/jogl/pull/102
Comment 2 Julien Gouesse 2016-11-23 17:15:21 CET
Very good job and your pull request is quite clean :)
Comment 3 Sven Gothel 2019-03-27 00:09:57 CET
Thank you Danny, reviewing your pull request.
Excellent work.

The only issue I have is the usage of the static 'touch_coordinates' array,
which may lead to threading issues having multiple EDT threads on different X11 Displays running?

I will pull and merge your work and may add a fix to this.
Comment 4 Sven Gothel 2019-03-27 03:12:41 CET
commit 746383476aa449e9cab4a25df27be85b61aa074b

(Note to Danny: I cannot test this now - please re-test and/or review)
    
X11Common::JavaWindow
- Owns XI extension's xiOpcode, selected xiTouchDeviceId and tracked XITouchPosition array
    
X11Window::CreateWindow
- Query XI extension only once @ window creation and earmark xiOpcode in JavaWindow instance
    
- Fix: Device selection code was "class->type != XITouchClass",
  but shouldn't it be 'XITouchClass == class->type' (as patched here)
    
- Fix: Free XIQueryDevice returned XIDeviceInfo array via XIFreeDeviceInfo
    
- Earmark deviceid in JavaWindow instance
    
X11Display
- Moved global static touch_coordinates to JavaWindow::xiTouchCoords instance
    
X11Display::DispatchMessage
- Changed event handling structure similar to https://keithp.com/blogs/Cursor_tracking/
    
- Fix: Free XGetEventData's optional memory allocation via XFreeEventData
    
- Reuse JavaWindow's queried xiOpcode
    
- Fix: Don't overrise windowPointer, instead validate and require a match. JavaWindow must match!
    
- Fix: Also validate chosen deviceid with JavaWindow's registered device
    
Newt Build:
- Added libXi in build recipe and doc
Comment 5 Sven Gothel 2019-03-27 03:45:32 CET
Add more verbose DBG_PRINT
- @ CreateWindow: extension, scanning device/class, registered deviceid
- @ DispatchMessage: XI_TouchBegin, XI_TouchUpdate and XI_TouchEnd
    
On my test machine w/o a touchscreen I correctly:
- detected extension
- detected no XITouchClass device, hence no deviceid registered

This needs validation with a touchscreen now :)

X11: [CreateWindow]: XI: Window 0x6600016, Extension 131
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[1/7]: type 1 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[2/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[3/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[4/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[5/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[6/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[7/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[2/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[3/13].class[1/3]: type 1 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[3/13].class[2/3]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[3/13].class[3/3]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[4/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[5/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[6/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[7/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[1/7]: type 1 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[2/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[3/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[4/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[5/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[6/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[7/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[1/7]: type 1 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[2/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[3/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[4/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[5/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[6/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[7/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[10/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[11/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[1/7]: type 1 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[2/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[3/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[4/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[5/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[6/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[7/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[13/13].class[1/1]: type 0 (is XITouchClass 0)
Comment 6 danny.koernig 2019-03-27 07:44:54 CET
Sven, thanks for the review and improvements.

I tested your changes and unfortunately it didn't work.

I got following warning:
Warning: NEWT X11 DisplayDispatch 0x7fba740018f0, Couldn't handle event 35 for X11 window 0x1300000083

I will take a deeper look to the problem in the next days.
Comment 7 Sven Gothel 2019-03-27 18:56:49 CET
Fix X11 XI Multitouch
    
I got access to a touchscreen laptop w/ Debian 9, hence I could fix and test the implementation.
    
X11 DisplayDriver.java:
- Store and pass through xi_opcode of XI extension, queried at initialization stage
    
X11Window.c Fixes:
- Initialize JavaWindow's xiTouchCoords[].id w/ -1, as required to track the pointer
    
- Pass through xi_opcode as stored in X11 DisplayDriver
    
X11Display.c Fixes:
- sendTouchScreenEvent: Throw RuntimeException if 0 > actionId (Internal Error: based on xiTouchCoords[].id tracking)
    
- DispatchMessages's windowPointer determination:
-- Query potenial XI Event first: IF XI Event, must use XIDeviceEvent's event Window
-- Only IF not an XI Event, we can use evt.xany.window as the event window
    
- DispatchMessages's XI Event Handling:
-- Always break deviceid search loop if id found, preserving index and time spend
    
Works on my Debian 9 device, tested w/ com.jogamp.opengl.test.junit.jogl.demos.es2.newt.TestGearsES2NEWT:
- One pointer (finger) press, drag and release (click)
- PinchToZoomGesture works
- DoubleTabScrollGesture works
    
+++ 
    
Potential Issues:
    
JavaWindow's xiTouchCoords[].id accuracy is crucial to pointer tracking
during XI_TouchBegin -> XI_TouchUpdate -> XI_TouchEnd.
    
In the normal course of action:
- XI_TouchBegin sets the id, assuming it is yet set 
- XI_TouchUpdate assumes it is set 
- XI_TouchEnd clears the id, assuming it is set 
    
This field in the JavaWindow array only gets reset to -1 once at native window
creation. We may need to figure out when to reset this field to -1. 
    
If the XI_TouchEnd events would get lost for whatever reason,
the above tracking state would be broken.

+++

In case there are issues left and/or a good preemptive solution for above
potential issue is found: reopen and add the fix before 2.4.0 release.
Comment 8 danny.koernig 2019-03-28 07:50:14 CET
Works for me too. Thank you!

In my opinion the potential issue is not so great. The implementation supports 10 finger multitouch. If one XI_TouchEnd is lost (because of a bug of X11 or the display driver?) there still stay 9 fingers ;)

Possible solution:
* struct XITouchPosition gets a new field which is set with the current timestamp on every XI_TouchBegin and XI_TouchUpdate event
* on every new event we check the stored timestamps. At timestamps older than 30 seconds we can assume the XI_TouchEnd event was lost and we reset the id to -1 (also we send EVENT_MOUSE_RELEASED to the application?)
Comment 9 Julien Gouesse 2019-03-28 12:43:39 CET
(In reply to danny.koernig from comment #8)
> Works for me too. Thank you!
> 
> In my opinion the potential issue is not so great. The implementation
> supports 10 finger multitouch. If one XI_TouchEnd is lost (because of a bug
> of X11 or the display driver?) there still stay 9 fingers ;)
> 
> Possible solution:
> * struct XITouchPosition gets a new field which is set with the current
> timestamp on every XI_TouchBegin and XI_TouchUpdate event
> * on every new event we check the stored timestamps. At timestamps older
> than 30 seconds we can assume the XI_TouchEnd event was lost and we reset
> the id to -1 (also we send EVENT_MOUSE_RELEASED to the application?)

Personally, I'd prefer leaving the source code as it is right now as we're not sure that this problem will really occur and the suggested fix is messy. I think that the time stamp is already stored elsewhere.
Comment 10 Sven Gothel 2019-03-28 17:19:41 CET
(In reply to Julien Gouesse from comment #9)
> (In reply to danny.koernig from comment #8)
> > Works for me too. Thank you!
> > 
> > In my opinion the potential issue is not so great. The implementation
> > supports 10 finger multitouch. If one XI_TouchEnd is lost (because of a bug
> > of X11 or the display driver?) there still stay 9 fingers ;)
> > 
> > Possible solution:
> > * struct XITouchPosition gets a new field which is set with the current
> > timestamp on every XI_TouchBegin and XI_TouchUpdate event
> > * on every new event we check the stored timestamps. At timestamps older
> > than 30 seconds we can assume the XI_TouchEnd event was lost and we reset
> > the id to -1 (also we send EVENT_MOUSE_RELEASED to the application?)
> 
> Personally, I'd prefer leaving the source code as it is right now as we're
> not sure that this problem will really occur and the suggested fix is messy.
> I think that the time stamp is already stored elsewhere.

Agreed. We could handle this from the Java side and in case we need to reset the touch IDs issue a native call to X11Window.c.

Will let this idea keep floating around for a while .. 2.4.0 release is not yet due anyways.

Great we have this feature finally supported on X11 now,
thank you for your initial 'wall breaking' contribution!